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

refactor(tabs, tab, tab-nav, tab-title): getElementProp is refactored out in favor of inheritable props set directly on parent #7605

Merged
merged 60 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
cefb472
refactor(tabs, tab, tab-nav, tab-title): getElementProp is refactored…
Elijbet Aug 24, 2023
9f05d24
include tab scale
Elijbet Aug 25, 2023
7f5a322
Merge branch 'main' into elijbet/6038-refactor-getElementProp-tab-title
Elijbet Aug 30, 2023
3f704d3
Merge branch 'main' into elijbet/6038-refactor-getElementProp-tab-title
Elijbet Sep 6, 2023
44db41f
Merge branch 'main' into elijbet/6038-refactor-getElementProp-tab-title
Elijbet Sep 7, 2023
27c8c25
update tests per new changes
Elijbet Sep 7, 2023
b322908
adjust tab-nav tests to asserting on scales being inherited and not r…
Elijbet Sep 8, 2023
4b572a4
make testing scale inheritance down the ancestry tree dry
Elijbet Sep 8, 2023
c106a94
Merge branch 'main' into elijbet/6038-refactor-getElementProp-tab-title
Elijbet Sep 8, 2023
c64c5fc
Merge branch 'main' into elijbet/6038-refactor-getElementProp-tab-title
Elijbet Sep 11, 2023
c5fa9bf
update tabs test per new requirements: attribute is no longer reflect…
Elijbet Sep 11, 2023
85c5b5d
cleanup
Elijbet Sep 11, 2023
88df173
cleanup
Elijbet Sep 11, 2023
4fba6d2
clean loop with absent value
Elijbet Sep 11, 2023
6fd166b
Merge branch 'main' into elijbet/6038-refactor-getElementProp-tab-title
Elijbet Sep 12, 2023
41fefb6
refactor selectors out into resources, other refactors
Elijbet Sep 12, 2023
07d83bf
Refactor tab-nav scale test into one compact it block
Elijbet Sep 12, 2023
c17919e
add a class selector and refactor test loop
Elijbet Sep 14, 2023
e9a4969
Merge branch 'main' into elijbet/6038-refactor-getElementProp-tab-title
Elijbet Sep 14, 2023
c1500e8
Merge branch 'main' into elijbet/6038-refactor-getElementProp-tab-title
Elijbet Oct 12, 2023
25529f7
refactor use of attribute scale as selector and use a dynamically add…
Elijbet Oct 13, 2023
5d9254e
Merge branch 'main' into elijbet/6038-refactor-getElementProp-tab-title
Elijbet Oct 13, 2023
e115f45
cleanup tabs
Elijbet Oct 13, 2023
cf561c7
cleanup
Elijbet Oct 13, 2023
336ea70
use a css selector instead of an attribute selector in the tabs for p…
Elijbet Oct 13, 2023
342851e
cleanup old test setup
Elijbet Oct 13, 2023
d939191
Merge branch 'main' into elijbet/6038-refactor-getElementProp-tab-title
Elijbet Oct 13, 2023
c86e482
cleanup
Elijbet Oct 13, 2023
eac7b60
combine and simplify to make tests DRY
Elijbet Oct 13, 2023
9bef438
simplify tab-title tests
Elijbet Oct 13, 2023
edebe99
revert some refactors
Elijbet Oct 14, 2023
b9fd2c7
Merge branch 'main' into elijbet/6038-refactor-getElementProp-tab-title
Elijbet Oct 14, 2023
60a473f
Merge branch 'main' into elijbet/6038-refactor-getElementProp-tab-title
Elijbet Oct 17, 2023
2a1c695
refactor invalid test and cleanup docs
Elijbet Oct 23, 2023
e170cd5
Merge branch 'main' into elijbet/6038-refactor-getElementProp-tab-title
Elijbet Oct 23, 2023
b4bfbdf
cleanup
Elijbet Oct 24, 2023
9a96bcd
allocate resources per component
Elijbet Oct 24, 2023
d5af714
use for...x loop since forEach is not async.
Elijbet Oct 24, 2023
55160f8
consolidate duplication in tests
Elijbet Oct 24, 2023
5cbc0e0
Merge branch 'main' into elijbet/6038-refactor-getElementProp-tab-title
Elijbet Oct 24, 2023
948907a
update the mutation observer callback to bail if the mutation target …
Elijbet Oct 24, 2023
e29a715
query for a single tab-nav
Elijbet Oct 24, 2023
4084fec
update queries to ignore nested tabs
Elijbet Oct 24, 2023
596a7b0
Merge branch 'main' into elijbet/6038-refactor-getElementProp-tab-title
Elijbet Oct 26, 2023
ba66370
refactor out redundancies in the tests and resources
Elijbet Oct 26, 2023
00d97cc
doc editing
Elijbet Oct 26, 2023
bb8aa60
adjust flex on center and inline with the new selectors
Elijbet Oct 26, 2023
4293d25
🎲
Elijbet Oct 26, 2023
404cae5
drop any test checking visual changes, clean up duplication in css, a…
Elijbet Oct 26, 2023
37b877c
Merge branch 'main' into elijbet/6038-refactor-getElementProp-tab-title
Elijbet Oct 27, 2023
d4e508e
add screenshot coverage for simle center/inline/border scale variations
Elijbet Oct 27, 2023
c1a9c9d
cleanup
Elijbet Oct 27, 2023
d2b10fc
make these styles more robust by using a class vs an element
Elijbet Oct 28, 2023
9288052
set of screenshot tests to cover various scenarios for scales
Elijbet Oct 28, 2023
468787e
use a class method vs an assigned-bound function
Elijbet Oct 28, 2023
6aa6c87
rework margin application based on new scale class selectors vs attri…
Elijbet Oct 29, 2023
52ff51c
fix to missing inline tab gap and make sure tabs doesn't push it's in…
Elijbet Oct 30, 2023
ff3defb
fix omission
Elijbet Oct 30, 2023
3184404
Merge branch 'main' into elijbet/6038-refactor-getElementProp-tab-title
Elijbet Oct 30, 2023
f065e26
Merge branch 'main' into elijbet/6038-refactor-getElementProp-tab-title
Elijbet Oct 30, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions packages/calcite-components/src/components/tab-nav/tab-nav.scss
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,36 @@
min-block-size: theme("spacing.11");
}

// `tab-nav` in all scales in layout="center" has a padding of 20px on both ends
:host([layout="center"]:not([bordered])) {
padding-inline: theme("margin.5");

.scale-l {
::slotted(calcite-tab-title) {
margin-inline-end: theme("margin.6");
}
}
.scale-m {
::slotted(calcite-tab-title) {
margin-inline-end: theme("margin.5");
}
}
.scale-s {
::slotted(calcite-tab-title) {
margin-inline-end: theme("margin.4");
}
}
}

//override margin-inline-end for the last child for tab-nav to implement the 20px padding on both ends instead
:host([layout="center"]:not([bordered])) {
.tab-nav {
::slotted(calcite-tab-title:last-child) {
margin-inline-end: theme("margin.0");
}
}
}

.tab-nav {
@apply flex
w-full
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
:host {
@apply block outline-none;
margin-inline-start: theme("margin.0");
margin-inline-end: theme("margin.5");
}

:host([layout="inline"]) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export const CSS = {
container: "container",
content: "content",
};
16 changes: 8 additions & 8 deletions packages/calcite-components/src/components/tab/tab.scss
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,21 @@
@apply block h-full w-full overflow-auto;
}

section,
.container {
@apply hidden h-full w-full;
}

.scale-s section {
.scale-s .content {
@apply text-n2h py-1;
}

.scale-m section {
.scale-m .content {
@apply text-n1h py-2;
}

.scale-l section {
.scale-l .content {
@apply text-0h py-2.5;
}

section,
.container {
@apply hidden h-full w-full;
}

@include base-component();
2 changes: 1 addition & 1 deletion packages/calcite-components/src/components/tab/tab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export class Tab {
role="tabpanel"
tabIndex={this.selected ? 0 : -1}
>
<section>
<section class={CSS.content}>
Copy link
Member

Choose a reason for hiding this comment

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

Not for this PR, but it'd be good to replace all selectors using section with the content class.

<slot />
</section>
</div>
Expand Down
88 changes: 83 additions & 5 deletions packages/calcite-components/src/components/tabs/tabs.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,16 +163,94 @@ export const setWidth = (): string => html`
</div>
`;

export const justTabNav = (): string => html`
<calcite-tab-nav
position="${select("position", ["top", "bottom"], "top")}"
scale="${select("scale", ["s", "m", "l"], "l")}"
>
const TabNavHTMLSimple = html`
Copy link
Member

Choose a reason for hiding this comment

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

Awesome job adding more coverage! ✨🧪🧪🧪✨

<calcite-tab-nav slot="title-group">
<calcite-tab-title>Tab 1 Title</calcite-tab-title>
<calcite-tab-title>Tab 2 Title</calcite-tab-title>
<calcite-tab-title>Tab 3 Title</calcite-tab-title>
<calcite-tab-title selected>Tab 4 Title</calcite-tab-title>
</calcite-tab-nav>
<calcite-tab>Tab 1 Content</calcite-tab>
<calcite-tab>Tab 2 Content</calcite-tab>
<calcite-tab>Tab 3 Content</calcite-tab>
<calcite-tab selected>Tab 4 Content</calcite-tab>
`;

const TabNavHTMLVariedTabWidth = html`
<calcite-tab-nav slot="title-group">
<calcite-tab-title icon-start="arrow-left">Tab 1 Title</calcite-tab-title>
<calcite-tab-title icon-end="arrow-right">Tab 2 Title</calcite-tab-title>
<calcite-tab-title icon-start="arrow-left" icon-end="arrow-right">Tab 3 Title</calcite-tab-title>
<calcite-tab-title closable selected>Tab 4 Title</calcite-tab-title>
</calcite-tab-nav>
<calcite-tab>Tab 1 Content</calcite-tab>
<calcite-tab>Tab 2 Content</calcite-tab>
<calcite-tab>Tab 3 Content</calcite-tab>
<calcite-tab selected>Tab 4 Content</calcite-tab>
`;

const tabStyles = html`
<style>
calcite-tabs {
margin: 20px;
}
</style>
`;

export const centerScale_TestOnly = (): string => html`
${tabStyles}
<calcite-tabs layout="center" scale="s">${TabNavHTMLSimple}</calcite-tabs>
<calcite-tabs layout="center" scale="m">${TabNavHTMLSimple}</calcite-tabs>
<calcite-tabs layout="center" scale="l">${TabNavHTMLSimple}</calcite-tabs>
`;

export const centerVariedTabWidthScale_TestOnly = (): string => html`
${tabStyles}
<calcite-tabs layout="center" scale="s">${TabNavHTMLVariedTabWidth}</calcite-tabs>
<calcite-tabs layout="center" scale="m">${TabNavHTMLVariedTabWidth}</calcite-tabs>
<calcite-tabs layout="center" scale="l">${TabNavHTMLVariedTabWidth}</calcite-tabs>
`;

export const centerBorderedScale_TestOnly = (): string => html`
${tabStyles}
<calcite-tabs layout="center" scale="s" bordered>${TabNavHTMLSimple}</calcite-tabs>
<calcite-tabs layout="center" scale="m" bordered>${TabNavHTMLSimple}</calcite-tabs>
<calcite-tabs layout="center" scale="l" bordered>${TabNavHTMLSimple}</calcite-tabs>
`;

export const centerBorderedVariedTabWidthScale_TestOnly = (): string => html`
${tabStyles}
<calcite-tabs layout="center" scale="s" bordered>${TabNavHTMLVariedTabWidth}</calcite-tabs>
<calcite-tabs layout="center" scale="m" bordered>${TabNavHTMLVariedTabWidth}</calcite-tabs>
<calcite-tabs layout="center" scale="l" bordered>${TabNavHTMLVariedTabWidth}</calcite-tabs>
`;

export const inlineScale_TestOnly = (): string => html`
${tabStyles}
<calcite-tabs layout="inline" scale="s">${TabNavHTMLSimple}</calcite-tabs>
<calcite-tabs layout="inline" scale="m">${TabNavHTMLSimple}</calcite-tabs>
<calcite-tabs layout="inline" scale="l">${TabNavHTMLSimple}</calcite-tabs>
`;

export const inlineVariedTabWidthScale_TestOnly = (): string => html`
${tabStyles}
<calcite-tabs layout="inline" scale="s">${TabNavHTMLVariedTabWidth}</calcite-tabs>
<calcite-tabs layout="inline" scale="m">${TabNavHTMLVariedTabWidth}</calcite-tabs>
<calcite-tabs layout="inline" scale="l">${TabNavHTMLVariedTabWidth}</calcite-tabs>
`;

export const inlineBorderedScale_TestOnly = (): string => html`
${tabStyles}
<calcite-tabs layout="inline" scale="s" bordered>${TabNavHTMLSimple}</calcite-tabs>
<calcite-tabs layout="inline" scale="m" bordered>${TabNavHTMLSimple}</calcite-tabs>
<calcite-tabs layout="inline" scale="l" bordered>${TabNavHTMLSimple}</calcite-tabs>
`;

export const inlineBorderedVariedTabWidthScale_TestOnly = (): string => html`
${tabStyles}
<calcite-tabs layout="inline" scale="s" bordered>${TabNavHTMLVariedTabWidth}</calcite-tabs>
<calcite-tabs layout="inline" scale="m" bordered>${TabNavHTMLVariedTabWidth}</calcite-tabs>
<calcite-tabs layout="inline" scale="l" bordered>${TabNavHTMLVariedTabWidth}</calcite-tabs>
`;

export const disabledTabsAndMediumIconsForLargeTabsTitle_TestOnly = (): string => html`
Expand Down
9 changes: 4 additions & 5 deletions packages/calcite-components/src/components/tabs/tabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ export class Tabs {
}
});

private updateItems = (): void => {
private updateItems(): void {
Copy link
Member

Choose a reason for hiding this comment

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

✨🏆✨

const { position, scale } = this;

const nav = this.el.querySelector("calcite-tab-nav");
Expand All @@ -175,18 +175,17 @@ export class Tabs {
nav.scale = scale;
}

const tab = this.el.querySelector("calcite-tab");
if (tab) {
Array.from(this.el.querySelectorAll("calcite-tab")).forEach((tab: HTMLCalciteTabElement) => {
Copy link
Member

Choose a reason for hiding this comment

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

I think you'll need to update queries to ignore nested tabs.

Copy link
Member

Choose a reason for hiding this comment

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

Queries target all descendants, so this will be an issue when you have nested tabs with different props each. Not sure how common position and scale being different between nested tabs would be, so this could be an edge case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interim fix, I'll think of a way to have the selector do this for us instead.

tab.scale = scale;
}
});

Array.from(this.el.querySelectorAll("calcite-tab-nav > calcite-tab-title")).forEach(
(title: HTMLCalciteTabTitleElement) => {
title.position = position;
title.scale = scale;
}
);
};
}

//--------------------------------------------------------------------------
//
Expand Down