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

GE refactor #1748

Merged
merged 15 commits into from
Jan 28, 2019
Merged

GE refactor #1748

merged 15 commits into from
Jan 28, 2019

Conversation

smcmurtry
Copy link
Collaborator

Working on #1733, this should also make #1478 easier

This is my attempt at a massive refactor of the guided experience with the goal of making it simpler by turning each question into its own page. What do you think - is this simpler?

@sastels sastels temporarily deployed to vac-poc-pr-base-pr-1748 January 24, 2019 20:35 Inactive
@smcmurtry smcmurtry temporarily deployed to vac-poc-pr-base-pr-1748 January 24, 2019 20:36 Inactive
@smcmurtry smcmurtry temporarily deployed to vac-poc-pr-base-pr-1748 January 24, 2019 20:47 Inactive
@brdunfield
Copy link
Collaborator

I know this is still [WIP], but I notice that selected data gets cleared when you go back - is that desirable behaviour? Like if you select "veteran" and proceed forward, when you navigate back using the in-app back button that selection persists, but if you use the browser back it gets cleared.

@smcmurtry smcmurtry temporarily deployed to vac-poc-pr-base-pr-1748 January 25, 2019 16:01 Inactive
@smcmurtry smcmurtry changed the title [WIP] GE refactor GE refactor Jan 25, 2019
@smcmurtry smcmurtry temporarily deployed to vac-poc-pr-base-pr-1748 January 25, 2019 19:55 Inactive
@smcmurtry
Copy link
Collaborator Author

@brdunfield It looks like that is happening because I'm not immediately putting the user's selection into the url query parameters. It gets put into the url when you visit the next page. So when you click browser back, it goes back to a url that doesn't include the query param, and then that clears it from redux.

I tried changing the behaviour to be more like it was before, but it started introduced a lot of complexity to the code. So my preference would be to leave it like this until we get feedback from user testing that it's an issue. If someone hits browser back they probably wanted to re-answer the question anyway.

@brdunfield
Copy link
Collaborator

Tests are passing but getting a few failed propTypes for fontSize on AnchorLink in the tests.

Copy link
Collaborator

@brdunfield brdunfield left a comment

Choose a reason for hiding this comment

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

👍 🦑 Nice work!

@smcmurtry smcmurtry deleted the ge_pages branch February 25, 2019 16:16
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.

3 participants