Closed dpasque closed 1 year ago
Ahhh thank you @karenroldan! 😄 Thank you for taking on this monster of a PR, and I'm so sorry it got so big haha...
@karenroldan Thank you for the review and the feedback! 😄
When using the tab key for navigation, it's not possible to access the "Report an Issue" dropdown. The arrow keys do function correctly, however!
This may sound and feel a little weird initially, but this is actually per WAI ARIA specs! Essentially, you're only supposed to be able to tab into one piece of a menubar
, and then use keyboards to move within then. We are following a "roving tabindex" pattern to make that tabable element whichever one was last focused! 👍
VoiceOver: When navigating to the Reporting Flow page after selecting an issue type, VoiceOver only announces "heading level 2, Report a new issue, main". It would be helpful to provide more context such as announcing the specific form that is currently open.
Oh I love this idea! 😄 I added a screen-reader-only span that is also set as the accessible description of the h2
heading! 🎉
@john-legg Ah shoot, sorry I missed that! Thanks for the heads up! 😄
Got those styles fixed 👍 Thank you again for the review! 🙌
What Does This PR Add/Change?
Gahhh, sorry this PR got so big! 🙇 A lot of these page transitions/navigations changes all go together, so it was hard to break it up without creating a big rebase chain. And this change also prompted some needed modularizations to DRY things up!
But all in all, the changes are smaller than what may first appear!
First, some modularizations:
Banner
component for the banners (report a new issue, and start over) at the bottom of both pages.ReportIssueDropdownMenu
that can be used in both the top navbar and the report a new issue banner.PageNavigationProvider
and ausePageNavigation
hook that takes care of doing all the side-effects when we change pages (like focusing the header, scrolling to the top, updating the title).With that, we could finish everything up!
Testing Instructions
A quick heads-up: there is handling in there to not focus on the page subheading (like "Report a new issue") when we first render the app. However, we use
React.StrictMode
to catch errors while developing, andStrictMode
famously renders everything twice. So, that behavior won't work quite right locally. But it will in production! If you want to see for yourself, you can just delete theStrictMode
wrappers inindex.tsx
🙂 .Issues
Related to #
Closes #64