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

[Tabs] Tabs wrap on window resize #3980

Merged
merged 2 commits into from
Feb 17, 2021
Merged

Conversation

lhoffbeck
Copy link
Contributor

WHY are these changes introduced?

Fixes #3976

The Tabs component (and specifically the TabsMeasurer) currently doesn't take into account what happens when tab widths aren't integers. This can result in the "disclosure tab" (the ... overflow button) being wrapped to the next line.

WHAT is this pull request doing?

This PR rounds up the tab width values and adds equality to the comparison check in Tabs/utilities.ts when determining whether a tab should be wrapped, with the end result being that tabs are hidden 1px sooner but the disclosure tab isn't wrapped.

before
Screen Shot 2021-02-04 at 4 07 30 PM

after
Screen Shot 2021-02-04 at 4 24 03 PM

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:
import React, {useCallback, useState} from 'react';

import {Page, Card, Tabs} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      <TabsExample />
    </Page>
  );
}

function TabsExample() {
  const [selected, setSelected] = useState(0);

  const handleTabChange = useCallback(
    (selectedTabIndex) => setSelected(selectedTabIndex),
    [],
  );

  const tabs = [
    {
      id: 'all-customers-1',
      content: 'All',
      accessibilityLabel: 'All customers',
      panelID: 'all-customers-content-1',
    },
    {
      id: 'accepts-marketing-1',
      content: 'Accepts marketing',
      panelID: 'accepts-marketing-content-1',
    },
    {
      id: 'repeat-customers-1',
      content: 'Repeat customers',
      panelID: 'repeat-customers-content-1',
    },
    {
      id: 'prospects-1',
      content: 'Prospects',
      panelID: 'prospects-content-1',
    },
  ];

  return (
    <Card>
      <Tabs tabs={tabs} selected={selected} onSelect={handleTabChange}>
        <Card.Section title={tabs[selected].content}>
          <p>Tab {selected} selected</p>
        </Card.Section>
      </Tabs>
    </Card>
  );
}

🎩 checklist

  • Tested on mobile
  • Tested on multiple browsers
  • Tested for accessibility
  • Updated the component's README.md with documentation changes
  • Tophatted documentation changes in the style guide
  • For visual design changes, pinged one of @ HYPD, @ mirualves, @ sarahill, or @ ry5n to update the Polaris UI kit

@github-actions
Copy link
Contributor

github-actions bot commented Feb 5, 2021

🟢 This pull request modifies 3 files and might impact 1 other files.

Details:
All files potentially affected (total: 1)
📄 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

🧩 src/components/Tabs/components/TabMeasurer/TabMeasurer.tsx (total: 1)

Files potentially affected (total: 1)

🧩 src/components/Tabs/utilities.ts (total: 1)

Files potentially affected (total: 1)

@lhoffbeck lhoffbeck changed the title [Tabs] Tabs wrap on window resize (#3976) [Tabs] Tabs wrap on window resize Feb 11, 2021
Copy link
Contributor

@kyledurand kyledurand left a comment

Choose a reason for hiding this comment

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

Code looks good to me. Thanks for looking into this 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Tabs] Content wraps when tabs are non-integer widths
2 participants