[Tabs] not working correctly with `react-router-dom`

[norman-ags]

Bug report

Current Behavior

When content is long and you scroll down a bit, navigating to other route/tab with TabTrigger with react router-dom Link doesn't change its route.

Here's a video of showing it: https://streamable.com/vrw0hg

Expected behavior

When content is long and you scroll down a bit, navigating to other route/tab with TabTrigger with react router-dom Link should change its route.

Reproducible example

Repro

Your environment

Software	Name(s)	Version
Radix Package(s)	Tabs	0.1.4
React	n/a	17.0.2
Browser	Chrome	97.0.4692.99
Assistive tech
Node	n/a
npm/yarn
Operating System

[jjenzz]

This is happening because:

• Tabs change the active value on pointer down but react-router-dom updates the URL on click
• Your pointer down tries to change the active tab value to "account" (which is why password content disappears) but cannot because your react-router-dom logic hasn't mounted it yet
• When you release your mouse it is no longer over the tab so does not trigger click on Link to change the URL
• Account tab content is never mounted by the router

CleanShot.2022-02-08.at.15.35.32.mp4

You might find this example with react-router-dom works a bit smoother https://codesandbox.io/s/cranky-curie-hnshz?file=/src/App.js

Or you can control tabs to change when clicked instead of pointer down but that relies on knowing our implementation detail which may change in the future so your logic could break https://codesandbox.io/s/relaxed-gauss-rvs8z?file=/src/App.js

I'll close this for now as it isn't a bug with the component itself but feel free to continue discussing here if there is anything else we can help with.

[norman-ags]

I'll go with the first sandbox :D. Thanks @jjenzz for taking the time, I really appreciate it!

[johnopleda]

Hi @jjenzz I'm not sure if this is related but we tried implementing the first sandbox in a nested tab and the route isn't changing https://codesandbox.io/s/xenodochial-mountain-m90rl?file=/src/App.js

Here's the recording for reference: https://www.loom.com/share/5461beba6dde4fa6a481f2131e4526a1

[jjenzz]

@johnopleda ah, that is the same issue. The tab content is updating on pointer down so your mouse is not over the tab anymore when you release your pointer and click never fires on the Link to change the url.

It seems the best solution would be to control it. Here's a more declarative way of controlling that just dawned on me and wouldn't mean a reliance on implementation detail remaining stable https://codesandbox.io/s/zen-frog-88tv3?file=/src/App.js

[jjenzz]

@benoitgrelard @andy-hook this has made me wonder if we should be firing event.currentTarget.click() on pointerdown for tab triggers given that pointerdown is the event we're determining should activate them. That would cause tabs to activate links on pointer down.

We made a similar enhancement for dropdown item/anchor compositions. I'll reopen while we think about this.

[benoitgrelard]

@benoitgrelard @andy-hook this has made me wonder if we should be firing event.currentTarget.click() on pointerdown for tab triggers given that pointerdown is the event we're determining should activate them. That would cause tabs to activate links on pointer down.

We made a similar enhancement for dropdown item/anchor compositions. I'll reopen while we think about this.

I always feel confused by these situations (ie. using Tabs for router links). Didn't we discuss these a few times and said that they are usually different patterns? ie. Tabs for inline content in an app, vs. a navigation list (which looks like tabs) with aria-current="page".

[jjenzz]

I'm wondering if there is a valid use-case for something like this (a nested route):

The Home / Videos / Playlists tabs here don't change the entire page (so not app navigation), they just change context in the current page but also update the URL for permalinking.

[benoitgrelard]

But the same thing can be said of Github here no?

and that's the aria-current approach there.

[jjenzz]

I really don't know the answer tbh. I thought the same as you initially but then i found the youtube example which uses role=tab and it muddied the waters.

what if you have tabs in your app that aren't app navigation (so no aria-current) and then the boss asks for them to have permalink functionality for sharing? i've had this exact use-case before on a previous app where app state was in URL.