Skip to content

Commit

Permalink
fix: input clear button focus trap (#1864)
Browse files Browse the repository at this point in the history
* fix: input clear button focus trap
  • Loading branch information
NotNestor authored Jan 20, 2025
1 parent 02da252 commit a589891
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 9 deletions.
2 changes: 1 addition & 1 deletion web-components/.storybook/themeDecorator.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ export const withThemeDecorator = (story, context) => {
themeManager.setDarkMode(isDark);
themeManager.setThemeName(theme);

return html` <md-theme class="theme-toggle" ?darkTheme=${isDark} theme=${theme}> ${story()} </md-theme>`;
return html` <md-theme class="theme-toggle" ?darkTheme=${themeManager.isDarkMode} theme=${themeManager.themeName}> ${story()} </md-theme>`;
};
2 changes: 1 addition & 1 deletion web-components/package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
6 changes: 3 additions & 3 deletions web-components/src/components/datepicker/DatePicker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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);

Expand Down Expand Up @@ -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" }]
Expand Down
46 changes: 46 additions & 0 deletions web-components/src/components/input/Input.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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` <md-input clear value="test value"></md-input> `);

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` <md-input clear value="test value"></md-input> `);

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);
});
});
11 changes: 9 additions & 2 deletions web-components/src/components/input/Input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", {
Expand Down
2 changes: 1 addition & 1 deletion web-components/src/components/menu-overlay/MenuOverlay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion web-components/src/components/modal/scss/modal.scss
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@

h2 {
font-size: inherit;
font-weight: inherit;
font-weight: var(--brand-font-weight-bold, inherit);
line-height: inherit;
margin: 0;
}
Expand Down

0 comments on commit a589891

Please sign in to comment.