Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Mar 19, 2021

Conversation

armano2
Copy link
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

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Mar 19, 2021
@netlify
Copy link

netlify bot commented Mar 19, 2021

const handleTabChange = (
event: React.FocusEvent<HTMLLIElement> | React.MouseEvent<HTMLLIElement>,
) => {
const selectedTab = event.currentTarget;
Copy link
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

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

⚡️ 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 selectedTabIndex = tabRefs.indexOf(selectedTab);
const selectedTabValue = children[selectedTabIndex].props.value;
const selectedTabValue = values[selectedTabIndex].value;
Copy link
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
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