diff --git a/playwright/snapshots/composer/CIDER.spec.ts/emoji-picker-linux.png b/playwright/snapshots/composer/CIDER.spec.ts/emoji-picker-linux.png index f62b471c5b..8c0daa555c 100644 Binary files a/playwright/snapshots/composer/CIDER.spec.ts/emoji-picker-linux.png and b/playwright/snapshots/composer/CIDER.spec.ts/emoji-picker-linux.png differ diff --git a/playwright/snapshots/composer/CIDER.spec.ts/emoji-picker-small-linux.png b/playwright/snapshots/composer/CIDER.spec.ts/emoji-picker-small-linux.png index 96e5439442..e3911831de 100644 Binary files a/playwright/snapshots/composer/CIDER.spec.ts/emoji-picker-small-linux.png and b/playwright/snapshots/composer/CIDER.spec.ts/emoji-picker-small-linux.png differ diff --git a/res/css/views/emojipicker/_EmojiPicker.pcss b/res/css/views/emojipicker/_EmojiPicker.pcss index f6bca61ff5..bc14c2e958 100644 --- a/res/css/views/emojipicker/_EmojiPicker.pcss +++ b/res/css/views/emojipicker/_EmojiPicker.pcss @@ -182,7 +182,7 @@ Please see LICENSE files in the repository root for full details. } } -.mx_EmojiPicker_body .mx_EmojiPicker_item_wrapper [tabindex="0"] .mx_EmojiPicker_item { +.mx_EmojiPicker_body_showHighlight .mx_EmojiPicker_item_wrapper [tabindex="0"] .mx_EmojiPicker_item { background-color: $focus-bg-color; } diff --git a/src/components/views/emojipicker/EmojiPicker.tsx b/src/components/views/emojipicker/EmojiPicker.tsx index 5311e84c91..561361bb4d 100644 --- a/src/components/views/emojipicker/EmojiPicker.tsx +++ b/src/components/views/emojipicker/EmojiPicker.tsx @@ -9,6 +9,7 @@ Please see LICENSE files in the repository root for full details. import React, { type Dispatch } from "react"; import { DATA_BY_CATEGORY, getEmojiFromUnicode, type Emoji as IEmoji } from "@matrix-org/emojibase-bindings"; +import classNames from "classnames"; import { _t } from "../../../languageHandler"; import * as recent from "../../../emojipicker/recent"; @@ -50,6 +51,8 @@ interface IState { // should be enough to never have blank rows of emojis as // 3 rows of overflow are also rendered. The actual value is updated on scroll. viewportHeight: number; + // Track if user has interacted with arrow keys or search + showHighlight: boolean; } class EmojiPicker extends React.Component { @@ -66,6 +69,7 @@ class EmojiPicker extends React.Component { filter: "", scrollTop: 0, viewportHeight: 280, + showHighlight: false, }; // Convert recent emoji characters to emoji data, removing unknowns and duplicates @@ -233,6 +237,20 @@ class EmojiPicker extends React.Component { private onKeyDown = (ev: React.KeyboardEvent, state: RovingState, dispatch: Dispatch): void => { if (state.activeNode && [Key.ARROW_DOWN, Key.ARROW_RIGHT, Key.ARROW_LEFT, Key.ARROW_UP].includes(ev.key)) { + // If highlight is not shown yet, show it and reset to first emoji + if (!this.state.showHighlight) { + this.setState({ showHighlight: true }); + // Reset to first emoji when showing highlight for the first time (or after it was hidden) + if (state.nodes.length > 0) { + dispatch({ + type: Type.SetFocus, + payload: { node: state.nodes[0] }, + }); + } + ev.preventDefault(); + ev.stopPropagation(); + return; + } this.keyboardNavigation(ev, state, dispatch); } }; @@ -274,6 +292,15 @@ class EmojiPicker extends React.Component { private onChangeFilter = (filter: string): void => { const lcFilter = filter.toLowerCase().trim(); // filter is case insensitive + + // User has typed a query, show highlight + // If filter is cleared, hide highlight again + if (lcFilter && !this.state.showHighlight) { + this.setState({ showHighlight: true }); + } else if (!lcFilter && this.state.showHighlight) { + this.setState({ showHighlight: false }); + } + for (const cat of this.categories) { let emojis: IEmoji[]; // If the new filter string includes the old filter string, we don't have to re-filter the whole dataset. @@ -335,6 +362,9 @@ class EmojiPicker extends React.Component { }; private onEnterFilter = (): void => { + // Only select emoji if highlight is shown + if (!this.state.showHighlight) return; + const btn = this.scrollRef.current?.containerRef.current?.querySelector( '.mx_EmojiPicker_item_wrapper [tabindex="0"]', ); @@ -391,7 +421,9 @@ class EmojiPicker extends React.Component { /> diff --git a/src/components/views/emojipicker/Search.tsx b/src/components/views/emojipicker/Search.tsx index db6ca813df..007fa2251d 100644 --- a/src/components/views/emojipicker/Search.tsx +++ b/src/components/views/emojipicker/Search.tsx @@ -71,7 +71,9 @@ class Search extends React.PureComponent { onChange={(ev) => this.props.onChange(ev.target.value)} onKeyDown={this.onKeyDown} ref={this.inputRef} - aria-activedescendant={this.context.state.activeNode?.id} + // Setting aria-activedescendant on the input allows screen readers to identify the active emoji. + // Setting it when there is not a query causes screen readers to read out the first emoji when focusing the input, and it continually tells you you are in the table vs the input. + aria-activedescendant={this.props.query ? this.context.state.activeNode?.id : undefined} aria-controls="mx_EmojiPicker_body" aria-haspopup="grid" aria-autocomplete="list" diff --git a/test/unit-tests/components/views/emojipicker/EmojiPicker-test.tsx b/test/unit-tests/components/views/emojipicker/EmojiPicker-test.tsx index ad56b76adc..2226a70c1c 100644 --- a/test/unit-tests/components/views/emojipicker/EmojiPicker-test.tsx +++ b/test/unit-tests/components/views/emojipicker/EmojiPicker-test.tsx @@ -17,6 +17,10 @@ import SettingsStore from "../../../../../src/settings/SettingsStore"; describe("EmojiPicker", function () { stubClient(); + // Helper to get the currently active emoji's text content from the grid + const getActiveEmojiText = (container: HTMLElement): string => + container.querySelector('.mx_EmojiPicker_body .mx_EmojiPicker_item_wrapper [tabindex="0"]')?.textContent || ""; + beforeEach(() => { // Clear recent emojis to prevent test pollution jest.spyOn(SettingsStore, "getValue").mockImplementation((settingName) => { @@ -77,11 +81,14 @@ describe("EmojiPicker", function () { expect(input).toHaveFocus(); function getEmoji(): string { - const activeDescendant = input.getAttribute("aria-activedescendant"); - return container.querySelector("#" + activeDescendant)!.textContent!; + return getActiveEmojiText(container); } expect(getEmoji()).toEqual("😀"); + // First arrow key press shows highlight without navigating + await userEvent.keyboard("[ArrowDown]"); + expect(getEmoji()).toEqual("😀"); + // Subsequent arrow keys navigate await userEvent.keyboard("[ArrowDown]"); expect(getEmoji()).toEqual("🙂"); await userEvent.keyboard("[ArrowUp]"); @@ -129,9 +136,7 @@ describe("EmojiPicker", function () { } function getVirtuallyFocusedEmoji(): string { - const activeDescendant = input.getAttribute("aria-activedescendant"); - if (!activeDescendant) return ""; - return container.querySelector("#" + activeDescendant)?.textContent || ""; + return getActiveEmojiText(container); } // Initially, arrow keys use virtual focus (aria-activedescendant) @@ -140,6 +145,13 @@ describe("EmojiPicker", function () { expect(getVirtuallyFocusedEmoji()).toEqual("😀"); expect(getEmoji()).toEqual(""); // No actual emoji has focus + // First arrow key press shows highlight without navigating + await userEvent.keyboard("[ArrowDown]"); + expect(input).toHaveFocus(); // Input still has focus + expect(getVirtuallyFocusedEmoji()).toEqual("😀"); // Virtual focus stayed on first emoji + expect(getEmoji()).toEqual(""); // No actual emoji has focus + + // Second arrow key press navigates await userEvent.keyboard("[ArrowDown]"); expect(input).toHaveFocus(); // Input still has focus expect(getVirtuallyFocusedEmoji()).toEqual("🙂"); // Virtual focus moved @@ -163,4 +175,98 @@ describe("EmojiPicker", function () { expect(getEmoji()).toEqual("🙃"); // Actual focus moved right expect(input).not.toHaveFocus(); }); + + it("should not select emoji on Enter press before highlight is shown", async () => { + // mock offsetParent + Object.defineProperty(HTMLElement.prototype, "offsetParent", { + get() { + return this.parentNode; + }, + }); + + const onChoose = jest.fn(); + const onFinished = jest.fn(); + const { container } = render(); + + const input = container.querySelector("input")!; + expect(input).toHaveFocus(); + + // Wait for emojis to render + await waitFor(() => { + expect(container.querySelector('[role="gridcell"]')).toBeInTheDocument(); + }); + + // Press Enter immediately without interacting with arrow keys or search + await userEvent.keyboard("[Enter]"); + + // onChoose and onFinished should not be called + expect(onChoose).not.toHaveBeenCalled(); + expect(onFinished).not.toHaveBeenCalled(); + + // Now press arrow key to show highlight + await userEvent.keyboard("[ArrowDown]"); + + // Press Enter again - now it should work + await userEvent.keyboard("[Enter]"); + + // onChoose and onFinished should be called + expect(onChoose).toHaveBeenCalledWith("😀"); + expect(onFinished).toHaveBeenCalled(); + }); + + it("should reset to first emoji when filter is cleared after navigation", async () => { + // mock offsetParent + Object.defineProperty(HTMLElement.prototype, "offsetParent", { + get() { + return this.parentNode; + }, + }); + + const onChoose = jest.fn(); + const onFinished = jest.fn(); + const { container } = render(); + + const input = container.querySelector("input")!; + expect(input).toHaveFocus(); + + function getEmoji(): string { + return getActiveEmojiText(container); + } + + // Initially on first emoji + expect(getEmoji()).toEqual("😀"); + + // Show highlight with first arrow press + await userEvent.keyboard("[ArrowDown]"); + expect(getEmoji()).toEqual("😀"); + + // Navigate to a different emoji + await userEvent.keyboard("[ArrowDown]"); + expect(getEmoji()).toEqual("🙂"); + await userEvent.keyboard("[ArrowDown]"); + expect(getEmoji()).toEqual("🤩"); + + // Type a search query to filter emojis (this sets showHighlight=true) + await userEvent.type(input, "think"); + await waitFor(() => { + // After filtering, we should be on the "thinking" emoji + expect(getEmoji()).toEqual("🤔"); + }); + + // Clear the search filter + await userEvent.clear(input); + + // After clearing, showHighlight is false, so the highlight is hidden + // The activeNode might still be on 🤔, but we can't see it + + // Press arrow key - this should reset to first emoji AND show highlight + await userEvent.keyboard("[ArrowDown]"); + await waitFor(() => { + expect(getEmoji()).toEqual("😀"); // Should now be on first emoji with highlight shown + }); + + // Next arrow key should navigate from first emoji + await userEvent.keyboard("[ArrowDown]"); + expect(getEmoji()).toEqual("🙂"); + }); });