Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Upgrade to react router 4 #27

Closed
wants to merge 5 commits into from

Conversation

annekainicUSDS
Copy link
Contributor

@annekainicUSDS annekainicUSDS commented Nov 19, 2018

WIP version of ugprading to react-router v4. So far I've gotten things to mostly work by hard-coding the routes, but to programmatically generate them from a config is not working. Various things I've tried so far don't seem to illuminate why that is. Looking for a second pair of eyes on this!

const pageList = createPageList(formConfig, formPages);

export default function Form({ location }) {
const introRoute = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now these props passed to each route are hard-coded, but this will go away once we're able to figure out how to generate the routes automatically.

<FormApp formConfig={formConfig} currentLocation={location}>

<Switch>
<Route exact path="/introduction" render={props => (
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only way I've gotten the pages to render properly so far. I've commented out the programmatic way of generating routes at the bottom of this file so we can play around with it, but so far this doesn't seem to be working.

@@ -9,7 +9,7 @@ const formConfig = {
transformForSubmit: '',
submitUrl: '',
introduction: Introduction,
confirmation: '',
confirmation: Introduction,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Temporarily adding this so that the route generation doesn't break because '' isn't a valid React component. Will need to go back and remove/fix this later.


const routes = createRoutes(formConfig);
function createRouteConfig(form) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is a replacement of the createRoutes function from us-forms-system. The main difference right now is that it only generates the config for the main form page routes, not including intro, review-and-submit, and confirmation pages. We'll want to add those back in once we can get the route generation working properly.

"mocha": "^5.2.0",
"react-redux": "^5.1.1",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some packages needed only because I'm testing these changes against my local copy of us-forms-system right now. Can delete this once I revert to using the remote version of us-forms-system.

@dmethvin-gov
Copy link
Contributor

Closing this out but if someone wants to work on a refactor I'll be glad to talk. Ultimately I think the forms library itself shouldn't be wired to a particular router and instead would provide an API so that the app (this repo) would control things.

@dmethvin-gov dmethvin-gov deleted the upgrade-to-react-router-4 branch December 24, 2019 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants