TanStack / router

🤖 Fully typesafe Router for React (and friends) w/ built-in caching, 1st class search-param APIs, client-side cache integration and isomorphic rendering.
https://tanstack.com/router
MIT License
7.52k stars 528 forks source link

Accessibility #918

Open mayank99 opened 7 months ago

mayank99 commented 7 months ago

Describe the bug

Now that tanstack router has reached v1, I was excited to try it because accessibility was promised for 1.0. I fired up one of the examples and was very disappointed to find no route announcements or focus management.

When a screen-reader user (or even a sighted keyboard user) activates a regular link, the browser will move focus to the top of the new page and the screen-reader announces meta information about the new page.

When the same user activates a <Link> from tanstack-router, the focus stays where it is or gets lost (if the link is no longer present on the new route), and the screen-reader provides absolutely no feedback. From a blind user's perspective, it's as if nothing happened or their input was ignored.

Okay, fair enough, maybe it's opt-in or there's some helpers available. So I go through the docs, and am again disappointed to find no mention of accessibility at all. So tanstack-router users are completely on their own if they want to make their applications accessible. In absence of any guidance, it's safe to assume that many (if not most) developers are not going to do that.

Your Example Website or App

https://stackblitz.com/github/tanstack/router/tree/main/examples/react/kitchen-sink-file-based

Steps to Reproduce the Bug or Issue

  1. Open the page in any common screen-reader+browser combination (such as Safari+VoiceOver or Firefox+NVDA or Chrome+Talkback Android).
  2. Traverse through the page using arrow keys (might also need to hold down a modifier key; see VO instructions) or Tab.
  3. Activate one of the links in the sidebar.
    • The navigation happens silently, focus stays on the link.

Expected behavior

Screen-reader users should receive some feedback when clicking a link, such as "Navigated to page [title]". Focus should also generally move to the top of the new view (with some exceptions).

Not both of these (triggering an announcement and moving focus) need to happen together; one or the other may be sufficient depending on the scenario. What matters is that there is some feedback, and that it's easy to configure from a developer perspective.

Screenshots or Videos

No response

Platform

All browsers / all OS

Additional context

I highly recommend reading Marcy Sutton's writeup on accessible client-side routing techniques which includes real user testing and actionable suggestions for improving the experience for disabled users.

CHE1RON commented 6 months ago

@mayank99 This is a great write-up, thanks for posting it! I guess a11y will definitely happen down the road, considering that the project is still somewhat in its infancy .. 🤷

TheBinaryGuy commented 4 months ago

As a temporary workaround, you could use @react-aria/live-announcer combined with static route data to announce route changes.

Here's a small PoC, not sure if it's the "right" way though: https://github.com/TheBinaryGuy/tsr-live-announcer

Important part is the RouteAnnouncer component.

gregmsanderson commented 1 month ago

Ah. That's a bit of a concern. I've just been working through the docs.

Given the mention of React Aria, I see they have a Link componentt https://react-spectrum.adobe.com/react-aria/Link.html and they have a strong focus on accessibility. However I would then miss out on some of the extra features the provided Tanstack Router Link component has (preload intent, activeOptions etc). Hmm ...

CHE1RON commented 1 month ago

Have a look at their RouterProvider for that 😉

gregmsanderson commented 1 month ago

@CHE1RON Thanks.

Yep, so good news is they have a guide specifically mentioning using it with Tanstack Router to get client-side routing working: https://react-spectrum.adobe.com/react-aria/routing.html#tanstack-router

I noticed a missing import in their example code ToPathOption but even with that added, this line href: ToPathOption<RegisteredRouter["routeTree"]>; is showing as a TypeScript error:

Type 'Route<any, "/", "/", string, "__root__", {}, {}, {}, {}, {}, {}, {}, RouteContext, RouteContext, RouteContext, {}, {}, {}, { ...; }>' does not satisfy the constraint 'AnyRouter'.
  Type 'Route<any, "/", "/", string, "__root__", {}, {}, {}, {}, {}, {}, {}, RouteContext, RouteContext, RouteContext, {}, {}, {}, { ...; }>' is missing the following properties from type 'Router<any, any, any, any>': tempLocationKey, resetNextScroll, latestLoadPromise, subscribers, and 40 more.

I tried regenerating the routes but no difference.

Not sure if it's because there is also a RouterProvider in main.tsx (that's the TanStack Router). However the React Aria docs say to put their RouterProvider in the root route which is in __root.tsx.

CHE1RON commented 1 month ago

Try this:

declare module '@adobe/react-spectrum' {
  interface RouterConfig {
    href: ToPathOption<RegisteredRouter, '/', '/'> | ({} & string);
    routerOptions: Omit<NavigateOptions, 'to' | 'from'>;
  }
}

<Provider
  // ..

  router={{
    /**
     * NOTE:
     * This DOES NOT represent the "official" implementation as described in the docs,
     * but rather a workaround from the nice people over at GitHub,
     * see https://github.com/adobe/react-spectrum/issues/6413
     */
    navigate: (to, options) => router.navigate({ ...options, to: to as ToPathOption<RegisteredRouter, '/', '/'> }),
    useHref: (to) => router.buildLocation({ to }).href,
  }}
>
  {props.children}
</Provider>
gregmsanderson commented 1 month ago

@CHE1RON Ah, ok. Thanks. I see from the referenced issue you've come across this one before!

I'm not sure if you added that to the main.tsx in place of the Tanstack RouterProvider, or to __root.tsx? Neither seem to quite match up for me (at least based on a copy of their example app using file-based configuration) and it seems to throw even more errors.

I basically copied the files from:

https://tanstack.com/router/latest/docs/framework/react/quick-start#srcmaintsx

Good news is I did get my TypeScript error to go away by following the comments in that issue. But it seems from this open issue:

https://github.com/adobe/react-spectrum/issues/6587

... that regardless it still may not be handled correctly. Hmm.

Plus even with that integration you lose things like preload that are nice. At least based on Devon's post here:

https://github.com/adobe/react-spectrum/issues/5476#issuecomment-1942214623

It's not clear what the best approach is: it seems like the Tanstack Router Link component is the most comprehensive while the React Aria one is the most accessible.

gregmsanderson commented 1 month ago

Ok, having gone back and forward it seems to me like the best option (at least for now) is to stick with Tanstack Router's Link component. It avoids all of the issues referenced above.

That still leaves the open question (which prompted this to begin with!) of accessibility. Having replicated the findings of @mayank99 with VoiceOver I've gone for the suggestion from @TheBinaryGuy and added a route announcer. That at least informs the user that the navigation has happened.

Thanks @CHE1RON for your help trying to get RAC working.

levrik commented 1 month ago

@gregmsanderson There's a fix for https://github.com/adobe/react-spectrum/issues/6587 which is waiting to get reviewed in https://github.com/adobe/react-spectrum/pull/6591. I'm already successfully using it through pnpm patch at work in production.

I also created wrapper components for RAC. Internally I'm simply remapping all of TanStack Router's props to href and routerOptions. Auto-completion works like with TanStack Router's native Link component as I'm using the same generics setup. Preloading doesn't work right now but should be possible to add by some more rewiring. It's not super pretty behind the scenes but it combines the best of both worlds. Sadly I cannot share the code as it's part of a proprietary codebase but you might get the idea.