fix(dropdown): keep checkbox trigger semantics and open on Enter #669

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<HTMLElement | null>(null);
 
+  /** Only a Space press that starts on the trigger may activate it on keyup. */
+  private spacePressed = false;
+
   private readonly hasPopup = computed<DropdownTriggerAriaHasPopup>(() => {
     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<HTMLElement>(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() {

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: `
+    <tedi-dropdown>
+      <input
+        data-testid="checkbox"
+        tedi-checkbox
+        tedi-dropdown-trigger
+        type="checkbox"
+        aria-label="Select rows"
+        (click)="$event.preventDefault()"
+      />
+      <tedi-dropdown-content dropdownRole="menu">
+        <li tedi-dropdown-item>Select page</li>
+      </tedi-dropdown-content>
+    </tedi-dropdown>
+
+    <tedi-dropdown>
+      <div data-testid="generic" tedi-dropdown-trigger>Trigger</div>
+      <tedi-dropdown-content dropdownRole="menu">
+        <li tedi-dropdown-item>Item</li>
+      </tedi-dropdown-content>
+    </tedi-dropdown>
+
+    <tedi-dropdown>
+      <div
+        data-testid="custom-role"
+        tedi-dropdown-trigger
+        role="checkbox"
+        aria-checked="mixed"
+        tabindex="-1"
+      >
+        Trigger
+      </div>
+      <tedi-dropdown-content dropdownRole="menu">
+        <li tedi-dropdown-item>Item</li>
+      </tedi-dropdown-content>
+    </tedi-dropdown>
+
+    <tedi-dropdown>
+      <input data-testid="text" tedi-dropdown-trigger type="text" />
+      <tedi-dropdown-content dropdownRole="listbox">
+        <li tedi-dropdown-item value="a">Item</li>
+      </tedi-dropdown-content>
+    </tedi-dropdown>
+  `,
+  imports: [
+    DropdownComponent,
+    DropdownTriggerDirective,
+    DropdownContentComponent,
+    DropdownItemComponent,
+    CheckboxComponent,
+  ],
+})
+class NonButtonTriggerHostComponent {}
+
 describe("DropdownComponent", () => {
   let fixture: ComponentFixture<TestHostComponent>;
   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<NonButtonTriggerHostComponent>;
+
+    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<InteractiveContentHostComponent>;
     let interactiveDropdown: DropdownComponent;