Improve keyboard handling and accessibility for Select.svelte and refactor (#2811)

* resolve TagAddButton a11y
better comments to document tagindex reasoning

* resolved a11y for TagsSelectedButton
allow focus to TagsSelectedButton with Shift+Tab and Enter or Space to show popover

* safely ignore a11y warning as container for interactables is not itself interactable

* Update CONTRIBUTORS

* quick fix syntax

* quick fix syntax

* quick fix syntax

* quick fix syntax

* resolved a11y in accordance with ARIA APG Disclure pattern

* resolved a11y
ideally should replace with  with
a11y-click-events-have-key-events is explicitly ignored as the alternative (adding ) seems more clunky

* resolved SpinBox a11y
cannot focus on these buttons, so no key event handling needed (keyboard editting already possible by just typing in the field)
widget already properly follows ARIA APG Spinbutton pattern

* cleanup

* onEnterOrSpace() function implemented as discussed in #2787 and #2564

* I think this is the main keyboard handling of Select
Still need to fix focus and handle roles and attributes

* fixed the keyboard interaction

focus is janky because you need to wait until after the listed options load and for some reason that needs a tiny delay on onMount
I think this technically violates a11y, but it really doesn't since the delay is literally zero. But the code still needs it to happen.

* Select and SelectOption reference the same focus function

* SelectOption moved inside Select
+ started roles and a11y

* quick syntax and such changes

* finish handling roles and attributes

* fixed keyboard handling and only visual focus

* cleanup and slight refactoring

* fixed syntax

* what even is this?

* bug fixes + revert key selection

* fixed scrolling

* better control scrolling and focus

* Adjusted selection
Up/Down Arrows: start selection on active option
Enter/Space/Click: no initial selection, down arrow to first option, up arrow to last option

* Only set selected the first time Select is opened, all subsequent times use the previous selected

ts/change-notetype/MapperRow.svelte

@@ -6,7 +6,6 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     import Col from "../components/Col.svelte";
     import Row from "../components/Row.svelte";
     import Select from "../components/Select.svelte";
-    import SelectOption from "../components/SelectOption.svelte";
     import type { ChangeNotetypeState, MapContext } from "./lib";
 
     export let state: ChangeNotetypeState;
@@ -26,11 +25,12 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
 <Row>
     <Col>
-        <Select value={oldIndex} {label} on:change={onChange}>
-            {#each $info.getOldNamesIncludingNothing(ctx) as name, idx}
-                <SelectOption value={idx}>{name}</SelectOption>
-            {/each}
-        </Select>
+        <Select
+            value={oldIndex}
+            {label}
+            list={$info.getOldNamesIncludingNothing(ctx)}
+            on:change={onChange}
+        />
     </Col>
     <Col>
         {$info.getNewName(ctx, newIndex)}

ts/change-notetype/NotetypeSelector.svelte

@@ -7,7 +7,6 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     import ButtonToolbar from "../components/ButtonToolbar.svelte";
     import LabelButton from "../components/LabelButton.svelte";
     import Select from "../components/Select.svelte";
-    import SelectOption from "../components/SelectOption.svelte";
     import { arrowLeftIcon, arrowRightIcon } from "./icons";
     import type { ChangeNotetypeState } from "./lib";
     import SaveButton from "./SaveButton.svelte";
@@ -34,11 +33,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
             {@html arrowRightIcon}
         {/if}
     </Badge>
-    <Select class="flex-grow-1" bind:value {label}>
-        {#each options as option, idx}
-            <SelectOption value={idx}>{option}</SelectOption>
-        {/each}
-    </Select>
+    <Select class="flex-grow-1" list={options} bind:value {label} />
 
     <SaveButton {state} />
 </ButtonToolbar>

ts/components/DropdownItem.svelte

@@ -4,6 +4,8 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 -->
 <script lang="ts">
     export let id: string | undefined = undefined;
+    export let role: string | undefined = undefined;
+    export let selected = false;
     let className = "";
     export { className as class };
 
@@ -29,6 +31,8 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 <button
     bind:this={buttonRef}
     {id}
+    {role}
+    aria-selected={selected}
     tabindex={tabbable ? 0 : -1}
     class="dropdown-item {className}"
     class:active
@@ -64,6 +68,11 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
             color: var(--highlight-fg);
         }
 
+        &.focus {
+            // TODO this is subtly different from hovering with the mouse for some reason
+            @extend button, :hover;
+        }
+
         &[disabled] {
             cursor: default;
             color: var(--fg-disabled);

ts/components/EnumSelector.svelte

@@ -11,7 +11,6 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
 <script lang="ts">
     import Select from "./Select.svelte";
-    import SelectOption from "./SelectOption.svelte";
 
     type T = $$Generic;
 
@@ -23,13 +22,14 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     $: label = choices.find((c) => c.value === value)?.label;
 </script>
 
-<Select bind:value {label} {disabled}>
-    {#each choices as { label: optionLabel, value: optionValue }}
-        <SelectOption
-            value={optionValue}
-            disabled={disabledChoices.includes(optionValue)}
-        >
-            {optionLabel}
-        </SelectOption>
-    {/each}
-</Select>
+<Select
+    bind:value
+    {label}
+    {disabled}
+    list={choices}
+    parser={(item) => ({
+        content: item.label,
+        value: item.value,
+        disabled: disabledChoices.includes(item.value),
+    })}
+/>

ts/components/Popover.svelte

@@ -4,19 +4,21 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 -->
 <script lang="ts">
     import type { Placement } from "@floating-ui/dom";
-    import { getContext, onMount } from "svelte";
+    import { createEventDispatcher, getContext, onMount } from "svelte";
     import type { Writable } from "svelte/store";
 
     import { floatingKey } from "./context-keys";
 
+    export let id = "";
     export let scrollable = false;
-    let element: HTMLDivElement;
     let wrapper: HTMLDivElement;
     let hidden = true;
     let minHeight = 0;
 
     let placement: Placement;
 
+    const dispatch = createEventDispatcher();
+
     const placementStore = getContext<Writable<Promise<Placement>>>(floatingKey);
 
     /* await computed placement of floating element to determine animation direction */
@@ -34,6 +36,9 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
            popover placement at animation start */
         minHeight = wrapper.offsetHeight;
     });
+    function revealed(el: HTMLElement) {
+        dispatch("revealed", el);
+    }
 </script>
 
 <div
@@ -49,7 +54,9 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
         class:right={placement === "right"}
         class:bottom={placement === "bottom"}
         class:left={placement === "left"}
-        bind:this={element}
+        use:revealed
+        {id}
+        role="listbox"
     >
         <slot />
     </div>

ts/components/Select.svelte

@@ -3,6 +3,7 @@ Copyright: Ankitects Pty Ltd and contributors
 License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 -->
 <script lang="ts">
+    import { altPressed, isArrowDown, isArrowUp } from "@tslib/keys";
     import { createEventDispatcher, setContext } from "svelte";
     import { writable } from "svelte/store";
 
@@ -10,13 +11,12 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     import IconConstrain from "./IconConstrain.svelte";
     import { chevronDown } from "./icons";
     import Popover from "./Popover.svelte";
+    import SelectOption from "./SelectOption.svelte";
     import WithFloating from "./WithFloating.svelte";
 
     // eslint-disable
     type T = $$Generic;
 
-    export let id: string | undefined = undefined;
-
     let className = "";
     export { className as class };
 
@@ -24,6 +24,41 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     export let label = "<br>";
     export let value: T;
 
+    // E may need to derive content, but we default to them being the same for convenience of usage
+    type E = $$Generic;
+    type C = $$Generic;
+    let selected: number | undefined = undefined;
+    let initialSelected: number;
+    export let list: E[];
+    export let parser: (item: E) => { content: C; value?: T; disabled?: boolean } = (
+        item,
+    ) => {
+        return {
+            content: item as unknown as C,
+        };
+    };
+    const parsed = list
+        .map(parser)
+        .map(({ content, value: initialValue, disabled = false }, i) => {
+            if ((initialValue === undefined && i === value) || initialValue === value) {
+                initialSelected = i;
+            }
+
+            return {
+                content,
+                parsedValue: initialValue === undefined ? (i as T) : initialValue,
+                disabled,
+            };
+        });
+    const buttons: HTMLButtonElement[] = Array(list.length);
+    const last = list.length - 1;
+    const ids = {
+        popover: "popover",
+        focused: "focused",
+    };
+
+    export let id: string | undefined = undefined;
+
     const dispatch = createEventDispatcher();
 
     function setValue(v: T) {
@@ -41,16 +76,108 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     let showFloating = false;
     let clientWidth: number;
 
-    async function handleKey(e: KeyboardEvent) {
-        if (e.code === "Enter") {
-            e.preventDefault();
-            showFloating = !showFloating;
-        }
-    }
-
     const selectStore = writable({ value, setValue });
     $: $selectStore.value = value;
     setContext(selectKey, selectStore);
+
+    function onKeyDown(event: KeyboardEvent) {
+        // In accordance with ARIA APG combobox (https://www.w3.org/WAI/ARIA/apg/patterns/combobox/)
+        const arrowDown = isArrowDown(event);
+        const arrowUp = isArrowUp(event);
+        const alt = altPressed(event);
+        if (arrowDown || arrowUp || event.code === "Space") {
+            event.preventDefault();
+        }
+
+        if (
+            !showFloating &&
+            ((arrowDown && alt) ||
+                event.code === "Enter" ||
+                event.code === "Space" ||
+                arrowDown ||
+                event.code === "Home" ||
+                arrowUp ||
+                event.code === "End")
+        ) {
+            showFloating = true;
+            if (selected === undefined) {
+                selected = initialSelected;
+            }
+            return;
+        }
+        if (selected === undefined) {
+            return;
+        }
+
+        if (
+            event.code === "Enter" ||
+            event.code === "Space" ||
+            event.code === "Tab" ||
+            (arrowUp && alt)
+        ) {
+            showFloating = false;
+            setValue(parsed[selected].parsedValue);
+        } else if (arrowUp) {
+            if (selected < 0) {
+                selected = last + 1;
+            }
+            selectFocus(selected - 1);
+        } else if (arrowDown) {
+            selectFocus(selected + 1);
+        } else if (event.code === "Escape") {
+            // TODO This doesn't work as the window typically catches the Escape as well
+            // and closes the window
+            // - qt/aqt/browser/browser.py:377
+            showFloating = false;
+        } else if (event.code === "Home") {
+            selectFocus(0);
+        } else if (event.code === "End") {
+            selectFocus(last);
+        }
+    }
+
+    function revealed() {
+        if (selected === undefined) {
+            return;
+        }
+        setTimeout(selectFocus, 0, selected);
+    }
+
+    /**
+     * Focus on an option.
+     * Values outside the range clip to either end
+     * @param num index number to focus on
+     */
+    function selectFocus(num: number) {
+        if (selected === -2) {
+            selected = -1;
+            return;
+        }
+        if (num < 0) {
+            num = 0;
+        } else if (num > last) {
+            num = last;
+        }
+
+        if (selected !== undefined && 0 <= selected && selected <= last) {
+            buttons[selected].classList.remove("focus");
+        }
+
+        if (num >= 0) {
+            const el = buttons[num];
+            el.classList.add("focus");
+            if (!isScrolledIntoView(el)) {
+                el.scrollIntoView();
+            }
+        }
+        selected = num;
+    }
+
+    function isScrolledIntoView(el: HTMLElement) {
+        // This could probably be a helper function of some sort, I don't know where to put it
+        const rect = el.getBoundingClientRect();
+        return rect.top >= 0 && rect.bottom <= window.innerHeight;
+    }
 </script>
 
 <WithFloating
@@ -64,6 +191,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     on:close={() => (showFloating = false)}
     let:asReference
 >
+    <!-- TODO implement aria-label with semantic label -->
     <div
         {id}
         class="{className} select-container"
@@ -72,10 +200,19 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
         class:disabled
         title={tooltip}
         tabindex="0"
-        on:keypress={handleKey}
+        role="combobox"
+        aria-controls={ids.popover}
+        aria-expanded={showFloating}
+        aria-activedescendant={ids.focused}
+        on:keydown={onKeyDown}
         on:mouseenter={() => (hover = true)}
         on:mouseleave={() => (hover = false)}
-        on:click={() => (showFloating = !showFloating)}
+        on:click={() => {
+            if (selected === undefined) {
+                selected = initialSelected;
+            }
+            showFloating = !showFloating;
+        }}
         bind:this={element}
         use:asReference
         bind:clientWidth
@@ -89,8 +226,24 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
             </IconConstrain>
         </div>
     </div>
-    <Popover slot="floating" scrollable --popover-width="{clientWidth}px">
-        <slot />
+    <Popover
+        slot="floating"
+        scrollable
+        --popover-width="{clientWidth}px"
+        id={ids.popover}
+        on:revealed={revealed}
+    >
+        {#each parsed as { content, parsedValue, disabled }, idx (idx)}
+            <SelectOption
+                value={parsedValue}
+                bind:element={buttons[idx]}
+                {disabled}
+                selected={idx === selected}
+                id={ids.focused}
+            >
+                {content}
+            </SelectOption>
+        {/each}
     </Popover>
 </WithFloating>
 

ts/components/SelectOption.svelte

@@ -11,36 +11,12 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
     type T = $$Generic;
 
+    export let selected = false;
     export let disabled = false;
+    export let id: string;
     export let value: T;
 
-    let element: HTMLButtonElement;
-
-    function handleKey(e: KeyboardEvent) {
-        /* Arrow key navigation */
-        switch (e.code) {
-            case "ArrowUp": {
-                const prevSibling = element?.previousElementSibling as HTMLElement;
-                if (prevSibling) {
-                    prevSibling.focus();
-                } else {
-                    // close popover
-                    document.body.click();
-                }
-                break;
-            }
-            case "ArrowDown": {
-                const nextSibling = element?.nextElementSibling as HTMLElement;
-                if (nextSibling) {
-                    nextSibling.focus();
-                } else {
-                    // close popover
-                    document.body.click();
-                }
-                break;
-            }
-        }
-    }
+    export let element: HTMLButtonElement;
 
     const selectContext: Writable<{ value: T; setValue: Function }> =
         getContext(selectKey);
@@ -49,11 +25,12 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
 <DropdownItem
     {disabled}
+    {selected}
+    id={selected ? id : undefined}
     active={value == $selectContext.value}
+    role="option"
     on:click={() => setValue(value)}
-    on:keydown={handleKey}
     bind:buttonRef={element}
-    tabbable
 >
     <slot />
 </DropdownItem>

ts/components/context-keys.ts

@@ -9,3 +9,5 @@ export const modalsKey = Symbol("modals");
 export const floatingKey = Symbol("floating");
 export const overlayKey = Symbol("overlay");
 export const selectKey = Symbol("select");
+export const showKey = Symbol("selectShow");
+export const focusIdKey = Symbol("selectFocusId");

ts/deck-options/ConfigSelector.svelte

@@ -11,7 +11,6 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     import ButtonToolbar from "../components/ButtonToolbar.svelte";
     import { modalsKey } from "../components/context-keys";
     import Select from "../components/Select.svelte";
-    import SelectOption from "../components/SelectOption.svelte";
     import StickyContainer from "../components/StickyContainer.svelte";
     import type { ConfigListEntry, DeckOptionsState } from "./lib";
     import SaveButton from "./SaveButton.svelte";
@@ -94,11 +93,17 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
 <StickyContainer --gutter-block="0.5rem" --sticky-borders="0 0 1px" breakpoint="sm">
     <ButtonToolbar class="justify-content-between flex-grow-1" wrap={false}>
-        <Select class="flex-grow-1" bind:value {label} on:change={blur}>
-            {#each $configList as entry}
-                <SelectOption value={entry.idx}>{configLabel(entry)}</SelectOption>
-            {/each}
-        </Select>
+        <Select
+            class="flex-grow-1"
+            bind:value
+            {label}
+            list={$configList}
+            parser={(entry) => ({
+                content: configLabel(entry),
+                value: entry.idx,
+            })}
+            on:change={blur}
+        />
 
         <SaveButton
             {state}

ts/import-csv/MapperRow.svelte

@@ -6,7 +6,6 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     import Col from "../components/Col.svelte";
     import Row from "../components/Row.svelte";
     import Select from "../components/Select.svelte";
-    import SelectOption from "../components/SelectOption.svelte";
     import type { ColumnOption } from "./lib";
 
     let rowLabel: string;
@@ -23,10 +22,15 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
         {rowLabel}
     </Col>
     <Col --col-size={1}>
-        <Select bind:value {label}>
-            {#each columnOptions as { label, value, disabled }}
-                <SelectOption {value} {disabled}>{label}</SelectOption>
-            {/each}
-        </Select>
+        <Select
+            bind:value
+            {label}
+            list={columnOptions}
+            parser={(item) => ({
+                content: item.label,
+                value: item.value,
+                disabled: item.disabled,
+            })}
+        />
     </Col>
 </Row>