[WIP] Pull out routing #262
Conversation
| // * config, pulls out the config for each page, then generates a list of Route components with the | ||
| // * config as props | ||
| // */ | ||
| // export function createRoutes(formConfig) { |
There was a problem hiding this comment.
Copied this function into the starter app and tested using it there to create the routes to make sure everything still works with this pulled out.
src/js/containers/FormPage.jsx
Outdated
| } | ||
|
|
||
| goBack = () => { | ||
| const { form, route: { pageList }, location } = this.props; | ||
| const path = getPreviousPagePath(pageList, form.data, location.pathname); | ||
|
|
||
| this.props.router.push(path); | ||
| // this.props.router.push(path); | ||
| this.props.route.goToRoute(path); |
There was a problem hiding this comment.
I need a way to pass a function to <FormPage/> from the starter app that will push up the next path we want to go to to the starter app. Once the starter app has the next path, it can determine the routing to get there. The tricky part is how to create this hook from the starter app side. <FormPage/> is rendered as the component for a particular route, not directly rendered in a parent component, so it gets a little tricky to pass props to it.
There was a problem hiding this comment.
I think the way to do this is to have the app have a component, say WholePage, that gets withRoutered. That component puts together the parts of the page that the app wants, including the FormPage but also a header, footer, sidebar, or whatever. Any of that can be route specific. When WholePage renders it passes the current route (or page key or whatever lets USFS find the right page) to FormPage. Note that this gives the app flexibility over layout if they want it, although I'd expect our WholePage implementation does what we do today except that we'd put the formConfig.footer in it.
There was a problem hiding this comment.
Yes that's what I was thinking, but also including the navigation buttons. This moves us towards a USFS library that is focused on rendering and validating form fields, with basically nothing else. All of the surrounding structure, like header, footer, navigation, etc., exists in the starter app. Just confirming that's the direction we think it makes sense to go in?
|
yes!
Sent from my iPhone
On Sep 12, 2018, at 1:55 PM, Anne Kainic <[email protected]<mailto:[email protected]>> wrote:
@annekainicUSDS commented on this pull request.
________________________________
In src/js/containers/FormPage.jsx<#262 (comment)>:
}
goBack = () => {
const { form, route: { pageList }, location } = this.props;
const path = getPreviousPagePath(pageList, form.data, location.pathname);
- this.props.router.push(path);
+ // this.props.router.push(path);
+ this.props.route.goToRoute(path);
Yes that's what I was thinking, but also including the navigation buttons. This moves us towards a USFS library that is focused on rendering and validating form fields, with basically nothing else. All of the surrounding structure, like header, footer, navigation, etc., exists in the starter app. Just confirming that's the direction we think it makes sense to go in?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub<#262 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AXw_4eIjXBT7YFhNT_mU4SE-FAR-tboIks5uaUpygaJpZM4Wly6D>.
|
|
@jcmeloni-usds Yes, I talked with @annekainicUSDS yesterday and she pointed out that the current nav would also go into the app. The nice thing about that is the app can implement a left nav rather than top, or add route-context-sensitive help text in a sidebar for example, but the forms system doesn't need to be involved in that. |
There was a problem hiding this comment.
In order to pull out routing, I tried keeping the knowledge of the form state within USFS as opposed to trying to pull that out also. That became complicated, as many of the routing decisions need to have a knowledge of the current form data (mainly due to array pages and conditionally hidden pages). Figuring out how to pass information about the form state up to the routing components was complex and messy, and didn't seem like the best design.
| @@ -49,36 +49,15 @@ class FormPage extends React.Component { | |||
| newData = _.set([this.props.route.pageConfig.arrayPath, this.props.params.index], formData, this.props.form.data); | |||
| } | |||
| this.props.setData(newData); | |||
| } | |||
|
|
|||
| onSubmit = ({ formData }) => { | |||
There was a problem hiding this comment.
The onChange function in this component provides the same functionality as the onSubmit function except for the router push, which is exactly what we wanted to extract. Therefore, all I had to do was pull out the onChange and goBack functions (all goBack does is the router push), and provide a hook into the onChange function for the high-order component to know when it can push the next path (i.e., after the new data has been set).
|
Closing for now. |
Addresses #257.
Types of changes
Checklist:
npm run lint.npm run build.npm test.