From 00c8e528602b3f322da6eb304fef8b5fd7f96a8d Mon Sep 17 00:00:00 2001 From: Rando Leppik Date: Wed, 23 Sep 2026 13:25:41 +0300 Subject: [PATCH] fix(dropdown): keep checkbox trigger semantics and open on Enter #669 --- .../dropdown-trigger.directive.ts | 82 ++++++- .../dropdown/dropdown.component.spec.ts | 228 ++++++++++++++++++ .../overlay/dropdown/dropdown.stories.ts | 42 ++++ 3 files changed, 341 insertions(+), 11 deletions(-) diff --git a/src/tedi/components/overlay/dropdown/dropdown-trigger/dropdown-trigger.directive.ts b/src/tedi/components/overlay/dropdown/dropdown-trigger/dropdown-trigger.directive.ts index dcd66e015..8814a2c45 100644 --- a/src/tedi/components/overlay/dropdown/dropdown-trigger/dropdown-trigger.directive.ts +++ b/src/tedi/components/overlay/dropdown/dropdown-trigger/dropdown-trigger.directive.ts @@ -16,7 +16,14 @@ import { DropdownComponent } from "../dropdown.component"; export type DropdownTriggerAriaHasPopup = "menu" | "listbox" | "dialog" | "true" | "false"; -const FOCUSABLE_SELECTOR = "button, a[href], [tabindex]"; +/** Native controls keep their own semantics and keyboard behavior. */ +const NATIVE_INTERACTIVE_SELECTOR = + 'button, a[href], input:not([type="hidden"]), select, textarea'; + +const FOCUSABLE_SELECTOR = `${NATIVE_INTERACTIVE_SELECTOR}, [tabindex]`; + +/** Native controls that fire `click` on Space but not on Enter. */ +const SPACE_ACTIVATED_SELECTOR = 'input[type="checkbox"], input[type="radio"]'; @Directive({ standalone: true, @@ -48,6 +55,9 @@ export class DropdownTriggerDirective implements AfterViewInit { */ private readonly triggerElement = signal(null); + /** Only a Space press that starts on the trigger may activate it on keyup. */ + private spacePressed = false; + private readonly hasPopup = computed(() => { const explicit = this.ariaHaspopup(); if (explicit) return explicit; @@ -97,16 +107,24 @@ export class DropdownTriggerDirective implements AfterViewInit { "aria-expanded", String(this.dropdown.isOpen()), ); + }); + } + + ngAfterViewInit() { + const el = this.resolveTriggerElement(); - if (!this.isNativelyFocusable(el)) { + // Non-native triggers need button semantics, but a role or tabindex the + // consumer set (e.g. `role="checkbox"`) is theirs to keep. + if (!this.isNativelyInteractive(el)) { + if (!el.hasAttribute("role")) { this.renderer.setAttribute(el, "role", "button"); + } + if (!el.hasAttribute("tabindex")) { this.renderer.setAttribute(el, "tabindex", "0"); } - }); - } + } - ngAfterViewInit() { - this.triggerElement.set(this.resolveTriggerElement()); + this.triggerElement.set(el); } focus() { @@ -129,6 +147,20 @@ export class DropdownTriggerDirective implements AfterViewInit { const key = event.key; switch (key) { + case "Enter": + if (!this.handlesKey(event, key)) break; + // Also keeps Enter on a checkbox from submitting its form. + event.preventDefault(); + this.dropdown.toggleDropdown(); + break; + + case " ": + if (!this.handlesKey(event, key)) break; + // Match button timing and prevent page scrolling. + event.preventDefault(); + this.spacePressed = true; + break; + case "ArrowDown": if (!this.isWidgetContent()) break; event.preventDefault(); @@ -149,20 +181,48 @@ export class DropdownTriggerDirective implements AfterViewInit { } } + @HostListener("keyup", ["$event"]) + onKeyup(event: KeyboardEvent) { + if (event.key !== " " || !this.spacePressed) return; + + this.spacePressed = false; + if (event.target !== this.focusableElement) return; + this.dropdown.toggleDropdown(); + } + + @HostListener("focusout", ["$event"]) + onFocusout(event: FocusEvent) { + if (event.target === this.focusableElement) this.spacePressed = false; + } + + /** + * Whether the directive must activate the trigger on `key` itself, because the + * element does not turn that key into a native click: both keys for a generic + * trigger, Enter for a checkbox or radio. Other native controls handle both. + */ + private handlesKey(event: KeyboardEvent, key: "Enter" | " "): boolean { + const el = this.focusableElement; + + // Keys from other controls inside the host are theirs. + if (event.target !== el) return false; + + if (!this.isNativelyInteractive(el)) return true; + + return key === "Enter" && el.matches(SPACE_ACTIVATED_SELECTOR); + } + private isWidgetContent(): boolean { return this.dropdown.dropdownContent().isWidget(); } private resolveTriggerElement(): HTMLElement { const el = this.host.nativeElement; - if (this.isNativelyFocusable(el)) return el; + if (this.isNativelyInteractive(el)) return el; return el.querySelector(FOCUSABLE_SELECTOR) ?? el; } - private isNativelyFocusable(el: HTMLElement): boolean { - return ( - el.tagName === "BUTTON" || (el.tagName === "A" && el.hasAttribute("href")) - ); + private isNativelyInteractive(el: HTMLElement): boolean { + return el.matches(NATIVE_INTERACTIVE_SELECTOR); } private openAndFocusFirst() { diff --git a/src/tedi/components/overlay/dropdown/dropdown.component.spec.ts b/src/tedi/components/overlay/dropdown/dropdown.component.spec.ts index a6da0df71..1895f575e 100644 --- a/src/tedi/components/overlay/dropdown/dropdown.component.spec.ts +++ b/src/tedi/components/overlay/dropdown/dropdown.component.spec.ts @@ -17,6 +17,7 @@ import { DropdownContentComponent } from "./dropdown-content/dropdown-content.co import { DropdownRole } from "./dropdown.tokens"; import { DropdownItemComponent } from "./dropdown-item/dropdown-item.component"; import { getFocusableElements } from "../../../utils/elements.util"; +import { CheckboxComponent } from "../../form/checkbox/checkbox.component"; @Component({ standalone: true, @@ -151,6 +152,62 @@ class InteractiveListboxHostComponent {} }) class ListRoleHostComponent {} +@Component({ + standalone: true, + template: ` + + + +
  • Select page
    • +     +     + + +
    Trigger
    + +
  • Item
    • +     +     + + +
    + Trigger +
    + +
  • Item
    • +     +     + + + + +
  • Item
    • +     +     + `, + imports: [ + DropdownComponent, + DropdownTriggerDirective, + DropdownContentComponent, + DropdownItemComponent, + CheckboxComponent, + ], +}) +class NonButtonTriggerHostComponent {} + describe("DropdownComponent", () => { let fixture: ComponentFixture; let host: TestHostComponent; @@ -832,6 +889,19 @@ describe("DropdownComponent", () => { expect(focusSpy).toHaveBeenCalled(); }); + it("does not synthesize keyboard clicks for a native button", () => { + const spy = jest.spyOn(dropdown, "toggleDropdown"); + + for (const key of ["Enter", " "]) { + const event = new KeyboardEvent("keydown", { key, cancelable: true }); + trigger.dispatchEvent(event); + trigger.dispatchEvent(new KeyboardEvent("keyup", { key })); + expect(event.defaultPrevented).toBe(false); + } + + expect(spy).not.toHaveBeenCalled(); + }); + it("Escape should hide dropdown and return focus to trigger", () => { openDropdown(); @@ -942,6 +1012,164 @@ describe("DropdownComponent", () => { }); }); + describe("DropdownTriggerDirective on a non-button host", () => { + let nbFixture: ComponentFixture; + + beforeEach(() => { + nbFixture = TestBed.createComponent(NonButtonTriggerHostComponent); + nbFixture.detectChanges(); + }); + + const el = (id: string) => + nbFixture.nativeElement.querySelector( + `[data-testid="${id}"]`, + ) as HTMLElement; + + const dropdownOf = (id: string) => + nbFixture.debugElement + .queryAll(By.directive(DropdownComponent)) + .map((d) => d.componentInstance as DropdownComponent) + .find((d) => d.dropdownTrigger().host.nativeElement === el(id))!; + + const press = (target: HTMLElement, type: string, key: string) => { + const event = new KeyboardEvent(type, { + key, + bubbles: true, + cancelable: true, + }); + target.dispatchEvent(event); + nbFixture.detectChanges(); + return event; + }; + + afterEach(() => { + nbFixture.debugElement + .queryAll(By.directive(DropdownComponent)) + .forEach((d) => + (d.componentInstance as DropdownComponent).hideDropdown(), + ); + }); + + describe("checkbox", () => { + it("keeps its native checkbox semantics", () => { + const checkbox = el("checkbox"); + + expect(checkbox.getAttribute("role")).toBeNull(); + expect(checkbox.getAttribute("tabindex")).toBeNull(); + expect(checkbox.getAttribute("aria-haspopup")).toBe("menu"); + expect(checkbox.getAttribute("aria-expanded")).toBe("false"); + }); + + it("handles Enter and prevents its native action", () => { + const checkbox = el("checkbox"); + const dropdown = dropdownOf("checkbox"); + + const event = press(checkbox, "keydown", "Enter"); + + expect(event.defaultPrevented).toBe(true); + expect(dropdown.isOpen()).toBe(true); + expect(checkbox.getAttribute("aria-expanded")).toBe("true"); + + press(checkbox, "keydown", "Enter"); + expect(dropdown.isOpen()).toBe(false); + }); + + it("leaves Space to the checkbox and opens on click", () => { + const checkbox = el("checkbox") as HTMLInputElement; + const dropdown = dropdownOf("checkbox"); + + const event = press(checkbox, "keydown", " "); + press(checkbox, "keyup", " "); + + expect(event.defaultPrevented).toBe(false); + expect(dropdown.isOpen()).toBe(false); + + checkbox.click(); + nbFixture.detectChanges(); + + expect(dropdown.isOpen()).toBe(true); + expect(checkbox.checked).toBe(false); + }); + }); + + describe("generic element", () => { + it("gets button semantics", () => { + expect(el("generic").getAttribute("role")).toBe("button"); + expect(el("generic").getAttribute("tabindex")).toBe("0"); + }); + + it("Enter toggles the dropdown", () => { + const dropdown = dropdownOf("generic"); + + press(el("generic"), "keydown", "Enter"); + expect(dropdown.isOpen()).toBe(true); + + press(el("generic"), "keydown", "Enter"); + expect(dropdown.isOpen()).toBe(false); + }); + + it("Space toggles the dropdown on keyup, like a native button", () => { + const dropdown = dropdownOf("generic"); + + const event = press(el("generic"), "keydown", " "); + expect(event.defaultPrevented).toBe(true); + expect(dropdown.isOpen()).toBe(false); + + press(el("generic"), "keyup", " "); + expect(dropdown.isOpen()).toBe(true); + }); + + it("ignores the keyup of a Space press that started elsewhere", () => { + const dropdown = dropdownOf("generic"); + + press(el("generic"), "keyup", " "); + + expect(dropdown.isOpen()).toBe(false); + }); + + it("does not activate when Space is released on a child", () => { + const trigger = el("generic"); + const child = document.createElement("span"); + trigger.append(child); + + press(trigger, "keydown", " "); + press(child, "keyup", " "); + + expect(dropdownOf("generic").isOpen()).toBe(false); + }); + + it("forgets a Space press when focus leaves the trigger", () => { + const trigger = el("generic"); + + press(trigger, "keydown", " "); + trigger.dispatchEvent(new FocusEvent("focusout", { bubbles: true })); + press(trigger, "keyup", " "); + + expect(dropdownOf("generic").isOpen()).toBe(false); + }); + }); + + it("keeps a role and tabindex the consumer set", () => { + expect(el("custom-role").getAttribute("role")).toBe("checkbox"); + expect(el("custom-role").getAttribute("tabindex")).toBe("-1"); + }); + + it("leaves Enter and Space to a text field", () => { + const input = el("text"); + const dropdown = dropdownOf("text"); + + expect(input.getAttribute("role")).toBeNull(); + + const enter = press(input, "keydown", "Enter"); + const space = press(input, "keydown", " "); + press(input, "keyup", " "); + + expect(enter.defaultPrevented).toBe(false); + expect(space.defaultPrevented).toBe(false); + expect(dropdown.isOpen()).toBe(false); + }); + }); + describe("interactiveContent items", () => { let interactiveFixture: ComponentFixture; let interactiveDropdown: DropdownComponent; diff --git a/src/tedi/components/overlay/dropdown/dropdown.stories.ts b/src/tedi/components/overlay/dropdown/dropdown.stories.ts index 6816abacb..1907a751c 100644 --- a/src/tedi/components/overlay/dropdown/dropdown.stories.ts +++ b/src/tedi/components/overlay/dropdown/dropdown.stories.ts @@ -15,6 +15,7 @@ import { DropdownItemValueLabelComponent } from "./dropdown-item-value/dropdown- import { DropdownItemValueMetaComponent } from "./dropdown-item-value/dropdown-item-value-meta.component"; import { ButtonComponent } from "../../buttons/button/button.component"; import { IconComponent } from "../../base"; +import { CheckboxComponent } from "../../form/checkbox/checkbox.component"; import { expect, userEvent, waitFor, within } from "storybook/test"; @Component({ @@ -65,6 +66,7 @@ export default { ButtonComponent, IconComponent, DemoWrappingButtonComponent, + CheckboxComponent, ], }), ], @@ -373,6 +375,46 @@ export const WithWrappingButtonComponent: Story = { }), }; +/** + * A checkbox trigger keeps its native role and checked or mixed state. Enter + * opens the menu; Space uses the checkbox's native click. The click is cancelled + * so selection changes only through menu actions. + */ +export const WithCheckboxTrigger: Story = { + name: "Checkbox Trigger", + args: { + position: "bottom-start", + preventOverflow: true, + }, + render: (args) => ({ + props: { + ...args, + selection: { count: 3 }, + total: 10, + pageSize: 5, + }, + template: ` + + + +
  • Select page
    • +
  • Select all
    • +
  • Clear selection
    • +     +     +

    {{ selection.count }} of {{ total }} rows selected

    + `, + }), +}; + /** * Items with `[closeOnSelect]="false"` keep the dropdown open after activation * and emit `(itemSelect)` for both mouse click and keyboard (Enter / Space).