-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add navigator #73
Add navigator #73
Conversation
The navigator allows us to split up large forms into multiple pages. It has a side navigation that allows any page to be selected. The side nav displays the state of each page.
Switch from using fireEvent to userEvent. Will update additional tests as I come accross them. Reduce the instances of using "act" in tests, now we properly wait for all the rerenders to occur. Some tests still need to use act. Most deal with third party libraries
Conflicts: frontend/src/App.js frontend/src/pages/Unauthenticated/index.js
Conflicts: frontend/yarn.lock
Conflicts: frontend/yarn.lock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice!
A lot of great features.
I did notice the UI jump a bit when navigating sections, but this is minor.
I wonder if we should start adding validations to the UI. I know we have a separate PR for validations. Do you think it would be easier to handle them all at once or start implementing them at least partially, e.g. the file uploads validations?
We should probably also add some cucumber tests, either here or in a different PR.
There shouldn't be too many fields that require UI validations as most fields are select-boxes or radio/check-boxes. The fields that do require validations (text or number fields) should be easy to use because we are using
My vote would be a different PR, this PR is already pretty big. |
Description of change
The navigator allows us to split up large forms into multiple pages. It has a side navigation that allows any page to be selected. The side navigation displays the state of each page and is stickied to the top of the page. Some notes:
react-helmet
to help update page titles. This library helps keep the page title updated with the page the user is viewing in the activity report.act(...)
in tests. Previously we would get warnings that looked likeWarning: An update to *COMPONENT* inside a test was not wrapped in act(...).
. This indicates updates to the DOM were happening outside of a test. I've updated tests to not need to be wrapped in act. Some tests still use act but they deal with third party libraries or using mock timers.fireEvent
withuserEvent
per this suggestion. Will make a separate PR to switch everything overAfter Fix url routing #62 is merged in I'll update this branch to deploy to sandbox (unless you are using sandbox @SarahJaine!).How to test
OR
localhost:3000/activity-report
Issue(s)
Checklist