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.

Sign up for GitHub

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

Jump to bottom

feat(Tab): make tab functional component #9722

Merged
merged 27 commits into from
Oct 19, 2021
Merged

feat(Tab): make tab functional component #9722

merged 27 commits into from
Oct 19, 2021

Conversation

jnm2377
Copy link
Contributor

@jnm2377 jnm2377 commented Sep 22, 2021 •
edited
Loading

Ref #9712

Changelog

  • refactors Tab to be a functional component
  • adds new tests

Removed

Testing / Reviewing

  1. Pull PR down locally
  2. in packages/react run CARBON_ENABLE_V11_RELEASE=true yarn storybook to use the v11/next component
  3. test out the Tabs story:
  • ensure styles are working correctly
  • ensure click selection is working
  • ensure keyboard navigation (arrow keys to nav between tabs) is working
  • ensure tabs are responsive (scrollable)
  • ensure scrollable tabs update L/R overflow buttons accordingly
    • test scrolling via mouse/trackpad
    • test scrolling via overflow button click
  • ensure tests are passing

To-do:

  • update styles to work for non --scrollable classNames in v11
  • add new RTL tests for Tab
  • add new RTL tests for Tabs

@netlify
Copy link

netlify bot commented Sep 22, 2021 •
edited
Loading

✔️ Deploy Preview for carbon-react-next ready!

🔨 Explore the source changes: 009724f

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/616f0d9acb8b510008d830a8

😎 Browse the preview: https://deploy-preview-9722--carbon-react-next.netlify.app

@netlify
Copy link

netlify bot commented Sep 22, 2021 •
edited
Loading

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: 009724f

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/616f0d9a6b1b72000841ac6e

😎 Browse the preview: https://deploy-preview-9722--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Sep 22, 2021 •
edited
Loading

✔️ Deploy Preview for carbon-components-react ready!
Built without sensitive environment variables

🔨 Explore the source changes: 009724f

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/616f0d9a1f01950007d10a4a

😎 Browse the preview: https://deploy-preview-9722--carbon-components-react.netlify.app

@jnm2377 jnm2377 marked this pull request as ready for review October 6, 2021 06:56
@jnm2377 jnm2377 requested a review from a team as a code owner October 6, 2021 06:56
dakahn
dakahn approved these changes Oct 13, 2021
View reviewed changes
Copy link
Contributor

@dakahn dakahn left a comment

Choose a reason for hiding this comment

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

Nice job!

tay1orjones
tay1orjones reviewed Oct 13, 2021
View reviewed changes
Copy link
Member

@tay1orjones tay1orjones left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hefty refactor! Looks awesome 🔥 I pulled up the existing storybook and started up the local storybook with the flag and went through the same interactions between them both with keyboard and mouse, small screens w/ scrolling, etc and it appears to be functionally the same without regressions.

Just a couple minor comments and questions below. Otherwise looks good to merge!

packages/react/src/components/Tab/next/Tab-test.js Outdated Show resolved Hide resolved
packages/react/src/components/Tab/next/Tab.js Outdated Show resolved Hide resolved
packages/react/src/components/Tab/next/Tab.js Outdated Show resolved Hide resolved
packages/react/src/components/Tab/next/Tab.js Outdated Show resolved Hide resolved
packages/react/src/components/Tabs/index.js Outdated Show resolved Hide resolved
packages/react/src/components/Tabs/next/Tabs-test.js Show resolved Hide resolved
packages/react/src/components/Tabs/next/Tabs.js Outdated Show resolved Hide resolved
@jnm2377 jnm2377 requested a review from a team as a code owner October 18, 2021 21:10
@kodiakhq kodiakhq bot merged commit 7276911 into carbon-design-system:main Oct 19, 2021
@jnm2377 jnm2377 mentioned this pull request Dec 14, 2021
Closed
kennylam pushed a commit to kennylam/carbon that referenced this pull request Jul 30, 2024
@annawen1
chore(cwc): update references in storybook and codesandbox examples (c…
695b4e3 
…arbon-design-system#9722)

### Related Ticket(s)

{{Provide url(s) to the related ticket(s) that this pull request addresses}}

### Description

Updating the carbon-web-components references in the carbon-web-components' storybook docs and codesandbox examples.

### Changelog

**Changed**

1. `carbon-web-components/es` ---> `@carbon/carbon-web-components/es`
2. CDN urls
   `https://1.www.s81c.com/common/carbon/web-components/...` --> `https://1.www.s81c.com/common/carbon-for-ibm-dotcom/carbon-web-components/...`
3. Codesandbox links in the storybook docs
  `https://codesandbox.io/s/github/carbon-design-system/carbon-web-components/tree/main/examples/codesandbox/basic/components/...` --> `https://codesandbox.io/s/github/carbon-design-system/carbon-for-ibm-dotcom/tree/main/packages/carbon-web-components/examples/codesandbox/basic/components/...`


<!-- React and Web Component deploy previews are enabled by default. -->
<!-- To enable additional available deploy previews, apply the following -->
<!-- labels for the corresponding package: -->
<!-- *** "test: e2e": Codesandbox examples and e2e integration tests -->
<!-- *** "package: services": Services -->
<!-- *** "package: utilities": Utilities -->
<!-- *** "RTL": React / Web Components (RTL) -->
<!-- *** "feature flag": React / Web Components (experimental) -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dakahn dakahn dakahn approved these changes

@tay1orjones tay1orjones tay1orjones approved these changes

@aledavila aledavila Awaiting requested review from aledavila aledavila was automatically assigned from carbon-design-system/developers-system

@andreancardona andreancardona Awaiting requested review from andreancardona andreancardona was automatically assigned from carbon-design-system/developers-system

Assignees
No one assigned
Labels
status: ready to merge 🎉
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jnm2377 @tay1orjones @dakahn