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 32 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
14 changes: 8 additions & 6 deletions packages/calcite-components/src/components/tab-nav/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,16 @@ The tab-nav groups several [calcite-tab-title](../tab-title) components and buil

### Basic

When tab-nav is the only parent, tab-title can inherit its `scale` and `position` from tab-nav:
Tab-nav and tab-title inherit their `scale` and `position` from tab-title parent:
Copy link
Member

Choose a reason for hiding this comment

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

Sidebar: If standalone tab-nav usage wasn't supported 1.0.3, let's see if we can remove this and any other doc that might indicate otherwise. cc @geospatialem


```html
<calcite-tab-nav scale="l" position="bottom">
<calcite-tab-title>Layers</calcite-tab-title>
<calcite-tab-title>Maps</calcite-tab-title>
<calcite-tab-title selected>Data</calcite-tab-title>
</calcite-tab-nav>
<calcite-tabs scale="l" position="bottom">
<calcite-tab-nav>
<calcite-tab-title>Layers</calcite-tab-title>
<calcite-tab-title>Maps</calcite-tab-title>
<calcite-tab-title selected>Data</calcite-tab-title>
</calcite-tab-nav>
</calcite-tabs>
```

## Properties
Expand Down
104 changes: 19 additions & 85 deletions packages/calcite-components/src/components/tab-nav/tab-nav.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import { newE2EPage } from "@stencil/core/testing";
import { accessible, renders, hidden } from "../../tests/commonTests";
import { accessible, defaults, renders, hidden } from "../../tests/commonTests";
import { html } from "../../../support/formatting";

describe("calcite-tab-nav", () => {
const tabNavHtml = "<calcite-tab-nav></calcite-tab-nav>";

describe("defaults (immediate parent tabs or tab-nav)", () => {
defaults("calcite-tabs", [{ propertyName: "scale", defaultValue: "m" }]);
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
defaults(`<calcite-tabs>${tabNavHtml}</calcite-tabs>`, [{ propertyName: "scale", defaultValue: "m" }]);
});

describe("renders", () => {
renders(tabNavHtml, { display: "flex" });
});
Expand Down Expand Up @@ -78,94 +83,23 @@ describe("calcite-tab-nav", () => {
});
});

describe("scale property", () => {
describe("default", () => {
it("should render without scale", async () => {
const page = await newE2EPage({
html: `${tabNavHtml}`,
});
const element = await page.find("calcite-tab-nav");
expect(element).not.toHaveAttribute("scale");
});
});
describe("scales", () => {
const scaleMinHeightPairs = {
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
s: "24px",
m: "32px",
l: "44px",
};

describe("when scale is small", () => {
it("should render with small scale", async () => {
const page = await newE2EPage({
html: `<calcite-tab-nav scale='s'>
<calcite-tab-title selected>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>Tab 4 Title</calcite-tab-title>
</calcite-tab-nav>`,
});
const element = await page.find("calcite-tab-nav");
expect(await (await element.getComputedStyle())["minHeight"]).toEqual("24px");
expect(element).toEqualAttribute("scale", "s");
});
});
for (const [key, value] of Object.entries(scaleMinHeightPairs)) {
it(`${key} scale`, async () => {
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
const page = await newE2EPage();
await page.setContent(html`<calcite-tabs scale="${key}">${tabNavHtml}</calcite-tabs>`);

describe("when scale is medium", () => {
it("should render with medium scale", async () => {
const page = await newE2EPage({
html: `<calcite-tab-nav scale='m'>
<calcite-tab-title selected>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>Tab 4 Title</calcite-tab-title>
</calcite-tab-nav>`,
});
const element = await page.find("calcite-tab-nav");
expect(await (await element.getComputedStyle())["minHeight"]).toEqual("32px");
expect(element).toEqualAttribute("scale", "m");
expect((await element.getComputedStyle())["height"]).toEqual(value);
expect(await element.getProperty("scale")).toBe(`${key}`);
});
});

describe("when scale is large", () => {
it("should render with medium scale", async () => {
const page = await newE2EPage({
html: `<calcite-tab-nav scale='l'>
<calcite-tab-title selected>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>Tab 4 Title</calcite-tab-title>
</calcite-tab-nav>`,
});
const element = await page.find("calcite-tab-nav");
expect(await (await element.getComputedStyle())["minHeight"]).toEqual("44px");
expect(element).toEqualAttribute("scale", "l");
});
});

describe("when nested within tabs parent", () => {
it("should render with default medium scale", async () => {
const page = await newE2EPage({
html: `<calcite-tabs>${tabNavHtml}</calcite-tabs>`,
});
const element = await page.find("calcite-tab-nav");
expect(element).toEqualAttribute("scale", "m");
});

describe("when tabs scale is small", () => {
it("should render with small scale", async () => {
const page = await newE2EPage({
html: `<calcite-tabs scale="s">${tabNavHtml}</calcite-tabs>`,
});
const element = await page.find("calcite-tab-nav");
expect(element).toEqualAttribute("scale", "s");
});
});

describe("when tabs scale is large", () => {
it("should render with large scale", async () => {
const page = await newE2EPage({
html: `<calcite-tabs scale="l">${tabNavHtml}</calcite-tabs>`,
});
const element = await page.find("calcite-tab-nav");
expect(element).toEqualAttribute("scale", "l");
});
});
});
}
});

it("focuses on keyboard interaction", async () => {
Expand Down
12 changes: 6 additions & 6 deletions packages/calcite-components/src/components/tab-nav/tab-nav.scss
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
@apply relative flex;
}

:host([scale="s"]) {
.scale-s {
min-block-size: theme("spacing.6");
}

:host([scale="m"]) {
.scale-m {
min-block-size: theme("spacing.8");
}

:host([scale="l"]) {
.scale-l {
min-block-size: theme("spacing.11");
}

Expand Down Expand Up @@ -45,12 +45,12 @@
@apply justify-evenly;
}

:host([position="bottom"]) .tab-nav-active-indicator {
:host .position-bottom .tab-nav-active-indicator {
inset-block-end: unset;
@apply top-0;
}

:host([position="bottom"]) .tab-nav-active-indicator-container {
:host .position-bottom .tab-nav-active-indicator-container {
inset-block-end: unset;
@apply top-0;
}
Expand All @@ -59,7 +59,7 @@
inset-block-end: unset; // display active blue line above instead of below
}

:host([bordered][position="bottom"]) .tab-nav-active-indicator-container {
:host([bordered]) .position-bottom .tab-nav-active-indicator-container {
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
inset-block-end: 0; // display active blue line below instead of above
inset-block-start: unset;
}
Expand Down
27 changes: 17 additions & 10 deletions packages/calcite-components/src/components/tab-nav/tab-nav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { createObserver } from "../../utils/observers";
import { Scale } from "../interfaces";
import { TabChangeEventDetail, TabCloseEventDetail } from "../tab/interfaces";
import { TabID, TabLayout, TabPosition } from "../tabs/interfaces";
import { CSS } from "../tabs/resources";

/**
* @slot - A slot for adding `calcite-tab-title`s.
Expand Down Expand Up @@ -55,19 +56,23 @@ export class TabNav {
@Prop({ mutable: true }) selectedTitle: HTMLCalciteTabTitleElement = null;

/**
* Specifies the size of the component inherited from the parent `calcite-tabs`, defaults to `m`.
*
* @internal
*/
@Prop({ reflect: true, mutable: true }) scale: Scale = "m";
@Prop() scale: Scale = "m";

/**
* @internal
*/
@Prop({ reflect: true, mutable: true }) layout: TabLayout = "inline";

/**
* @internal
* Specifies the position of `calcite-tab-nav` and `calcite-tab-title` components in relation to, and is inherited from the parent `calcite-tabs`, defaults to `top`.
*
* @internal
*/
@Prop({ reflect: true, mutable: true }) position: TabPosition = "bottom";
@Prop() position: TabPosition = "bottom";

/**
* @internal
Expand Down Expand Up @@ -121,10 +126,6 @@ export class TabNav {
this.resizeObserver?.observe(this.el);
}

disconnectedCallback(): void {
this.resizeObserver?.disconnect();
}

componentWillLoad(): void {
const storageKey = `calcite-tab-nav-${this.storageId}`;
if (localStorage && this.storageId && localStorage.getItem(storageKey)) {
Expand All @@ -137,8 +138,6 @@ export class TabNav {
const { parentTabsEl } = this;

this.layout = parentTabsEl?.layout;
this.position = parentTabsEl?.position;
this.scale = parentTabsEl?.scale;
this.bordered = parentTabsEl?.bordered;
// fix issue with active tab-title not lining up with blue indicator
if (this.selectedTitle) {
Expand All @@ -161,6 +160,10 @@ export class TabNav {
}
}

disconnectedCallback(): void {
this.resizeObserver?.disconnect();
}

render(): VNode {
const dir = getElementDir(this.el);
const width = `${this.indicatorWidth}px`;
Expand All @@ -169,7 +172,11 @@ export class TabNav {
return (
<Host role="tablist">
<div
class="tab-nav"
class={{
[CSS.tabNav]: true,
[`scale-${this.scale}`]: true,
[`position-${this.position}`]: true,
}}
onScroll={this.handleContainerScroll}
// eslint-disable-next-line react/jsx-sort-props -- ref should be last so node attrs/props are in sync (see https://github.com/Esri/calcite-design-system/pull/6530)
ref={(el: HTMLDivElement) => (this.tabNavEl = el)}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
When tab-nav is the only parent, tab-title can inherit its `scale` and `position` from tab-nav:
Tab-nav and tab-title inherit their `scale` and `position` from tabs parent.

Elijbet marked this conversation as resolved.
Show resolved Hide resolved
```html
<calcite-tab-nav scale="l" position="bottom">
<calcite-tab-title>Layers</calcite-tab-title>
<calcite-tab-title>Maps</calcite-tab-title>
<calcite-tab-title selected>Data</calcite-tab-title>
</calcite-tab-nav>
<calcite-tabs scale="l" position="bottom">
<calcite-tab-nav>
<calcite-tab-title>Layers</calcite-tab-title>
<calcite-tab-title>Maps</calcite-tab-title>
<calcite-tab-title selected>Data</calcite-tab-title>
</calcite-tab-nav>
</calcite-tabs>
```
14 changes: 0 additions & 14 deletions packages/calcite-components/src/components/tab-title/resources.ts

This file was deleted.

Loading
Loading