[WIP] Move routing here #21
Conversation
| schema: { | ||
| type: 'object', | ||
| properties: {} | ||
| properties: { |
There was a problem hiding this comment.
This stuff is added for testing purposes, I'll revert these changes once the PR is working.
js/routes.jsx
Outdated
| pageConfig: page, | ||
| pageList, | ||
| urlPrefix: formConfig.urlPrefix, | ||
| goToRoute, |
There was a problem hiding this comment.
Because FormPage is rendered here with a route, I wasn't sure how to pass the necessary goToRoute function to it. I did it by passing it to the route object, which is accessible from within FormPage as this.props.route.goToRoute(), which doesn't seem ideal because if someone is using react-router v4 or a different router altogether, they will probably pass this function to FormPage differently, and it wouldn't be under this.props.route, so hard-coding that in USFS doesn't seem like a good idea.
There was a problem hiding this comment.
There's a more direct way to pass props to a component in react-router v4: https://tylermcginnis.com/react-router-pass-props-to-components/
js/routes.jsx
Outdated
| const routes = createRoutes(formConfig); | ||
| // const routes = createRoutes(formConfig); | ||
|
|
||
| function goToRoute(path) { |
There was a problem hiding this comment.
This is just a mock so I can make sure the path is getting passed up, which it is. The problem is now how to trigger the router to change the path... I think that needs to be done from within a component, but I'm not sure where to put that, and how that component would also have access to the path.
| @@ -0,0 +1,87 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
The motivation for moving the <FormApp/> component from USFS into the starter app is that with pulling out routing we are also going to pull out navigation elements, footer, header, nav, basically anything that wraps around a section of questions and has opinions about what a form page should look like. We think that should be the responsibility of the starter app so that users can decide their own navigation elements or headers and footers if needed.
| */ | ||
| class FormApp extends React.Component { | ||
| componentWillMount() { | ||
| // setGlobalScroll(); |
There was a problem hiding this comment.
Still need to figure out where the global scroll should be set. My inclination is within the starter app, but there are many lower level components in USFS that need knowledge of a global scroll. In general I think this is a bad pattern, but it's what we have now. We should come up with a better solution for setting scroll in components that need it.
| onSubmit={this.onSubmit} | ||
| onPageChange={this.getUpdatedFormData}> | ||
| </FormPage> | ||
| <div className="row form-progress-buttons schemaform-buttons"> |
There was a problem hiding this comment.
One of the problems we have in regards to navigation buttons comes from a component called <Form/>, which comes from rjsf and lives further down the component hierarchy tree as a child of <SchemaForm/>. There's logic in <Form/> that automatically renders a submit button if there aren't other children elements to render. Before we moved the navigation elements out of USFS, they were nested under <SchemaForm/> in <FormPage/>, which meant they were passed as children to <Form/>. Because they are no longer passed as children, <Form/> is rendering a separate submit button in addition to these two. Not sure of the best way to resolve this problem other than changing the logic in <Form/>.
| } | ||
| } | ||
|
|
||
| getUpdatedFormData = (formData) => { |
There was a problem hiding this comment.
This is the new hook for getting the updated formData after setData has been called within <FormPage/>'s function onChange in USFS. Not sure if this is the best pattern :/
| <div className="row form-progress-buttons schemaform-buttons"> | ||
| <div className="small-6 medium-5 columns"> | ||
| {!isFirstRoutePage && | ||
| <ProgressButton |
There was a problem hiding this comment.
These buttons aren't actually firing anything when clicked. Need to figure out why.
|
Closing for now. |
Addresses usds/us-forms-system#257