-
-
Notifications
You must be signed in to change notification settings - Fork 27
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
1478 - fixes checklist link from faq page #1526
Conversation
* added ChecklistPage including routes in App * added redirect for "faqs/true" which is in DB * updated ModalDialog to have title for a11y and testing * simplified the terms and conditions state management
ProjectTableRow.test needs to pass handleHide skip broken feedback test
…into 1478-b-fix-checklist-link-for-faq
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.
In an effort to be able to access the checklist from an answer in the FAQs, we've completely broken the primary use case for the checklist.
The primary purpose of the checklist is for users in the Wizard to have a way to get a list of the materials and information they need to successfully complete the wizard. At first, we put the checklist directly on the first page of the Wizard, but then decided that it would be better as a modal that is automatically shown when the user starts the Wizard, so they won't miss it (Issue #1086). This was pretty good, but we wanted them to be able to re-open the wizard from any page, so added the link in the toolbar (Issue #1157) that would allow them to re-open the dialog from any page in the wizard, and (later) anywhere in the site for reference purposes. When using the wizard it is important that we do not navigate away from the wizard, because they will lose any work they have done on their calculation. We also added an animation that took 1.5 seconds to close the modal and showed it shrinking and disappearing into the footer link, so users would notice the link, and deduce that the link could be used to re-open the checklist if they needed it again (See comments on Issue #1157).
Normally, if you try to leave the wizard without saving your changes, you should see the NavConfirmModal, asking you if you really want to leave the wizard and lose your changes - but i think this got disconnected about two weeks ago when I upgraded the react router and didn't get this part working again. We need to get this working again, and I'll create an issue for it.
However, if we fix this and leave the checklist implementation as it is in this PR, a user on the wizard with a partially-completed calculation will not be able to access the checklist at all without losing their partially completed calculation or saving it.
Since there a number of other changes included in this PR that are good steps forward, we should probably just try to start restoring the checklist functionality by having the footer Checklist link open the checklist as a dialog without navigating the router. I'll create separate issues to hookup the NavConfirmModal again and re-implement the animation when the confirm dialog is closed.
Thank you, John, that's exactly what I was hoping to understand. I think that a collapsible section would be more usable, but for now I will replace the modal behavior. |
latest commit reverts to old user behavior. first run gets a double modal prompt with terms and checklist. footer link shows the modal. faq still shows a separate page. |
…into 1478-b-fix-checklist-link-for-faq
Fixes #1478 by switching the checklist content from a modal prompt to a navigable page.
What changes did you make?
fix 1478 by making ChecklistPage
* added ChecklistPage including routes in App
* added redirect for "faqs/true" which is in DB
* updated ModalDialog to have title for a11y and testing
* simplified the terms and conditions state management
* clicking question on FAQ now intutively expands answer
* fixed/skipped broken client tests
* fixed hide/unhide in ProjectContextMenu
Why did you make the changes (we will use this info to test)?
checklist modal was not navigable,
see also slack: https://hackforla.slack.com/archives/CKY65G266/p1700149562824069