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(tabs): add headerWidth prop (FE-4161) #4202

Merged
merged 3 commits into from
Jul 6, 2021
Merged

feat(tabs): add headerWidth prop (FE-4161) #4202

merged 3 commits into from
Jul 6, 2021

Conversation

dawidzarzycki
Copy link
Contributor

@dawidzarzycki dawidzarzycki commented Jun 28, 2021 •
edited
Loading

headerWidth prop will set a width to TabsHeader component.
It will allow users with very long title to make well aligned Tabs
if there will be more of tabs group than one

fixes: #4024

Proposed behaviour

image

add headerWidth prop

Current behaviour

lack of support for custom width

Checklist

  • Commits follow our style guide
  • Screenshots are included in the PR if useful
  • All themes are supported if required
  • Unit tests added or updated if required
  • Cypress automation tests added or updated if required
  • Storybook added or updated if required
  • Typescript d.ts file added or updated if required
  • Carbon implementation and Design System documentation are congruent

Testing instructions

New storybook story created with tabsHeaderWidth

@dawidzarzycki dawidzarzycki self-assigned this Jun 28, 2021
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 28, 2021 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit bfd85df:

Sandbox Source
carbon-quickstart Configuration
trusting-hooks-zw1hh Issue #4024

@cypress
Copy link

cypress bot commented Jun 28, 2021 •
edited
Loading


Test summary

1236 • 0 • 2 • 0 • Flakiness 0

Run details
Project carbon
Status Passed
Commit 86e5c04919 ℹ️
Started Jul 6, 2021 7:12 AM
Ended Jul 6, 2021 7:18 AM
Duration 05:58 💡
OS Linux Debian - 10.8
Browser Chrome 90

View run in Cypress Dashboard ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@dawidzarzycki dawidzarzycki force-pushed the FE-4161-tabs-changing-width branch from 7d1dda9 to 7c1c7c1 Compare June 28, 2021 12:17
samtjo
samtjo suggested changes Jun 28, 2021
View reviewed changes
src/components/tabs/__internal__/tabs-header/tabs-header.component.js Outdated Show resolved Hide resolved
src/components/tabs/tabs.component.js Outdated Show resolved Hide resolved
src/components/tabs/tabs.component.js Outdated Show resolved Hide resolved
src/components/tabs/tabs.d.ts Outdated Show resolved Hide resolved
src/components/tabs/tabs.stories.mdx Outdated Show resolved Hide resolved
DipperTheDan
DipperTheDan reviewed Jun 28, 2021
View reviewed changes
Copy link
Contributor

@DipperTheDan DipperTheDan 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.

I had pretty much the same comments as @sjohnson-sage so once you address the requested changes, I'll be happy to give the PR a tick ✅

DipperTheDan
DipperTheDan previously approved these changes Jun 28, 2021
View reviewed changes
samtjo
samtjo previously approved these changes Jun 28, 2021
View reviewed changes
@dawidzarzycki dawidzarzycki removed their assignment Jun 28, 2021
@dawidzarzycki dawidzarzycki changed the title feat(tabs): add tabsHeaderWidth prop feat(tabs): add headerWidth prop Jun 28, 2021
@dawidzarzycki dawidzarzycki dismissed stale reviews from samtjo and DipperTheDan via f9b0195 June 28, 2021 14:24
@dawidzarzycki dawidzarzycki force-pushed the FE-4161-tabs-changing-width branch from 93bcbe6 to f9b0195 Compare June 28, 2021 14:24
@dawidzarzycki dawidzarzycki marked this pull request as ready for review June 28, 2021 16:43
@dawidzarzycki dawidzarzycki requested review from a team as code owners June 28, 2021 16:43
@harpalsingh
Copy link
Member

Is this an issue with the width prop or just storybook?

tabs.mov

@dawidzarzycki
Copy link
Contributor Author

Is this an issue with the width prop or just storybook?

tabs.mov

I'm pretty sure it is a bug, I will fix it

@dawidzarzycki dawidzarzycki marked this pull request as draft June 29, 2021 13:54
DipperTheDan
DipperTheDan reviewed Jun 29, 2021
View reviewed changes
src/components/tabs/tabs.component.js
@@ -356,6 +358,10 @@ Tabs.propTypes = {
"no right side",
"no sides",
]),
/** sets width to the tab headers. Can be any valid CSS string.
* The headerWidth prop works only for `position="left"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth maybe having a custom prop validation here now that this prop works only when the position="left"

DipperTheDan
DipperTheDan previously approved these changes Jun 30, 2021
View reviewed changes
@nicktitchmarsh nicktitchmarsh changed the title feat(tabs): add headerWidth prop feat(tabs): add headerWidth prop (FE-4161) Jul 1, 2021
nicktitchmarsh
nicktitchmarsh previously approved these changes Jul 1, 2021
View reviewed changes
@dawidzarzycki dawidzarzycki dismissed stale reviews from nicktitchmarsh and DipperTheDan via f990ae9 July 1, 2021 12:58
@dawidzarzycki dawidzarzycki force-pushed the FE-4161-tabs-changing-width branch 3 times, most recently from 3f715dd to f48b03f Compare July 1, 2021 13:03
feat(tabs): add tabsHeaderWidth prop
b76b8a7 
`tabsHeaderWidth` prop will set a width to `TabsHeader` component.
It will allow users with very long title to make well aligned `Tabs`
if there will be more of tabs group than one

fixes: #4024
@dawidzarzycki dawidzarzycki force-pushed the FE-4161-tabs-changing-width branch from f48b03f to b76b8a7 Compare July 1, 2021 13:08
@dawidzarzycki dawidzarzycki marked this pull request as ready for review July 1, 2021 14:24
@dawidzarzycki dawidzarzycki merged commit 4626892 into master Jul 6, 2021
@dawidzarzycki dawidzarzycki deleted the FE-4161-tabs-changing-width branch July 6, 2021 07:35
@carbonci
Copy link
Collaborator

carbonci commented Jul 6, 2021

🎉 This PR is included in version 77.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicktitchmarsh nicktitchmarsh nicktitchmarsh approved these changes

@DipperTheDan DipperTheDan DipperTheDan approved these changes

@DlgSHi DlgSHi DlgSHi approved these changes

@samtjo samtjo samtjo left review comments

Assignees

@dawidzarzycki dawidzarzycki

Labels
Passed QA Passed Review Passed UX QA released Squad Front End
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Tabs on left are changing width when content change
7 participants
@dawidzarzycki @harpalsingh @carbonci @samtjo @DlgSHi @DipperTheDan @nicktitchmarsh