[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

https://user-images.githubusercontent.com/175330/153020849-61f6eab5-0fe7-4dee-866a-0326a74f0d1a.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.

[asherccohen]

I'm really interested in the topic. I have a couple of use cases that could serve as material for thoughts.

First is a minimalistic dashboard screen. Top area is used for data visualization, bottom shows cards used as navigation items. Clicking a card switches charts/tables at the top.

Two possible solutions:

• as tabs (I used this solution): each card is a TabTrigger, content is switched but the URL doesn't know about it, so in order to persist a selection (when moving back and forward) we need to use localstorage/context/global state

• as nested routes (awesome DX in RRv6!!, I prefer this solution but haven't tried it yet): each card is a link, each chart/table renders in an Outlet, we get an active class name and deep linking is a breeze. We can also leverage query params for filters etc..

Second and more complex: a screen with horizontal split view (50/50), top area has a set of tabs, bottom area has a different set of tabs but the two could also be synchronised (switching a top tab might need to switch a specific bottom one and viceversa).

• as tabs (I used this solution): each area is a Tab group, independent from each other, and the page uses context to hold both tabs state and handle synchronisation. We loose all router UX but we can share everything we need to persist in context.

• as nested routes: the setup would be too complex, probably even involving 2 routers (or we could probably do the top area with Links, and the bottom with Tabs, these could synchronise with a route match). Deep linking is still problematic as we don't have the state of the bottom tabs.

Closing questions:

• is this something that Tabs should account for?
• is there a case for creating TabLinks?
• when to use each pattern?

[nikosrossolatos]

@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.

Hey @jjenzz In your code sandbox, if i remove the "replace" prop on the components, and try navigating with the browser buttons nothing happens. Do you have any idea why? I should maybe post this to react-router as it seems like they are more relevant to this

[jjenzz]

@nikosrossolatos the issue is that the tabs component is responding to state change and when you press the back button the state is not being updated to reflect the url. there a few ways to solve this:

• update a key prop, but it will reinstantiate the tabs component every time the url pathname changes so choosing this solution depends on how heavy your tabs are https://codesandbox.io/s/reverent-borg-wxcuwk?file=/src/App.js
• sync the state with the url in an effect https://codesandbox.io/s/sleepy-resonance-1gw8fh?file=/src/App.js

[AlexGalays]

You provided some workarounds but what would be your favorite solution for someone starting this from scratch? Avoid Tabs completely and just sync content with the usual <a> -> URL change -> render something different or use Tabs and every single time this pattern arises, synchronize with the URL manually? Or third, use navigation menu? (not sure yet if it fits all UXs)

I'm biased toward the former because I like the default behavior of anchors (right click, etc) and there's less work, also it feels more robust.

Am I right in assuming that whenever radix-ui export a Trigger component, an actual <button> (and nothing else) is expected, even if we use as?