From a5898912bb67471bacda2337a810de54f11a031b Mon Sep 17 00:00:00 2001 From: James Nestor <87858909+NotNestor@users.noreply.github.com> Date: Mon, 20 Jan 2025 14:00:51 +0000 Subject: [PATCH] fix: input clear button focus trap (#1864) * fix: input clear button focus trap --- web-components/.storybook/themeDecorator.js | 2 +- web-components/package.json | 2 +- .../src/components/datepicker/DatePicker.ts | 6 +-- .../src/components/input/Input.test.ts | 46 +++++++++++++++++++ web-components/src/components/input/Input.ts | 11 ++++- .../components/menu-overlay/MenuOverlay.ts | 2 +- .../src/components/modal/scss/modal.scss | 2 +- 7 files changed, 62 insertions(+), 9 deletions(-) diff --git a/web-components/.storybook/themeDecorator.js b/web-components/.storybook/themeDecorator.js index 837ea2f9cc..56e2c0cdae 100644 --- a/web-components/.storybook/themeDecorator.js +++ b/web-components/.storybook/themeDecorator.js @@ -9,5 +9,5 @@ export const withThemeDecorator = (story, context) => { themeManager.setDarkMode(isDark); themeManager.setThemeName(theme); - return html` ${story()} `; + return html` ${story()} `; }; diff --git a/web-components/package.json b/web-components/package.json index d15a141bca..d2b2db9468 100644 --- a/web-components/package.json +++ b/web-components/package.json @@ -1,6 +1,6 @@ { "name": "@momentum-ui/web-components", - "version": "2.16.17", + "version": "2.16.18", "author": "Yana Harris", "license": "MIT", "repository": "https://github.com/momentum-design/momentum-ui.git", diff --git a/web-components/src/components/datepicker/DatePicker.ts b/web-components/src/components/datepicker/DatePicker.ts index e7acb73acb..7c6d67d4f0 100644 --- a/web-components/src/components/datepicker/DatePicker.ts +++ b/web-components/src/components/datepicker/DatePicker.ts @@ -214,7 +214,7 @@ export namespace DatePicker { : undefined; }; - isValueValid = () => { + isValueValid = (): boolean => { if (!this.value && this.value !== EMPTY_STRING) return true; const dateRangePicker = closestElement("md-date-range-picker", this) as DateRangePicker.ELEMENT; const regexString = @@ -227,7 +227,7 @@ export namespace DatePicker { const filters: DayFilters = { maxDate: this.maxDateData, minDate: this.minDateData, filterDate: this.filterDate }; const isValid = - this.value && + !!this.value && regex.test(this.value) && !isDayDisabled(DateTime.fromISO(this.value, { locale: this.locale }), filters); @@ -261,7 +261,7 @@ export namespace DatePicker { @keydown=${(event: KeyboardEvent) => this.handleInputKeyDown(event)} @input-change="${(e: CustomEvent) => this.handleDateInputChange(e)}" ?disabled=${this.disabled} - hide-message=${!this.errorMessage && this.isValueValid()} + ?hide-message=${!this.errorMessage && this.isValueValid()} ariaInvalid=${!!this.errorMessage || !this.isValueValid()} .messageArr=${this.errorMessage ? [{ message: this.errorMessage, type: "error" }] diff --git a/web-components/src/components/input/Input.test.ts b/web-components/src/components/input/Input.test.ts index ecf00cd6ec..5d179812e4 100644 --- a/web-components/src/components/input/Input.test.ts +++ b/web-components/src/components/input/Input.test.ts @@ -344,4 +344,50 @@ describe("Input Component", () => { const rightTemplate = element.shadowRoot!.querySelector(".md-input__after")?.querySelector("md-button"); expect(rightTemplate).toBeNull(); }); + + test("Should propogate tab key event", async () => { + //Test case to ensure issue where the clear button became a focus trap does not reoccur + const el = await fixture(html` `); + + const input = el.shadowRoot!.querySelector("input") as HTMLInputElement; + const clearButton = el.shadowRoot!.querySelector(".md-input__icon-clear") as HTMLElement; + + // Focus the input and then the clear button + input.focus(); + clearButton.focus(); + + // Simulate Tab key press + const tabEvent = new KeyboardEvent("keydown", { + key: "Tab", + bubbles: true, + cancelable: true + }); + clearButton.dispatchEvent(tabEvent); + + // Check if the event was propagated and focus moved to the next element + expect(document.activeElement).not.toBe(clearButton); + }); + + test("Clicking clear button should not propogate", async () => { + //The focus trap bug was caused by not wanting to propogate the click event + //Test that this still works + const el = await fixture(html` `); + + const input = el.shadowRoot!.querySelector("input") as HTMLInputElement; + const clearButton = el.shadowRoot!.querySelector(".md-input__icon-clear") as HTMLElement; + + // Focus the input and then the clear button + input.focus(); + clearButton.focus(); + + // Simulate Click event + const clickEvent = new MouseEvent("click", { + bubbles: true, + cancelable: true + }); + clearButton.dispatchEvent(clickEvent); + + // Check if the event was propagated and focus moved to the next element + expect(document.activeElement).not.toBe(clearButton); + }); }); diff --git a/web-components/src/components/input/Input.ts b/web-components/src/components/input/Input.ts index ea796d021c..6e258180ca 100644 --- a/web-components/src/components/input/Input.ts +++ b/web-components/src/components/input/Input.ts @@ -293,14 +293,21 @@ export namespace Input { } handleClear(event: MouseEvent | KeyboardEvent) { - event.preventDefault(); - event.stopPropagation(); if (event.type === "keydown") { const { code } = event as KeyboardEvent; if (code !== Key.Space && code !== Key.Enter) { return; } + + event.preventDefault(); + event.stopPropagation(); + } else if (event.type === "click") { + //When handling the click clear button do not propagate the event to the parent + //As this will close overlay menus that we do not want to close + event.preventDefault(); + event.stopPropagation(); } + this.input.focus(); this.dispatchEvent( new CustomEvent("input-clear", { diff --git a/web-components/src/components/menu-overlay/MenuOverlay.ts b/web-components/src/components/menu-overlay/MenuOverlay.ts index 4884fe640a..e19b3729b7 100644 --- a/web-components/src/components/menu-overlay/MenuOverlay.ts +++ b/web-components/src/components/menu-overlay/MenuOverlay.ts @@ -167,7 +167,7 @@ export namespace MenuOverlay { } checkIsInputField(element: HTMLElement) { - if (element && element.tagName && element.tagName.toLowerCase() === "md-input") { + if (element?.tagName?.toLowerCase() === "md-input") { return true; } return false; diff --git a/web-components/src/components/modal/scss/modal.scss b/web-components/src/components/modal/scss/modal.scss index 1ccdd61fd3..6da7ea063b 100644 --- a/web-components/src/components/modal/scss/modal.scss +++ b/web-components/src/components/modal/scss/modal.scss @@ -31,7 +31,7 @@ h2 { font-size: inherit; - font-weight: inherit; + font-weight: var(--brand-font-weight-bold, inherit); line-height: inherit; margin: 0; }