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

feat(dialog, modal, popover, input-date-picker, input-time-picker, sheet): support stacked component sequential closing with escape #9231

Open
wants to merge 71 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
71 commits
Select commit Hold shift + click to select a range
b9b684d
set up demo for testing component sequential closing behavior
Elijbet Apr 30, 2024
281619e
WIP
driskull Apr 30, 2024
2ad2c8e
WIP
Elijbet May 1, 2024
c95e3aa
WIP
Elijbet May 1, 2024
c54cab1
WIP
Elijbet May 1, 2024
e3823ea
add docs
Elijbet May 1, 2024
1187a3a
simplified HTML for Demo and e2e test to reflect the demo workflow
Elijbet May 1, 2024
fadbb0d
cleanup
Elijbet May 2, 2024
0622da1
stacked focus-trap components test
Elijbet May 2, 2024
00e576b
relocate test for closing of stacked focus-trap components to globalS…
Elijbet May 8, 2024
55fbb07
fix failing tests
Elijbet May 8, 2024
0af108e
add wait for events open/close to input-date-picker for additional check
Elijbet Jun 20, 2024
47cf21d
add wait for events open/close to input-time-picker for additional check
Elijbet Jun 20, 2024
6a1ad45
merge dev
Elijbet Jul 30, 2024
8ca1dc5
docs
Elijbet Jul 30, 2024
d2324b2
merge dev
Elijbet Jul 30, 2024
5861f82
fix timing issues with modal and sheet
Elijbet Aug 2, 2024
95af390
remove focus-trapping logic and let popover handle it
Elijbet Aug 2, 2024
69dbd5a
WIP
Elijbet Aug 2, 2024
ba39a7f
WIP
Elijbet Aug 2, 2024
3e4b38e
enable focus trap in input-time-picker and fix failing tests
Elijbet Aug 4, 2024
85f4ef2
Helper to manage focusTrap by determining if a click event occurred o…
Elijbet Aug 8, 2024
7418a25
WIP
Elijbet Aug 8, 2024
a9dd5c0
WIP
Elijbet Aug 8, 2024
58622a4
WIP test
Elijbet Aug 9, 2024
0376e14
add focus trap to dialog
Elijbet Aug 9, 2024
f11fb0f
WIP
Elijbet Aug 9, 2024
31764b2
WIP
Elijbet Aug 9, 2024
a7cebb6
WIP should not toggle popovers with triggerDisabled
Elijbet Aug 9, 2024
c993e02
merge dev
jcfranco Aug 19, 2024
327aa38
WIP popover
Elijbet Aug 20, 2024
785fed5
WIP popover
Elijbet Aug 20, 2024
84f881e
WIP popover
Elijbet Aug 21, 2024
84c20d8
WIP input-date-picker
Elijbet Aug 21, 2024
9db785e
merge dev
Elijbet Aug 21, 2024
06570cd
merge dev
Elijbet Aug 21, 2024
c3e5da4
test cleanup
Elijbet Aug 21, 2024
ef5ad2a
cleanup
Elijbet Aug 21, 2024
16564f1
merge dev
jcfranco Aug 22, 2024
f7dd170
WIP
Elijbet Aug 23, 2024
a493275
WIP
Elijbet Aug 23, 2024
6d3ed51
WIP
Elijbet Aug 23, 2024
d8d0412
WIP
Elijbet Aug 23, 2024
9f18ccc
WIP
Elijbet Aug 24, 2024
1e67bf8
Utilities for implementing FocusTrapComponent are no longer necessary…
Elijbet Aug 25, 2024
3e94a4d
WIP
Elijbet Aug 25, 2024
16c7f1a
WIP
Elijbet Aug 25, 2024
5d5b90b
WIP
Elijbet Aug 26, 2024
20cde10
make clickOutsideDeactivates depend on outsideCloseDisabled value
Elijbet Sep 1, 2024
249081a
merge dev
Elijbet Sep 4, 2024
829d93b
remove clickOutsideDeactivates option from focus-trap since scrim han…
Elijbet Sep 6, 2024
d8acece
merge dev
Elijbet Sep 24, 2024
76a4cf7
cleanup
Elijbet Sep 24, 2024
766ec1e
merge dev
Elijbet Sep 26, 2024
f720d10
change logic to handle ignoring an Escape key press in focus-trap
Elijbet Sep 26, 2024
6bf5a5f
change logic to handle ignoring an Escape key press in focus-trap on …
Elijbet Sep 26, 2024
7f110b6
adjust test timing to accomodate changes
Elijbet Sep 26, 2024
0b582b8
adjust timing on modal tests
Elijbet Sep 26, 2024
7afdb64
remove closable from panel so it doesn't prevent shell from closing
Elijbet Sep 26, 2024
d8f34c5
this.effectiveReferenceElement
Elijbet Sep 26, 2024
87f4f56
cleanup
Elijbet Sep 27, 2024
b645dea
remove redundant connectFocusTrap(this);
Elijbet Sep 30, 2024
353ec39
sub waiting for timeout to waiting for event
Elijbet Oct 1, 2024
79ddd20
seperate stackedFocusTrap.e2e.ts
Elijbet Oct 1, 2024
337342b
cleanup stackedFocusTrap test
Elijbet Oct 1, 2024
212f6bc
clean up test
Elijbet Oct 1, 2024
0f8c4e2
assert on focus component
Elijbet Oct 2, 2024
fd53ff4
remove redundant components from modal, add dialog, account for focus…
Elijbet Oct 2, 2024
ee5d656
add camelCase util and target sheet-button id instead of sheet as it …
Elijbet Oct 3, 2024
7e3dda8
tab out of focus-retaining input components to assert on focus return…
Elijbet Oct 3, 2024
06e68ff
add comments to focusTrap options and cleanu up tests
Elijbet Oct 3, 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 @@ -853,6 +853,11 @@ export class InputDatePicker
this.datePickerEl.reset();
}

/** Leverage the `focus-trap` builtin stack to handle closing a sequence of open components, instead of components handling own `escape`. */
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
onFocusTrapDeactivate(): void {
this.open = false;
}

setStartInput = (el: HTMLCalciteInputElement): void => {
this.startInput = el;
};
Expand Down Expand Up @@ -932,10 +937,6 @@ export class InputDatePicker
this.open = true;
this.focusOnOpen = true;
event.preventDefault();
} else if (key === "Escape") {
this.open = false;
event.preventDefault();
jcfranco marked this conversation as resolved.
Show resolved Hide resolved
this.restoreInputFocus();
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,12 @@ export class InputTimePicker
this.calciteInputTimePickerClose.emit();
}

/** Leverage the `focus-trap` builtin stack to handle closing a sequence of open components, instead of components handling own `escape`. */
onFocusTrapDeactivate(): void {
this.open = false;
this.calciteInputEl.setFocus();
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
}

private delocalizeTimeString(value: string): string {
// we need to set the corresponding locale before parsing, otherwise it defaults to English (possible dayjs bug)
dayjs.locale(this.effectiveLocale.toLowerCase());
Expand Down Expand Up @@ -716,10 +722,6 @@ export class InputTimePicker
this.open = true;
this.focusOnOpen = true;
event.preventDefault();
} else if (key === "Escape" && this.open) {
this.open = false;
event.preventDefault();
this.calciteInputEl.setFocus();
}
};

Expand Down
20 changes: 5 additions & 15 deletions packages/calcite-components/src/components/modal/modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
EventEmitter,
h,
Host,
Listen,
Method,
Prop,
State,
Expand Down Expand Up @@ -393,20 +392,6 @@ export class Modal

@State() defaultMessages: ModalMessages;

//--------------------------------------------------------------------------
//
// Event Listeners
//
//--------------------------------------------------------------------------

@Listen("keydown", { target: "window" })
handleEscape(event: KeyboardEvent): void {
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
if (this.open && !this.escapeDisabled && event.key === "Escape" && !event.defaultPrevented) {
this.open = false;
event.preventDefault();
}
}

//--------------------------------------------------------------------------
//
// Events
Expand Down Expand Up @@ -498,6 +483,11 @@ export class Modal
deactivateFocusTrap(this);
}

/** Leverage the `focus-trap` builtin stack to handle closing a sequence of open components, instead of components handling own `escape`. */
onFocusTrapDeactivate(): void {
this.open = false;
}

@Watch("open")
toggleModal(value: boolean): void {
if (this.ignoreOpenChange) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,11 @@ export class Popover
deactivateFocusTrap(this);
}

/** Leverage the `focus-trap` builtin stack to handle closing a sequence of open components, instead of components handling own `escape`. */
onFocusTrapDeactivate(): void {
this.open = false;
}

storeArrowEl = (el: SVGElement): void => {
this.arrowEl = el;
this.reposition(true);
Expand Down
142 changes: 141 additions & 1 deletion packages/calcite-components/src/components/sheet/sheet.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { newE2EPage } from "@stencil/core/testing";
import { E2EElement, newE2EPage, E2EPage } from "@stencil/core/testing";
import { html } from "../../../support/formatting";
import { focusable, renders, hidden, defaults, accessible } from "../../tests/commonTests";
import { GlobalTestProps, newProgrammaticE2EPage, skipAnimations } from "../../tests/utils";
Expand Down Expand Up @@ -541,4 +541,144 @@ describe("calcite-sheet properties", () => {
expect(closeSpy).toHaveReceivedEventTimes(1);
});
});

const componentStack = html`
<calcite-sheet id="example-sheet" label="libero nunc" position="inline-start" display-mode="overlay">
<calcite-panel closable>
<calcite-block open heading="Preview Sheet options"> </calcite-block>
<calcite-button onClick="openComponent('example-modal')"> Open Modal from Sheet</calcite-button>
</calcite-panel>
</calcite-sheet>

<calcite-modal id="example-modal">
<div slot="content">
<p>This is an example modal that opens from a Sheet.</p>
</div>
<calcite-button slot="back" width="full" onClick="openComponent('another-modal')"
>Open Another Modal</calcite-button
>
</calcite-modal>

<calcite-modal id="another-modal">
<div slot="content" style="display: flex; flex-direction: column; gap: 12px">
<p>
This is an example of a another modal that opens from a modal. This modal an input date picker, a combobox, a
dropdown, a popover and a tooltip.
</p>
<calcite-label>
Input Date Picker
<calcite-input-date-picker value="2023-03-07" id="input-date-picker"></calcite-input-date-picker>
</calcite-label>
<calcite-label>
Input Time Picker
<calcite-input-time-picker name="calcite-input-time-picker"></calcite-input-time-picker>
</calcite-label>
<calcite-combobox
label="test"
placeholder="placeholder"
max-items="8"
selection-mode="ancestors"
style="width: 200px"
id="combobox"
>
<calcite-combobox-item value="Grand 1" text-label="Grand 1">
<calcite-combobox-item value="Parent 1" text-label="Parent 1">
<calcite-combobox-item value="Child 1" text-label="Child 1"></calcite-combobox-item>
<calcite-combobox-item value="Child 2" text-label="Child 2"></calcite-combobox-item>
</calcite-combobox-item>
</calcite-combobox-item>
</calcite-combobox>
<calcite-dropdown scale="s" width-scale="s" id="dropdown">
<calcite-button icon-end="hamburger" appearance="outline" slot="trigger">Scale S</calcite-button>
<calcite-dropdown-group group-title="View">
<calcite-dropdown-item icon-end="list-bullet" selected>List</calcite-dropdown-item>
<calcite-dropdown-item icon-end="grid">Grid</calcite-dropdown-item>
</calcite-dropdown-group>
</calcite-dropdown>
<calcite-popover
heading="Heading"
label="right end popover"
reference-element="popover-button"
placement="right-end"
id="popover-heading"
closable
style="width: 25vw"
id="popover"
>
<div style="padding: 0.5rem 1rem 0.75rem">
<p style="margin-top: 0">Example Popover.</p>
</div>
</calcite-popover>
<calcite-button appearance="outline" id="popover-button" icon-start="popup">Example Popover</calcite-button>
<calcite-tooltip placement="auto" reference-element="tooltip-auto-ref"> Example Tooltip </calcite-tooltip>
<calcite-button appearance="outline" id="tooltip-auto-ref">auto</calcite-button>
</div>
</calcite-modal>
<calcite-button onClick="openComponent('example-sheet')"> Open Sheet </calcite-button>
`;

it("closes a stack of open components sequentially in visual order", async () => {
const page = await newE2EPage();
await page.setContent(componentStack);
await skipAnimations(page);

async function openAndCheckVisibility(page: E2EPage, element: E2EElement) {
element.setProperty("open", true);
await page.waitForChanges();
expect(await element.isVisible()).toBe(true);
}

const sheet = await page.find("calcite-sheet");
await openAndCheckVisibility(page, sheet);

const firstModal = await page.find("#example-modal");
await openAndCheckVisibility(page, firstModal);

const secondModal = await page.find("#another-modal");
await openAndCheckVisibility(page, secondModal);

async function testInputPicker(page: E2EPage, pickerSelector: string, modal: E2EElement) {
const inputPicker = await page.find(pickerSelector);
inputPicker.click();
await page.waitForChanges();
expect(await inputPicker.getProperty("open")).toBe(true);

await page.keyboard.press("Escape");
await page.waitForChanges();
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
expect(await inputPicker.getProperty("open")).toBe(false);

await page.keyboard.press("Escape");
await page.waitForChanges();
expect(await modal.isVisible()).toBe(false);

modal.setProperty("open", true);
await page.waitForChanges();
}

await testInputPicker(page, "calcite-input-date-picker", secondModal);
await testInputPicker(page, "calcite-input-time-picker", secondModal);

secondModal.setProperty("open", true);
await page.waitForChanges();

const popoverButton = await page.find("#popover-button");
popoverButton.click();
await page.waitForChanges();
const popover = await page.find("calcite-popover");
expect(await popover.getProperty("open")).toBe(true);

await page.keyboard.press("Escape");
await page.waitForChanges();
expect(await popover.getProperty("open")).toBe(false);

async function pressEscapeAndCheckVisibility(page: E2EPage, element: E2EElement, expectedVisibility: boolean) {
page.keyboard.press("Escape");
await page.waitForChanges();
expect(await element.isVisible()).toBe(expectedVisibility);
}
Elijbet marked this conversation as resolved.
Show resolved Hide resolved

await pressEscapeAndCheckVisibility(page, secondModal, false);
await pressEscapeAndCheckVisibility(page, firstModal, false);
await pressEscapeAndCheckVisibility(page, sheet, false);
});
});
20 changes: 5 additions & 15 deletions packages/calcite-components/src/components/sheet/sheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
EventEmitter,
h,
Host,
Listen,
Method,
Prop,
VNode,
Expand Down Expand Up @@ -218,20 +217,6 @@ export class Sheet implements OpenCloseComponent, FocusTrapComponent, LoadableCo
this.handleMutationObserver(),
);

//--------------------------------------------------------------------------
//
// Event Listeners
//
//--------------------------------------------------------------------------

@Listen("keydown", { target: "window" })
handleEscape(event: KeyboardEvent): void {
if (this.open && !this.escapeDisabled && event.key === "Escape" && !event.defaultPrevented) {
this.open = false;
event.preventDefault();
}
}

//--------------------------------------------------------------------------
//
// Events
Expand Down Expand Up @@ -298,6 +283,11 @@ export class Sheet implements OpenCloseComponent, FocusTrapComponent, LoadableCo
deactivateFocusTrap(this);
}

/** Leverage the `focus-trap` builtin stack to handle closing a sequence of open components, instead of components handling own `escape`. */
onFocusTrapDeactivate(): void {
this.open = false;
}

private setTransitionEl = (el: HTMLDivElement): void => {
this.transitionEl = el;
this.contentId = ensureId(el);
Expand Down
85 changes: 84 additions & 1 deletion packages/calcite-components/src/demos/sheet.html
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@

<body>
<demo-dom-swapper>
<h1 style="margin: 0 auto; text-align: center">Sheet</h1>
<!-- <h1 style="margin: 0 auto; text-align: center">Sheet</h1>
Elijbet marked this conversation as resolved.
Show resolved Hide resolved

<calcite-sheet
width-scale="l"
Expand Down Expand Up @@ -124,6 +124,89 @@ <h1 style="margin: 0 auto; text-align: center">Sheet</h1>
document.querySelectorAll("calcite-sheet").forEach((sheet) => (sheet.open = false));
document.querySelectorAll("calcite-panel").forEach((panel) => (panel.closed = false));
});
</script> -->

<calcite-sheet id="example-sheet" label="libero nunc" position="inline-start" display-mode="overlay">
<calcite-panel closable>
<calcite-block open heading="Preview Sheet options"> </calcite-block>
<calcite-button onClick="openComponent('example-modal')"> Open Modal from Sheet</calcite-button>
</calcite-panel>
</calcite-sheet>

<calcite-modal id="example-modal">
<div slot="content">
<p>This is an example modal that opens from a Sheet.</p>
</div>
<calcite-button slot="back" width="full" onClick="openComponent('another-modal')"
>Open Another Modal</calcite-button
>
</calcite-modal>

<calcite-modal id="another-modal">
<div slot="content" style="display: flex; flex-direction: column; gap: 12px">
<p>
This is an example of a another modal that opens from a modal. This modal an input date picker, a combobox,
a dropdown, a popover and a tooltip.
</p>
<calcite-label>
Input Date Picker
<calcite-input-date-picker value="2023-03-07" id="input-date-picker"></calcite-input-date-picker>
</calcite-label>
<calcite-label>
Input Time Picker
<calcite-input-time-picker name="calcite-input-time-picker"></calcite-input-time-picker>
</calcite-label>
<calcite-combobox
label="test"
placeholder="placeholder"
max-items="8"
selection-mode="ancestors"
style="width: 200px"
id="combobox"
>
<calcite-combobox-item value="Grand 1" text-label="Grand 1">
<calcite-combobox-item value="Parent 1" text-label="Parent 1">
<calcite-combobox-item value="Child 1" text-label="Child 1"></calcite-combobox-item>
<calcite-combobox-item value="Child 2" text-label="Child 2"></calcite-combobox-item>
</calcite-combobox-item>
</calcite-combobox-item>
</calcite-combobox>
<calcite-dropdown scale="s" width-scale="s" id="dropdown">
<calcite-button icon-end="hamburger" appearance="outline" slot="trigger">Scale S</calcite-button>
<calcite-dropdown-group group-title="View">
<calcite-dropdown-item icon-end="list-bullet" selected>List</calcite-dropdown-item>
<calcite-dropdown-item icon-end="grid">Grid</calcite-dropdown-item>
</calcite-dropdown-group>
</calcite-dropdown>
<calcite-popover
heading="Heading"
label="right end popover"
reference-element="heading-title-content-cta"
placement="right-end"
id="popover-heading"
closable
style="width: 25vw"
id="popover"
>
<div style="padding: 0.5rem 1rem 0.75rem">
<p style="margin-top: 0">Example Popover.</p>
</div>
</calcite-popover>
<calcite-button appearance="outline" id="heading-title-content-cta" icon-start="popup"
>Example Popover</calcite-button
>
<calcite-tooltip placement="auto" reference-element="tooltip-auto-ref"> Example Tooltip </calcite-tooltip>
<calcite-button appearance="outline" id="tooltip-auto-ref">auto</calcite-button>
</div>
</calcite-modal>

<calcite-button onClick="openComponent('example-sheet')"> Open Sheet </calcite-button>

<script>
function openComponent(id) {
const component = document.getElementById(id);
component.open = true;
}
</script>
</demo-dom-swapper>
</body>
Expand Down
Loading
Loading