mirror of
https://github.com/element-hq/element-web.git
synced 2025-12-27 23:11:21 +00:00
Fix screen readers not indicating the emoji picker search field is focused. (#31128)
* Only set active descendant when the user starts typing. * Fix jest tests. * Remove aria-hidden It was failing code quality checks and it actually wasn't addressing the issue. * Only show highlight on arrow key navigation or updating the search query. * Update screenshots * Enter should not select an emoji if it is not highlighted. * On clearing a query and using arrow kets again the highlighted emoji should be reset to the first. * Update selector in picker tests
This commit is contained in:
parent
017aee9a8f
commit
36ccc1ae9a
Binary file not shown.
|
Before Width: | Height: | Size: 41 KiB After Width: | Height: | Size: 41 KiB |
Binary file not shown.
|
Before Width: | Height: | Size: 26 KiB After Width: | Height: | Size: 25 KiB |
@ -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;
|
||||
}
|
||||
|
||||
|
||||
@ -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<IProps, IState> {
|
||||
@ -66,6 +69,7 @@ class EmojiPicker extends React.Component<IProps, IState> {
|
||||
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<IProps, IState> {
|
||||
|
||||
private onKeyDown = (ev: React.KeyboardEvent, state: RovingState, dispatch: Dispatch<RovingAction>): 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<IProps, IState> {
|
||||
|
||||
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<IProps, IState> {
|
||||
};
|
||||
|
||||
private onEnterFilter = (): void => {
|
||||
// Only select emoji if highlight is shown
|
||||
if (!this.state.showHighlight) return;
|
||||
|
||||
const btn = this.scrollRef.current?.containerRef.current?.querySelector<HTMLButtonElement>(
|
||||
'.mx_EmojiPicker_item_wrapper [tabindex="0"]',
|
||||
);
|
||||
@ -391,7 +421,9 @@ class EmojiPicker extends React.Component<IProps, IState> {
|
||||
/>
|
||||
<AutoHideScrollbar
|
||||
id="mx_EmojiPicker_body"
|
||||
className="mx_EmojiPicker_body"
|
||||
className={classNames("mx_EmojiPicker_body", {
|
||||
mx_EmojiPicker_body_showHighlight: this.state.showHighlight,
|
||||
})}
|
||||
ref={this.scrollRef}
|
||||
onScroll={this.onScroll}
|
||||
>
|
||||
|
||||
@ -71,7 +71,9 @@ class Search extends React.PureComponent<IProps> {
|
||||
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"
|
||||
|
||||
@ -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(<EmojiPicker onChoose={onChoose} onFinished={onFinished} />);
|
||||
|
||||
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(<EmojiPicker onChoose={onChoose} onFinished={onFinished} />);
|
||||
|
||||
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("🙂");
|
||||
});
|
||||
});
|
||||
|
||||
Loading…
Reference in New Issue
Block a user