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

fix(tab-nav): justify centered tab-titles evenly to extend across the width of tab-nav #7047

Merged
merged 7 commits into from
May 27, 2023
Merged

fix(tab-nav): justify centered tab-titles evenly to extend across the width of tab-nav #7047

merged 7 commits into from
May 27, 2023

Conversation

Elijbet
Copy link
Contributor

@Elijbet Elijbet commented May 26, 2023 •
edited
Loading

Related Issue: #7025

Summary

Follow-up to fix to #7026.

Centered tabs should spread the tab-titles evenly to extend across the entire width of tab-nav, as opposed to inline tab-titles that only justify against the start of tab-nav width.

Added coverage for both centeredTabsAreEvenlyJustifiedAcrossNavWidth and inlineTabsJustifyAgainstTheStartOfTheNavWidth.

@github-actions github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label May 26, 2023
@Elijbet Elijbet changed the title fix(tab-nav): justify centered tabs evenly to extend across the width of nav fix(tab-nav): justify centered tab-titles evenly to extend across the width of tab-nav May 26, 2023
@Elijbet Elijbet marked this pull request as ready for review May 26, 2023 21:02
@Elijbet Elijbet requested a review from a team as a code owner May 26, 2023 21:02
@Elijbet Elijbet added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label May 26, 2023
driskull
driskull approved these changes May 26, 2023
View reviewed changes
Copy link
Member

@driskull driskull left a comment

Choose a reason for hiding this comment

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

👍

Will this trigger a screenshot failure? If not can we add a test for it?

@Elijbet
Copy link
Contributor Author

Elijbet commented May 26, 2023 •
edited
Loading

👍

Will this trigger a screenshot failure? If not can we add a test for it?

Yeah, I think because I had too many tabs in the screenshot setup to cover all prop variations, it wasn't obvious tabs fail to spread to the whole width. That's how it got missed. I'll modify one now.

driskull
driskull reviewed May 26, 2023
View reviewed changes
src/components/tabs/tabs.stories.ts
@@ -233,6 +232,21 @@ export const centeredBorderedClosable_TestOnly = (): string => html`
</calcite-tabs>
`;

export const centeredTabsAreEvenlyJustifiedAcrossNavWidth_TestOnly = (): string => html`
Copy link
Member

Choose a reason for hiding this comment

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

👍

@Elijbet Elijbet added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels May 26, 2023
@Elijbet Elijbet added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels May 26, 2023
@Elijbet Elijbet added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels May 26, 2023
@Elijbet Elijbet added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels May 26, 2023
@Elijbet Elijbet merged commit 8ef0421 into master May 27, 2023
@Elijbet Elijbet deleted the elijbet/7025-follow-up-tab-centering-to-span-nav branch May 27, 2023 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@driskull driskull driskull approved these changes

Assignees

@Elijbet Elijbet

Labels
bug Bug reports for broken functionality. Issues should include a reproduction of the bug. pr ready for visual snapshots Adding this label will run visual snapshot testing.
Projects
None yet
Milestone
1.4.1 May patch - Bugs only
Development

Successfully merging this pull request may close these issues.
3 participants
@Elijbet @driskull @geospatialem