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 (#3976) #3978

Closed
wants to merge 4 commits into from
Closed

[Tabs] Tabs wrap on window resize (#3976) #3978

wants to merge 4 commits into from

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

@ghost
Copy link

ghost commented Feb 4, 2021

👋 Thanks for opening your first pull request. A contributor should give feedback soon. If you haven’t already, please check out the contributing guidelines.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 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)

@kyledurand
Copy link
Member

Looks like you cloned the repo and opened this in your personal GH. Can you open this PR against Shopify/polaris-react directly? It saves us time in tophatting and won't hang on some of the CI checks

@lhoffbeck
Copy link
Contributor Author

@kyledurand Ahh I was following the advice of that linked egghead tutorial from the contributions page, I'll close this out and reopen against Shopify/polaris-react in a sec. Thanks!

@lhoffbeck lhoffbeck closed this Feb 5, 2021
@lhoffbeck
Copy link
Contributor Author

Reopened against shopifyShopify/polaris-react here: #3980

@lhoffbeck lhoffbeck deleted the lh-tabs-wrap-on-window-resize branch February 5, 2021 18:29
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