search

GCTC-NTGC / gc-digital-talent

GC Digital Talent is the new recruitment platform for digital and tech jobs in the Government of Canada. // Talents numériques du GC est la nouvelle plateforme de recrutement pour les emplois numériques et technologiques au gouvernement du Canada.
https://talent.canada.ca
GNU Affero General Public License v3.0
22 stars 9 forks source link

Spike - Are there a11y advantages to React-Router? #2368

Closed tristan-orourke closed 2 years ago

tristan-orourke commented 2 years ago

Description

We're doing frontend routing with a custom hook and UniversalRouter. Are there any major features, especially around accessibility, that React Router offers that we are currently missing?

Timeboxed to 1-2 hours.

tristan-orourke commented 2 years ago

Just from a cursory search, I wonder if it has to do with this - announcing page navigations for screen readers? https://estevanmaito.me/blog/accessible-navigation-using-react-router

patcon commented 2 years ago

What I recall of reach-router and react-router differences and fork/merge:

https://blog.logrocket.com/react-router-v6-future-reach-router/

In May of 2019, Ryan Florence, co-creator of React Router and Reach Router, announced the impending release of a new version of React Router that takes advantage of React’s Hooks API. He also stated that React Router would be the surviving project, while Reach Router will continue to receive support in the form of bug fixes.

Both reach and react-router now handle focus management in much more accessible ways, for both client-side and server-size.

Number of issues mentioning accessibility keywords:

Open questions:

  1. Does accessibility concerns for "old" router versions only apply to server-side (of which we don't currently rely)? Will we ever use server-side rendering?
  2. How will out browser support goals intersect with router choice?
patcon commented 2 years ago

Still need to read this more deeply, but mentions (circa 2019) that server-side rendering is highly recommended for accessibility in govuk react sites: https://github.com/govuk-react/govuk-react/blob/main/ACCESSIBILITY.md (note that the references to the "service standard" points 12 & 13 are for the 2019 version

patcon commented 2 years ago

Via React Router v6 in Three Minutes (Feb 2020):

https://twitter.com/mjackson/status/1229156979714605056

what was the biggest factor that led to this change [in bundle size down 70%]?

A couple of things:

  • dropping support for anything older than IE11
  • dropping support for React < 16.8
  • using Google Closure Compiler
  • writing better code 😅
patcon commented 2 years ago

Not specifically a11y, but here's another downside of not having react-router.

I'm finding it hard to see how component state is exercised for stories whose components contain logic based on location.pathname. As it stands, we'd need to make pathname or location optionally injectable into each component (if it's checking pathname to determine its render state). This is due to how we're implementing universal-router without any context to override the navigator/history object we get from createBrowserHistory():

https://github.com/GCTC-NTGC/gc-digital-talent/blob/156e5e3fbe4f9245bc2006ae70abec74004310ec/frontend/common/src/helpers/router.tsx#L8-L22

For comparison of things possible with react-router, this is what using <MemoryRouter> in a story looks like: https://chestozo.medium.com/how-to-mock-location-inside-storybook-stories-76a7c0705354

Since our useLocation() hook hardcodes the use of createBrowserHistory(), we can't easily use createMemoryHistory() like react-router does. I believe this is what is offered by the extra layer of <Router>, which adds context that stores this kind of stuff, and can be modified.

(I tried making my own <RouterContainer> that defaults a navigator prop to be a createBrowserHistory() object stored in context. This could be overridden in stories via <RouterContainer navigator={createMemoryHistory()}>. But it started to get hairy in common/src/helpers/helpers/router.tsx and so I backed out before hitting success. I needed the new RouterContext from admin side of app, but that can't be imported in common. Wasn't sure it felt like the right approach when react-router offers this already.)

tristan-orourke commented 2 years ago

The answer to the spike seems to be YES