Skip to content

fix(v2): select correct tab when items are incorrectly ordered#4468

Merged
slorber merged 1 commit intofacebook:masterfrom
armano2:fix/tabs-unordered
Mar 19, 2021
Merged

fix(v2): select correct tab when items are incorrectly ordered#4468
slorber merged 1 commit intofacebook:masterfrom
armano2:fix/tabs-unordered

Conversation

@armano2
Copy link
Copy Markdown
Contributor

@armano2 armano2 commented Mar 19, 2021

Motivation

fixes #4465

Have you read the Contributing Guidelines on pull requests?

yes

Test Plan

create md(x) file in project with

import Tabs from '@theme/Tabs';
import TabItem from '@theme/TabItem';

<Tabs values={[ {label: 'Android', value: 'android'}, {label: 'iOS', value: 'ios'}, {label: 'Web', value: 'web'}, ]}>
  <TabItem value="web">Web</TabItem>
  <TabItem value="android">Android</TabItem>
  <TabItem value="ios">iOS</TabItem>
</Tabs>

currently there is no component gallery or place where specific test can be added

simple regression test can be done by checking: https://deploy-preview-4468--docusaurus-2.netlify.app/docs/typescript-support

@armano2 armano2 requested review from lex111 and slorber as code owners March 19, 2021 12:51
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Mar 19, 2021
@netlify
Copy link
Copy Markdown

netlify bot commented Mar 19, 2021

const handleTabChange = (
event: React.FocusEvent<HTMLLIElement> | React.MouseEvent<HTMLLIElement>,
) => {
const selectedTab = event.currentTarget;
Copy link
Copy Markdown
Contributor Author

@armano2 armano2 Mar 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we actually wants to use currentTarget here, instead of target
In case if tab header is not simple element (eg. swizzled) this will stop working

target: is the element that triggered the event (e.g., the user clicked on)
currentTarget: is the element that the event listener is attached to.

https://developer.mozilla.org/en-US/docs/Web/API/Event/currentTarget

@netlify
Copy link
Copy Markdown

netlify bot commented Mar 19, 2021

Deploy preview for docusaurus-2 ready!

Built without sensitive environment variables with commit a504a80

https://deploy-preview-4468--docusaurus-2.netlify.app

@github-actions
Copy link
Copy Markdown

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 77
🟢 Accessibility 96
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

Lighthouse ran on https://deploy-preview-4468--docusaurus-2.netlify.app/

const selectedTab = event.currentTarget;
const selectedTabIndex = tabRefs.indexOf(selectedTab);
const selectedTabValue = children[selectedTabIndex].props.value;
const selectedTabValue = values[selectedTabIndex].value;
Copy link
Copy Markdown
Contributor Author

@armano2 armano2 Mar 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of checking childrens we should check values, this also solves crash with missing TabItem during development

@slorber slorber added the pr: bug fix This PR fixes a bug in a past release. label Mar 19, 2021
@slorber
Copy link
Copy Markdown
Collaborator

slorber commented Mar 19, 2021

Seems to work fine! thanks

@slorber slorber merged commit 80e40c3 into facebook:master Mar 19, 2021
@armano2 armano2 deleted the fix/tabs-unordered branch March 19, 2021 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Signed Facebook CLA pr: bug fix This PR fixes a bug in a past release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

<Tabs> should be insensitive to ordering mismatches

4 participants