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

fix: tabs and panel get out of sync when no id is present #5907

Merged
merged 3 commits into from
Apr 28, 2022
Merged

fix: tabs and panel get out of sync when no id is present #5907

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
df0ecdd
ensure tab and tabpanel instances without ids do not go out of sync d…
chrisdholt Apr 27, 2022
e28f6a8
Merge branch 'master' into users/chhol/fix-tabs-and-panel-not-working…
chrisdholt Apr 27, 2022
9097139
Merge branch 'master' into users/chhol/fix-tabs-and-panel-not-working…
chrisdholt Apr 28, 2022
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
7 changes: 7 additions & 0 deletions change/@microsoft-fast-components-fdfdfb2e-037b-40ce-95c0-8073b5fe4e86.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "ensure that tabs and tabpanels without ids stay in sync",
"packageName": "@microsoft/fast-components",
"email": "chhol@microsoft.com",
"dependentChangeType": "patch"
}
7 changes: 7 additions & 0 deletions change/@microsoft-fast-foundation-3ad67a28-b5e7-48b9-9d64-eff861c33202.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "ensure that tabs and tabpanels without ids stay in sync",
"packageName": "@microsoft/fast-foundation",
"email": "chhol@microsoft.com",
"dependentChangeType": "patch"
}
5 changes: 4 additions & 1 deletion packages/web-components/fast-components/src/tabs/fixtures/base.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ <h2>No ID (multiple tabs)</h2>
<div>Testing</div>
</fast-tabs>

<fast-tabs id="noId2">
<fast-tabs id="addTabsExample">
<fast-tab>Tab four</fast-tab>
<fast-tab>Tab five</fast-tab>
<fast-tab>Tab six</fast-tab>
Expand All @@ -373,3 +373,6 @@ <h2>No ID (multiple tabs)</h2>
</fast-tab-panel>
<div>Testing</div>
</fast-tabs>
<fast-button id="add-button" appearance="accent">
Add Tabs
</fast-button>
23 changes: 23 additions & 0 deletions packages/web-components/fast-components/src/tabs/tabs.stories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,28 @@
import { STORY_RENDERED } from "@storybook/core-events";
import addons from "@storybook/addons";
import Examples from "./fixtures/base.html";

function addItem(): void {
const tabsElement = document.getElementById("addTabsExample");

if (tabsElement?.children !== undefined) {
const tab: any = document.createElement("fast-tab");
const tabPanel: any = document.createElement("fast-tab-panel");
tab.textContent = "Added tab";
tabPanel.textContent = "Added panel";

tabsElement?.appendChild(tab);
tabsElement.insertBefore(tabPanel, tab);
}
}

addons.getChannel().addListener(STORY_RENDERED, (name: string) => {
if (name.toLowerCase() === "tabs--tabs") {
const button = document.getElementById("add-button") as HTMLElement;
button.addEventListener("click", () => addItem());
}
});

export default {
title: "Tabs",
};
Expand Down
150 changes: 148 additions & 2 deletions packages/web-components/fast-foundation/src/tabs/tabs.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ describe("Tabs", () => {
it("should set an `id` attribute tab items with a unique ID when an `id is NOT provided", async () => {
const { element, connect, disconnect } = await fixture([FASTTabs(), FASTTabPanel(), FASTTab()])

for (let i = 1; i < 4; i++) {
for (let i = 0; i < 4; i++) {
const tab = document.createElement("fast-tab") as Tab;
const panel = document.createElement("fast-tab-panel") as TabPanel;

Expand All @@ -145,8 +145,82 @@ describe("Tabs", () => {

await connect();

expect(element.querySelector("fast-tab")?.getAttribute("id")).to.not.be.undefined;
expect(element.querySelectorAll("fast-tab")[0]?.getAttribute("id")).to.not.be.undefined;
expect(element.querySelectorAll("fast-tab")[1]?.getAttribute("id")).to.not.be.undefined;
expect(element.querySelectorAll("fast-tab")[2]?.getAttribute("id")).to.not.be.undefined;
expect(element.querySelectorAll("fast-tab")[3]?.getAttribute("id")).to.not.be.undefined;

await disconnect();
});

it("should set the corresponding tab panel aria-labelledby attribute to the corresponding tab unique ID when a tab id is NOT provided", async () => {
const { element, connect, disconnect } = await fixture([FASTTabs(), FASTTabPanel(), FASTTab()])

for (let i = 0; i < 4; i++) {
const tab = document.createElement("fast-tab") as Tab;
const panel = document.createElement("fast-tab-panel") as TabPanel;

element.appendChild(panel);
element.insertBefore(tab, element.querySelector("fast-tab-panel"));
}

await connect();

let tabId0: string | null = element.querySelectorAll("fast-tab")[0]?.getAttribute("id");
let tabId1: string | null = element.querySelectorAll("fast-tab")[1]?.getAttribute("id");
let tabId2: string | null = element.querySelectorAll("fast-tab")[2]?.getAttribute("id");
let tabId3: string | null = element.querySelectorAll("fast-tab")[3]?.getAttribute("id");

expect(element.querySelectorAll("fast-tab-panel")[0]?.getAttribute("aria-labelledby")).to.equal(tabId0);
expect(element.querySelectorAll("fast-tab-panel")[1]?.getAttribute("aria-labelledby")).to.equal(tabId1);
expect(element.querySelectorAll("fast-tab-panel")[2]?.getAttribute("aria-labelledby")).to.equal(tabId2);
expect(element.querySelectorAll("fast-tab-panel")[3]?.getAttribute("aria-labelledby")).to.equal(tabId3);

await disconnect();
});

it("should set the corresponding tab panel aria-labelledby attribute to the corresponding tab unique ID when a tab id is NOT provided and additional tabs and panels are added", async () => {
const { element, connect, disconnect } = await fixture([FASTTabs(), FASTTabPanel(), FASTTab()])

for (let i = 0; i < 4; i++) {
const tab = document.createElement("fast-tab") as Tab;
const panel = document.createElement("fast-tab-panel") as TabPanel;

element.appendChild(panel);
element.insertBefore(tab, element.querySelector("fast-tab-panel"));
}

await connect();

let tabId0: string | null = element.querySelectorAll("fast-tab")[0]?.getAttribute("id");
let tabId1: string | null = element.querySelectorAll("fast-tab")[1]?.getAttribute("id");
let tabId2: string | null = element.querySelectorAll("fast-tab")[2]?.getAttribute("id");
let tabId3: string | null = element.querySelectorAll("fast-tab")[3]?.getAttribute("id");

expect(element.querySelectorAll("fast-tab-panel")[0]?.getAttribute("aria-labelledby")).to.equal(tabId0);
expect(element.querySelectorAll("fast-tab-panel")[1]?.getAttribute("aria-labelledby")).to.equal(tabId1);
expect(element.querySelectorAll("fast-tab-panel")[2]?.getAttribute("aria-labelledby")).to.equal(tabId2);
expect(element.querySelectorAll("fast-tab-panel")[3]?.getAttribute("aria-labelledby")).to.equal(tabId3);

const newTab = document.createElement("fast-tab") as Tab;
const newPanel = document.createElement("fast-tab-panel") as TabPanel;

element.appendChild(newPanel);
element.insertBefore(newTab, element.querySelector("fast-tab-panel"));

await DOM.nextUpdate();

tabId0 = element.querySelectorAll("fast-tab")[0]?.getAttribute("id");
tabId1 = element.querySelectorAll("fast-tab")[1]?.getAttribute("id");
tabId2 = element.querySelectorAll("fast-tab")[2]?.getAttribute("id");
tabId3 = element.querySelectorAll("fast-tab")[3]?.getAttribute("id");
let tabId4 = element.querySelectorAll("fast-tab")[4]?.getAttribute("id");

expect(element.querySelectorAll("fast-tab-panel")[0]?.getAttribute("aria-labelledby")).to.equal(tabId0);
expect(element.querySelectorAll("fast-tab-panel")[1]?.getAttribute("aria-labelledby")).to.equal(tabId1);
expect(element.querySelectorAll("fast-tab-panel")[2]?.getAttribute("aria-labelledby")).to.equal(tabId2);
expect(element.querySelectorAll("fast-tab-panel")[3]?.getAttribute("aria-labelledby")).to.equal(tabId3);
expect(element.querySelectorAll("fast-tab-panel")[4]?.getAttribute("aria-labelledby")).to.equal(tabId4);

await disconnect();
});
Expand All @@ -166,6 +240,78 @@ describe("Tabs", () => {
await disconnect();
});

it("should set the tabpanel id to the corresponding tab aria-controls attribute when a tabpanel id is NOT provided", async () => {
const { element, connect, disconnect } = await fixture([FASTTabs(), FASTTabPanel(), FASTTab()])

for (let i = 0; i < 4; i++) {
const tab = document.createElement("fast-tab") as Tab;
const panel = document.createElement("fast-tab-panel") as TabPanel;

element.appendChild(panel);
element.insertBefore(tab, element.querySelector("fast-tab-panel"));
}

await connect();

let tabpanelId0: string | null = element.querySelectorAll("fast-tab-panel")[0]?.getAttribute("id");
let tabpanelId1: string | null = element.querySelectorAll("fast-tab-panel")[1]?.getAttribute("id");
let tabpanelId2: string | null = element.querySelectorAll("fast-tab-panel")[2]?.getAttribute("id");
let tabpanelId3: string | null = element.querySelectorAll("fast-tab-panel")[3]?.getAttribute("id");

expect(element.querySelectorAll("fast-tab")[0]?.getAttribute("aria-controls")).to.equal(tabpanelId0);
expect(element.querySelectorAll("fast-tab")[1]?.getAttribute("aria-controls")).to.equal(tabpanelId1);
expect(element.querySelectorAll("fast-tab")[2]?.getAttribute("aria-controls")).to.equal(tabpanelId2);
expect(element.querySelectorAll("fast-tab")[3]?.getAttribute("aria-controls")).to.equal(tabpanelId3);

await disconnect();
});

it("should set the tabpanel id to the corresponding tab aria-controls attribute when a tabpanel id is NOT provided and new tabs and tabpanels are added", async () => {
const { element, connect, disconnect } = await fixture([FASTTabs(), FASTTabPanel(), FASTTab()])

for (let i = 0; i < 4; i++) {
const tab = document.createElement("fast-tab") as Tab;
const panel = document.createElement("fast-tab-panel") as TabPanel;

element.appendChild(panel);
element.insertBefore(tab, element.querySelector("fast-tab-panel"));
}

await connect();

let tabpanelId0: string | null = element.querySelectorAll("fast-tab-panel")[0]?.getAttribute("id");
let tabpanelId1: string | null = element.querySelectorAll("fast-tab-panel")[1]?.getAttribute("id");
let tabpanelId2: string | null = element.querySelectorAll("fast-tab-panel")[2]?.getAttribute("id");
let tabpanelId3: string | null = element.querySelectorAll("fast-tab-panel")[3]?.getAttribute("id");

expect(element.querySelectorAll("fast-tab")[0]?.getAttribute("aria-controls")).to.equal(tabpanelId0);
expect(element.querySelectorAll("fast-tab")[1]?.getAttribute("aria-controls")).to.equal(tabpanelId1);
expect(element.querySelectorAll("fast-tab")[2]?.getAttribute("aria-controls")).to.equal(tabpanelId2);
expect(element.querySelectorAll("fast-tab")[3]?.getAttribute("aria-controls")).to.equal(tabpanelId3);

const newTab = document.createElement("fast-tab") as Tab;
const newPanel = document.createElement("fast-tab-panel") as TabPanel;

element.appendChild(newPanel);
element.insertBefore(newTab, element.querySelector("fast-tab-panel"));

await DOM.nextUpdate();

tabpanelId0 = element.querySelectorAll("fast-tab-panel")[0]?.getAttribute("id");
tabpanelId1 = element.querySelectorAll("fast-tab-panel")[1]?.getAttribute("id");
tabpanelId2 = element.querySelectorAll("fast-tab-panel")[2]?.getAttribute("id");
tabpanelId3 = element.querySelectorAll("fast-tab-panel")[3]?.getAttribute("id");
let tabpanelId4 = element.querySelectorAll("fast-tab-panel")[4]?.getAttribute("id");

expect(element.querySelectorAll("fast-tab")[0]?.getAttribute("aria-controls")).to.equal(tabpanelId0);
expect(element.querySelectorAll("fast-tab")[1]?.getAttribute("aria-controls")).to.equal(tabpanelId1);
expect(element.querySelectorAll("fast-tab")[2]?.getAttribute("aria-controls")).to.equal(tabpanelId2);
expect(element.querySelectorAll("fast-tab")[3]?.getAttribute("aria-controls")).to.equal(tabpanelId3);
expect(element.querySelectorAll("fast-tab")[4]?.getAttribute("aria-controls")).to.equal(tabpanelId4);

await disconnect();
});

describe("active tab", () => {
it("should set an `aria-selected` attribute on the active tab when `activeId` is provided", async () => {
const { element, connect, disconnect, tab2 } = await setup();
Expand Down
11 changes: 7 additions & 4 deletions packages/web-components/fast-foundation/src/tabs/tabs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ export class Tabs extends FoundationElement {
this.$fastController.isConnected &&
this.tabs.length <= this.tabpanels.length
) {
this.tabIds = this.getTabIds();
this.tabpanelIds = this.getTabPanelIds();

this.setTabs();
this.setTabPanels();
this.handleActiveIndicatorPosition();
Expand All @@ -114,6 +117,9 @@ export class Tabs extends FoundationElement {
this.$fastController.isConnected &&
this.tabpanels.length <= this.tabs.length
) {
this.tabIds = this.getTabIds();
this.tabpanelIds = this.getTabPanelIds();

this.setTabs();
this.setTabPanels();
this.handleActiveIndicatorPosition();
Expand Down Expand Up @@ -182,8 +188,7 @@ export class Tabs extends FoundationElement {
const gridProperty: string = this.isHorizontal()
? gridHorizontalProperty
: gridVerticalProperty;
this.tabIds = this.getTabIds();
this.tabpanelIds = this.getTabPanelIds();

this.activeTabIndex = this.getActiveIndex();
this.showActiveIndicator = false;
this.tabs.forEach((tab: HTMLElement, index: number) => {
Expand Down Expand Up @@ -218,8 +223,6 @@ export class Tabs extends FoundationElement {
};

private setTabPanels = (): void => {
this.tabIds = this.getTabIds();
this.tabpanelIds = this.getTabPanelIds();
this.tabpanels.forEach((tabpanel: HTMLElement, index: number) => {
const tabId: string = this.tabIds[index];
const tabpanelId: string = this.tabpanelIds[index];
Expand Down