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

fix(panel): tweak focusable content area #10141

Merged
merged 30 commits into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
2ec1b7a
fix(panel): tweak focusable content area
jcfranco Aug 21, 2024
32f873b
improve scrolling/non-scrolling test coverage
jcfranco Aug 27, 2024
12bee02
merge dev
jcfranco Aug 27, 2024
a64cff2
Merge branch 'dev' into jcfranco/10022-tidy-up-scroller-content-tabbing
jcfranco Aug 30, 2024
8726142
tidy up test
jcfranco Aug 30, 2024
90ccd29
remove invalid test
jcfranco Aug 30, 2024
d669873
update disabled helper to check different targets for click and click…
jcfranco Aug 30, 2024
c2c810d
use same default body click focus target
jcfranco Aug 31, 2024
cf84dd9
fix tests
jcfranco Aug 31, 2024
a863056
troubleshoot panel & flow-item tests
jcfranco Sep 1, 2024
c90a34e
restore previous assertions
jcfranco Sep 1, 2024
9d1a73a
add context to assertion error messages
jcfranco Sep 1, 2024
da1c313
test rest of assertions
jcfranco Sep 3, 2024
370e29e
restore assertion and add delay before asserting failing assertion
jcfranco Sep 3, 2024
ca43a99
log extra info
jcfranco Sep 3, 2024
241634f
tweak log info
jcfranco Sep 3, 2024
b546802
fix prop access
jcfranco Sep 3, 2024
52ba6c7
tweak log info
jcfranco Sep 4, 2024
ecabbe0
bump puppeteer
jcfranco Sep 4, 2024
ff2cc38
merge dev
jcfranco Sep 4, 2024
e145c64
bump puppeteer properly
jcfranco Sep 4, 2024
e4964e1
add large delay before assertion
jcfranco Sep 4, 2024
51e3d27
tweak page size and sandbox
jcfranco Sep 4, 2024
d7d77e8
roll back sandbox options to pinpoint fix
jcfranco Sep 4, 2024
1cc1f79
roll back dimensions from test config
jcfranco Sep 4, 2024
4238007
remove troubleshooting log messages and tidy up context messages
jcfranco Sep 4, 2024
ddc4d16
roll back puppeteer version
jcfranco Sep 4, 2024
4bcc814
revert lockfile
jcfranco Sep 4, 2024
f246b82
remove focused-test pattern for troubleshooting
jcfranco Sep 4, 2024
c9346af
fix test where clicking on body would restore focus on component
jcfranco Sep 4, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,15 @@ describe("calcite-checkbox", () => {
});

describe("disabled", () => {
disabled("calcite-checkbox");
disabled("calcite-checkbox", {
focusTarget: {
tab: "calcite-checkbox",
click: {
pointer: "calcite-checkbox",
method: "calcite-checkbox",
},
},
});
});

it("renders with correct default attributes", async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,15 @@ describe("calcite-combobox", () => {
});

describe("disabled", () => {
disabled("calcite-combobox");
disabled("calcite-combobox", {
focusTarget: {
tab: "calcite-combobox",
click: {
pointer: "calcite-combobox",
method: "calcite-combobox",
},
},
});
});

const simpleComboboxHTML = html`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
} from "../../tests/commonTests";
import { html } from "../../../support/formatting";
import { GlobalTestProps } from "../../tests/utils";
import { scrollingContentHtml, scrollingHeightStyle } from "../panel/panel.e2e";
import { IDS as PanelIDS } from "../panel/resources";
import { CSS, SLOTS } from "./resources";

Expand Down Expand Up @@ -124,7 +125,24 @@ describe("calcite-flow-item", () => {
});

describe("disabled", () => {
disabled(`<calcite-flow-item closable>scrolling content</calcite-flow-item>`);
disabled(html`<calcite-flow-item style="${scrollingHeightStyle}">${scrollingContentHtml}</calcite-flow-item>`, {
focusTarget: {
tab: "calcite-flow-item",
click: "body",
},
});

describe("closable", () => {
disabled(
html`<calcite-flow-item closable style="${scrollingHeightStyle}">${scrollingContentHtml}</calcite-flow-item>`,
{
focusTarget: {
tab: "calcite-flow-item",
click: "body",
},
},
);
});
});

describe("accessible", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,15 @@ describe("calcite-input-date-picker", () => {
});

describe("disabled", () => {
disabled("calcite-input-date-picker");
disabled("calcite-input-date-picker", {
focusTarget: {
tab: "calcite-input-date-picker",
click: {
pointer: "calcite-input-date-picker",
method: "calcite-input-date-picker",
},
},
});
});

describe("openClose", () => {
Expand Down
243 changes: 188 additions & 55 deletions packages/calcite-components/src/components/panel/panel.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,55 @@ const panelTemplate = (scrollable = false) =>
</calcite-panel>
</div>`;

export const scrollingContentHtml = html`
<p>
Enim nascetur erat faucibus ornare varius arcu fames bibendum habitant felis elit ante. Nibh morbi massa curae; leo
semper diam aenean congue taciti eu porta. Varius faucibus ridiculus donec. Montes sit ligula purus porta ante lacus
habitasse libero cubilia purus! In quis congue arcu maecenas felis cursus pellentesque nascetur porta donec non.
Quisque, rutrum ligula pharetra justo habitasse facilisis rutrum neque. Magnis nostra nec nulla dictumst taciti
consectetur. Non porttitor tempor orci dictumst magna porta vitae.
</p>
<p>
Ipsum nostra tempus etiam augue ullamcorper scelerisque sapien potenti erat nisi gravida. Vehicula sem tristique
sed. Nullam, sociis imperdiet ullamcorper? Dapibus fames primis ridiculus vulputate, habitant inceptos! Nunc
torquent lorem urna vehicula volutpat donec nec. Orci massa eu nec donec enim fames, faucibus quam aenean. Laoreet
tellus tempor quisque ornare lobortis praesent erat senectus natoque consectetur donec imperdiet. Quis sem cum
gravida dictumst a pretium purus aptent amet id. Orci habitasse, praesent facilisis condimentum. Nec elit turpis
leo.
</p>
<p>
Tempus per volutpat diam tempor mauris parturient vulputate leo id libero quisque. Mattis aliquam dictum venenatis
fringilla. Taciti venenatis, ultrices sollicitudin consequat. Sapien fusce est iaculis potenti ut auctor potenti.
Nisi malesuada feugiat vulputate vitae porttitor. Nullam nullam nullam accumsan quis magna in. Elementum, nascetur
gravida cras scelerisque inceptos aenean inceptos potenti. Lobortis condimentum accumsan posuere curabitur fermentum
diam, natoque quisque. Eget placerat sed aptent orci urna fusce magnis. Vel lacus magnis nunc.
</p>
<p>
Enim nascetur erat faucibus ornare varius arcu fames bibendum habitant felis elit ante. Nibh morbi massa curae; leo
semper diam aenean congue taciti eu porta. Varius faucibus ridiculus donec. Montes sit ligula purus porta ante lacus
habitasse libero cubilia purus! In quis congue arcu maecenas felis cursus pellentesque nascetur porta donec non.
Quisque, rutrum ligula pharetra justo habitasse facilisis rutrum neque. Magnis nostra nec nulla dictumst taciti
consectetur. Non porttitor tempor orci dictumst magna porta vitae.
</p>
<p>
Ipsum nostra tempus etiam augue ullamcorper scelerisque sapien potenti erat nisi gravida. Vehicula sem tristique
sed. Nullam, sociis imperdiet ullamcorper? Dapibus fames primis ridiculus vulputate, habitant inceptos! Nunc
torquent lorem urna vehicula volutpat donec nec. Orci massa eu nec donec enim fames, faucibus quam aenean. Laoreet
tellus tempor quisque ornare lobortis praesent erat senectus natoque consectetur donec imperdiet. Quis sem cum
gravida dictumst a pretium purus aptent amet id. Orci habitasse, praesent facilisis condimentum. Nec elit turpis
leo.
</p>
<p>
Tempus per volutpat diam tempor mauris parturient vulputate leo id libero quisque. Mattis aliquam dictum venenatis
fringilla. Taciti venenatis, ultrices sollicitudin consequat. Sapien fusce est iaculis potenti ut auctor potenti.
Nisi malesuada feugiat vulputate vitae porttitor. Nullam nullam nullam accumsan quis magna in. Elementum, nascetur
gravida cras scelerisque inceptos aenean inceptos potenti. Lobortis condimentum accumsan posuere curabitur fermentum
diam, natoque quisque. Eget placerat sed aptent orci urna fusce magnis. Vel lacus magnis nunc.
</p>
`;

export const scrollingHeightStyle = "height: 200px;";

describe("calcite-panel", () => {
describe("renders", () => {
renders("calcite-panel", { display: "flex" });
Expand Down Expand Up @@ -102,7 +151,38 @@ describe("calcite-panel", () => {
});

describe("disabled", () => {
disabled(`<calcite-panel closable>scrolling content</calcite-panel>`);
describe("with scrolling content", () => {
disabled(html`<calcite-panel style="${scrollingHeightStyle}">${scrollingContentHtml}</calcite-panel>`, {
focusTarget: {
tab: "calcite-panel",
click: "body",
},
});

describe("closable", () => {
disabled(
html`<calcite-panel closable style="${scrollingHeightStyle}">${scrollingContentHtml}</calcite-panel>`,
{
focusTarget: {
tab: "calcite-panel",
click: "body",
},
},
);
});
});

describe("without scrolling content", () => {
disabled(html`<calcite-panel>non-scrolling content</calcite-panel>`, {
focusTarget: "none",
});

describe("closable", () => {
disabled(html`<calcite-panel closable>non-scrolling content</calcite-panel>`, {
focusTarget: "none",
});
});
});
});

describe("translation support", () => {
Expand Down Expand Up @@ -252,9 +332,9 @@ describe("calcite-panel", () => {
<calcite-panel>
<calcite-action-bar slot="${SLOTS.actionBar}">
<calcite-action-group>
<calcite-action text="Add" icon="plus"> </calcite-action>
<calcite-action text="Save" icon="save"> </calcite-action>
<calcite-action text="Layers" icon="layers"> </calcite-action>
<calcite-action text="Add" icon="plus"></calcite-action>
<calcite-action text="Save" icon="save"></calcite-action>
<calcite-action text="Layers" icon="layers"></calcite-action>
</calcite-action-group>
</calcite-action-bar>
<div slot="${SLOTS.headerActionsStart}">test start</div>
Expand All @@ -270,9 +350,9 @@ describe("calcite-panel", () => {
<calcite-panel collapsible closable>
<calcite-action-bar slot="${SLOTS.actionBar}">
<calcite-action-group>
<calcite-action text="Add" icon="plus"> </calcite-action>
<calcite-action text="Save" icon="save"> </calcite-action>
<calcite-action text="Layers" icon="layers"> </calcite-action>
<calcite-action text="Add" icon="plus"></calcite-action>
<calcite-action text="Save" icon="save"></calcite-action>
<calcite-action text="Layers" icon="layers"></calcite-action>
</calcite-action-group>
</calcite-action-bar>
<div slot="${SLOTS.headerActionsStart}">test start</div>
Expand All @@ -285,15 +365,36 @@ describe("calcite-panel", () => {
`);
});

describe("should focus on close button", () => {
focusable(`<calcite-panel closable>test</calcite-panel>`, {
shadowFocusTargetSelector: "calcite-action",
describe("is focusable", () => {
describe("with scrolling content", () => {
describe("closable", () => {
focusable(
html`<calcite-panel closable style="${scrollingHeightStyle}">${scrollingContentHtml}</calcite-panel>`,
{
shadowFocusTargetSelector: "calcite-action",
},
);
});

describe("should focus on container", () => {
focusable(html`<calcite-panel style="${scrollingHeightStyle}">${scrollingContentHtml}</calcite-panel>`, {
shadowFocusTargetSelector: `.${CSS.contentWrapper}`,
});
});
});
});

describe("should focus on container", () => {
focusable(`<calcite-panel>test</calcite-panel>`, {
shadowFocusTargetSelector: "article",
describe("without scrolling content", () => {
describe("closable", () => {
focusable(html`<calcite-panel closable>non-scrolling content</calcite-panel>`, {
shadowFocusTargetSelector: "calcite-action",
});
});

describe("should not focus on container", () => {
focusable(html`<calcite-panel>non-scrolling-content</calcite-panel>`, {
focusTargetSelector: "body",
});
});
});
});

Expand Down Expand Up @@ -456,51 +557,83 @@ describe("calcite-panel", () => {
expect(await scrollEl.getProperty("scrollTop")).toBe(100);
});

it("should close when Escape key is pressed and closable is true", async () => {
const page = await newE2EPage();
await page.setContent("<calcite-panel>test</calcite-panel>");
const panel = await page.find("calcite-panel");
const calcitePanelClose = await panel.spyOnEvent("calcitePanelClose");
const container = await page.find(`calcite-panel >>> .${CSS.container}`);
expect(await panel.getProperty("closed")).toBe(false);
expect(await container.isVisible()).toBe(true);
await container.press("Escape");
await page.waitForChanges();
expect(await panel.getProperty("closed")).toBe(false);
expect(await container.isVisible()).toBe(true);
panel.setProperty("closable", true);
await page.waitForChanges();
await container.press("Escape");
await page.waitForChanges();
expect(await panel.getProperty("closed")).toBe(true);
expect(await container.isVisible()).toBe(false);
expect(calcitePanelClose).toHaveReceivedEventTimes(1);
});

it("should not close when Escape key is prevented and closable is true", async () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

@driskull Due to the contents of scrollingContentHtml, this test is only covered under the scrolling use case.

We could add coverage for this test for both scrolling and non-scrolling content by adding focusable elements to the content. Does that approach make sense? If so, would we want to keep scrolling content and non-scrolling content as the main test groups or do we need an additional group to cover scrolling with and without interactive content?

const page = await newE2EPage();
await page.setContent("<calcite-panel closable>test</calcite-panel>");
const panel = await page.find("calcite-panel");
const calcitePanelClose = await panel.spyOnEvent("calcitePanelClose");
const container = await page.find(`calcite-panel >>> .${CSS.container}`);

expect(await panel.getProperty("closed")).toBe(false);
expect(await container.isVisible()).toBe(true);
describe("closable", () => {
describe("with scrollable content (Escape emits from scroll container)", () => {
it("should close when Escape key is pressed and closable is true", async () => {
const page = await newE2EPage();
await page.setContent(
html`<calcite-panel style="${scrollingHeightStyle}">${scrollingContentHtml}</calcite-panel>`,
);
const panel = await page.find("calcite-panel");
const calcitePanelClose = await panel.spyOnEvent("calcitePanelClose");
const contentWrapper = await page.find(`calcite-panel >>> .${CSS.contentWrapper}`);
const container = await page.find(`calcite-panel >>> .${CSS.container}`);
expect(await panel.getProperty("closed")).toBe(false);
expect(await container.isVisible()).toBe(true);
await contentWrapper.press("Escape");
await page.waitForChanges();
expect(await panel.getProperty("closed")).toBe(false);
expect(await container.isVisible()).toBe(true);
panel.setProperty("closable", true);
await page.waitForChanges();

await contentWrapper.press("Escape");
await page.waitForChanges();
expect(await panel.getProperty("closed")).toBe(true);
expect(await container.isVisible()).toBe(false);
expect(calcitePanelClose).toHaveReceivedEventTimes(1);
});

await page.$eval("calcite-panel", (panel: HTMLCalcitePanelElement) => {
panel.addEventListener("keydown", (event) => {
if (event.key === "Escape") {
event.preventDefault();
}
it("should not close when Escape key is prevented and closable is true", async () => {
const page = await newE2EPage();
await page.setContent(
html`<calcite-panel closable style="${scrollingHeightStyle}">${scrollingContentHtml}</calcite-panel>`,
);
const panel = await page.find("calcite-panel");
const calcitePanelClose = await panel.spyOnEvent("calcitePanelClose");
const container = await page.find(`calcite-panel >>> .${CSS.container}`);

expect(await panel.getProperty("closed")).toBe(false);
expect(await container.isVisible()).toBe(true);

await page.$eval("calcite-panel", (panel: HTMLCalcitePanelElement) => {
panel.addEventListener("keydown", (event) => {
if (event.key === "Escape") {
event.preventDefault();
}
});
});

await panel.press("Escape");
await page.waitForChanges();

expect(await panel.getProperty("closed")).toBe(false);
expect(await container.isVisible()).toBe(true);
expect(calcitePanelClose).toHaveReceivedEventTimes(0);
});
});

await panel.press("Escape");
await page.waitForChanges();

expect(await panel.getProperty("closed")).toBe(false);
expect(await container.isVisible()).toBe(true);
expect(calcitePanelClose).toHaveReceivedEventTimes(0);
describe("without scrollable content (Escape emits from close button)", () => {
it("should close when Escape key is pressed and closable is true", async () => {
const page = await newE2EPage();
await page.setContent(html`<calcite-panel closable>non-scrolling content</calcite-panel>`);
const panel = await page.find("calcite-panel");
const calcitePanelClose = await panel.spyOnEvent("calcitePanelClose");
const closeButton = await page.find(`calcite-panel >>> #${IDS.close}`);
const container = await page.find(`calcite-panel >>> .${CSS.container}`);
expect(await panel.getProperty("closed")).toBe(false);
expect(await container.isVisible()).toBe(true);
expect(calcitePanelClose).toHaveReceivedEventTimes(0);

await closeButton.callMethod("setFocus");
await closeButton.press("Escape");
await page.waitForChanges();

expect(await panel.getProperty("closed")).toBe(true);
expect(await container.isVisible()).toBe(false);
expect(calcitePanelClose).toHaveReceivedEventTimes(1);
});
});
});

describe("theme", () => {
Expand Down
Loading
Loading