Side-effects in Tabs component after #152

[ocruze]

Hello, I would like to report some side-effects on the Tabs component after the PR #152.

• left and right arrow key interactions

Suppose there are 4 tabs. At first when the selectedTabIndex = 0, right key takes us to the 3rd tab. For some reason, it skips the 2nd one (I tried looking at the code but couldn't find a reason behind this behaviour). And then it stays stuck in the 3rd because the selectedTabIndex is never updated.

Same thing with the left key. When selectedTabIndex = 0, the left key takes us to the second-last tab, again skipping one tab.

• focus in useEffect

As there's a focus in a useEffect, as soon as the page loads, the selected tab is being focused. This is not really an expected behaviour because there can be other content before the Tabs comp. in a page.

Code to add in the vite example provided in this repo to reproduce the mentioned behaviour:

function TabsExample ( ){
    return (
        <Tabs
            tabs={[
                {
                    label: "Métadonnées",
                    content: <p>...liste de métadonnées...</p>,
                },
                {
                    label: "Jeux de données",
                    content: <p>...liste de jeux de données...</p>,
                    // isDefault: true
                },
                {
                    label: "Services",
                    content: <p>...liste de services...</p>,
                },
                {
                    label: "Services",
                    content: <p>...liste de services...</p>,
                },
            ]}
        />
    )
}

Also, the arrow keys interactions used to work already, so I don't really understand the need for this PR. Can you please explain? Thank you

mentioning the author of the PR: @BrianRid

[garronej]

Thanks for reporting @ocruze,
Sorry about that, I'm a bit overflowed with all my OSS project at the moment. I didn't review this carefully enough.

Do you suggest rolling back?

[ocruze]

Sorry I posted the issue before having finished writing. I updated with more info. Rolling back is the easy solution and would be ok for me, but maybe @BrianRid had something in mind that I didn't understand?

[garronej]

Ok thank you @ocruze,
On first sight I would tend to agree with you @ocruze,
However maybe @BrianRid had some specific insight here.

In absence of feedback I'll rollback since the rule of thumbs for this library is to add as little logic as possible.

[ClementNumericite]

Hi @ocruze, @garronej, I commissioned @BrianRid to do this update for me. I can take over the topic from here.

After some investigations, I understand what happened.

The keyboard navigation issue was only present in controlled Tabs (with selectedTabId), which I initially overlooked. When trying to implement this logic, there was a conflict with the Uncontrolled version which, indeed, was already functioning correctly. (my usecase : https://observatoire.numerique.gouv.fr/Aide/Observatoire)

For the "focus in useEffect" thing you are totally right.

This PR should correct both : https://github.com/codegouvfr/react-dsfr/pull/173

Regards

[ocruze]

Oh ok now I understood, it makes sense. I tested the correction that you provided and can confirm it works as expected in both controlled and uncontrolled Tabs. Thank you ;)

[ClementNumericite]

@ocruze great! apologies for the wast of time and thanks for investigating in the first place :muscle:

[BrianRid]

Hello everyone ! Sorry for the late answer I've been away from my computer for the last few days ! Indeed thank you @ocruze for the analysis ! And thank you @ClementNumericite for the fix provided 👍