From 2ce59df1fe5bb417fe6eecf79b6bc257a286fbb9 Mon Sep 17 00:00:00 2001 From: David Langley Date: Wed, 12 Nov 2025 16:39:27 +0000 Subject: [PATCH] Fix emoji category selection with keyboard (#31162) * Use firstVisible category for roving tab index * Adding category keyboard navigation tests * Reduce repetition in categories definition and add tests * Remove ternary operators * Simplify --- src/components/views/emojipicker/Category.tsx | 3 + .../views/emojipicker/EmojiPicker.tsx | 113 ++++----- src/components/views/emojipicker/Header.tsx | 13 +- .../views/emojipicker/EmojiPicker-test.tsx | 222 ++++++++++++++++++ 4 files changed, 276 insertions(+), 75 deletions(-) diff --git a/src/components/views/emojipicker/Category.tsx b/src/components/views/emojipicker/Category.tsx index 75734a5d16..21e5c97c33 100644 --- a/src/components/views/emojipicker/Category.tsx +++ b/src/components/views/emojipicker/Category.tsx @@ -23,7 +23,10 @@ export interface ICategory { id: CategoryKey; name: string; enabled: boolean; + // Whether the category is currently visible visible: boolean; + // Whether the category is the first visible category + firstVisible: boolean; ref: RefObject; } diff --git a/src/components/views/emojipicker/EmojiPicker.tsx b/src/components/views/emojipicker/EmojiPicker.tsx index 3760cbc4c7..9ba2445102 100644 --- a/src/components/views/emojipicker/EmojiPicker.tsx +++ b/src/components/views/emojipicker/EmojiPicker.tsx @@ -79,71 +79,44 @@ class EmojiPicker extends React.Component { ...DATA_BY_CATEGORY, }; - this.categories = [ - { - id: "recent", - name: _t("emoji|category_frequently_used"), - enabled: this.recentlyUsed.length > 0, - visible: this.recentlyUsed.length > 0, - ref: React.createRef(), - }, - { - id: "people", - name: _t("emoji|category_smileys_people"), - enabled: true, - visible: true, - ref: React.createRef(), - }, - { - id: "nature", - name: _t("emoji|category_animals_nature"), - enabled: true, - visible: false, - ref: React.createRef(), - }, - { - id: "foods", - name: _t("emoji|category_food_drink"), - enabled: true, - visible: false, - ref: React.createRef(), - }, - { - id: "activity", - name: _t("emoji|category_activities"), - enabled: true, - visible: false, - ref: React.createRef(), - }, - { - id: "places", - name: _t("emoji|category_travel_places"), - enabled: true, - visible: false, - ref: React.createRef(), - }, - { - id: "objects", - name: _t("emoji|category_objects"), - enabled: true, - visible: false, - ref: React.createRef(), - }, - { - id: "symbols", - name: _t("emoji|category_symbols"), - enabled: true, - visible: false, - ref: React.createRef(), - }, - { - id: "flags", - name: _t("emoji|category_flags"), - enabled: true, - visible: false, - ref: React.createRef(), - }, + const hasRecentlyUsed = this.recentlyUsed.length > 0; + + const categoryConfig: Array<{ + id: CategoryKey; + name: string; + }> = [ + { id: "recent", name: _t("emoji|category_frequently_used") }, + { id: "people", name: _t("emoji|category_smileys_people") }, + { id: "nature", name: _t("emoji|category_animals_nature") }, + { id: "foods", name: _t("emoji|category_food_drink") }, + { id: "activity", name: _t("emoji|category_activities") }, + { id: "places", name: _t("emoji|category_travel_places") }, + { id: "objects", name: _t("emoji|category_objects") }, + { id: "symbols", name: _t("emoji|category_symbols") }, + { id: "flags", name: _t("emoji|category_flags") }, ]; + + this.categories = categoryConfig.map((config) => { + let isEnabled = true; + let isVisible = false; + let firstVisible = false; + if (config.id === "recent") { + isEnabled = hasRecentlyUsed; + isVisible = hasRecentlyUsed; + firstVisible = hasRecentlyUsed; + } else if (config.id === "people") { + isVisible = true; + firstVisible = !hasRecentlyUsed; + } + return { + id: config.id, + name: config.name, + enabled: isEnabled, + visible: isVisible, + firstVisible: firstVisible, + ref: React.createRef(), + }; + }); } private onScroll = (): void => { @@ -259,6 +232,7 @@ class EmojiPicker extends React.Component { const body = this.scrollRef.current?.containerRef.current; if (!body) return; const rect = body.getBoundingClientRect(); + let firstVisibleFound = false; for (const cat of this.categories) { const elem = body.querySelector(`[data-category-id="${cat.id}"]`); if (!elem) { @@ -270,15 +244,24 @@ class EmojiPicker extends React.Component { const y = elemRect.y - rect.y; const yEnd = elemRect.y + elemRect.height - rect.y; cat.visible = y < rect.height && yEnd > 0; + if (cat.visible && !firstVisibleFound) { + firstVisibleFound = true; + cat.firstVisible = true; + } else { + cat.firstVisible = false; + } // We update this here instead of through React to avoid re-render on scroll. if (!cat.ref.current) continue; if (cat.visible) { cat.ref.current.classList.add("mx_EmojiPicker_anchor_visible"); cat.ref.current.setAttribute("aria-selected", "true"); - cat.ref.current.setAttribute("tabindex", "0"); } else { cat.ref.current.classList.remove("mx_EmojiPicker_anchor_visible"); cat.ref.current.setAttribute("aria-selected", "false"); + } + if (cat.firstVisible) { + cat.ref.current.setAttribute("tabindex", "0"); + } else { cat.ref.current.setAttribute("tabindex", "-1"); } } diff --git a/src/components/views/emojipicker/Header.tsx b/src/components/views/emojipicker/Header.tsx index 985882bd7b..ed2c6b9f21 100644 --- a/src/components/views/emojipicker/Header.tsx +++ b/src/components/views/emojipicker/Header.tsx @@ -9,7 +9,6 @@ Please see LICENSE files in the repository root for full details. import React from "react"; import classNames from "classnames"; -import { findLastIndex } from "lodash"; import { _t } from "../../../languageHandler"; import { type CategoryKey, type ICategory } from "./Category"; @@ -33,14 +32,8 @@ class Header extends React.PureComponent { } private changeCategoryRelative(delta: number): void { - let current: number; - // As multiple categories may be visible at once, we want to find the one closest to the relative direction - if (delta < 0) { - current = this.props.categories.findIndex((c) => c.visible); - } else { - // XXX: Switch to Array::findLastIndex once we enable ES2023 - current = findLastIndex(this.props.categories, (c) => c.visible); - } + // Move to the next/previous category using the first visible as the current. + const current = this.props.categories.findIndex((c) => c.visible); this.changeCategoryAbsolute(current + delta, delta); } @@ -104,7 +97,7 @@ class Header extends React.PureComponent { onClick={() => this.props.onAnchorClick(category.id)} title={category.name} role="tab" - tabIndex={category.visible ? 0 : -1} // roving + tabIndex={category.firstVisible ? 0 : -1} // roving aria-selected={category.visible} aria-controls={`mx_EmojiPicker_category_${category.id}`} /> diff --git a/test/unit-tests/components/views/emojipicker/EmojiPicker-test.tsx b/test/unit-tests/components/views/emojipicker/EmojiPicker-test.tsx index 2226a70c1c..951e17ce66 100644 --- a/test/unit-tests/components/views/emojipicker/EmojiPicker-test.tsx +++ b/test/unit-tests/components/views/emojipicker/EmojiPicker-test.tsx @@ -33,6 +33,101 @@ describe("EmojiPicker", function () { jest.restoreAllMocks(); }); + it("should initialize categories with correct state when no recent emojis", () => { + const ref = createRef(); + render( false} onFinished={jest.fn()} />); + + //@ts-ignore private access + const categories = ref.current!.categories; + + // Verify we have all expected categories + expect(categories).toHaveLength(9); + expect(categories.map((c) => c.id)).toEqual([ + "recent", + "people", + "nature", + "foods", + "activity", + "places", + "objects", + "symbols", + "flags", + ]); + + // Recent category should be disabled when empty + const recentCategory = categories.find((c) => c.id === "recent"); + expect(recentCategory).toMatchObject({ + id: "recent", + enabled: false, + visible: false, + firstVisible: false, + }); + + // People category should be the first visible when no recent emojis + const peopleCategory = categories.find((c) => c.id === "people"); + expect(peopleCategory).toMatchObject({ + id: "people", + enabled: true, + visible: true, + firstVisible: true, + }); + + // Other categories should start as not visible and not firstVisible + const natureCategory = categories.find((c) => c.id === "nature"); + expect(natureCategory).toMatchObject({ + id: "nature", + enabled: true, + visible: false, + firstVisible: false, + }); + + const flagsCategory = categories.find((c) => c.id === "flags"); + expect(flagsCategory).toMatchObject({ + id: "flags", + enabled: true, + visible: false, + firstVisible: false, + }); + + // All categories should have refs and names + categories.forEach((cat) => { + expect(cat.ref).toBeTruthy(); + expect(cat.name).toBeTruthy(); + }); + }); + + it("should initialize categories with recent as firstVisible when recent emojis exist", () => { + // Mock recent emojis + jest.spyOn(SettingsStore, "getValue").mockImplementation((settingName) => { + if (settingName === "recent_emoji") return ["😀", "🎉", "❤️"] as any; + return jest.requireActual("../../../../../src/settings/SettingsStore").default.getValue(settingName); + }); + + const ref = createRef(); + render( false} onFinished={jest.fn()} />); + + //@ts-ignore private access + const categories = ref.current!.categories; + + // Recent category should be enabled and firstVisible + const recentCategory = categories.find((c) => c.id === "recent"); + expect(recentCategory).toMatchObject({ + id: "recent", + enabled: true, + visible: true, + firstVisible: true, + }); + + // People category should be visible but NOT firstVisible when recent exists + const peopleCategory = categories.find((c) => c.id === "people"); + expect(peopleCategory).toMatchObject({ + id: "people", + enabled: true, + visible: true, + firstVisible: false, + }); + }); + it("should not mangle default order after filtering", async () => { const ref = createRef(); const { container } = render( @@ -269,4 +364,131 @@ describe("EmojiPicker", function () { await userEvent.keyboard("[ArrowDown]"); expect(getEmoji()).toEqual("🙂"); }); + + describe("Category keyboard selection", () => { + beforeEach(() => { + // mock offsetParent + Object.defineProperty(HTMLElement.prototype, "offsetParent", { + get() { + return this.parentNode; + }, + }); + }); + + it("check tabindex for the first category when no recent emojis", async () => { + const { container } = render(); + + await waitFor(() => { + expect(container.querySelector('[data-category-id="people"]')).toBeInTheDocument(); + }); + + // People category should have tabindex="0" + const peopleTab = container.querySelector('[title*="Smileys"]'); + expect(peopleTab).toHaveAttribute("tabindex", "0"); + expect(peopleTab).toHaveAttribute("aria-selected", "true"); + + // Other categories should have tabindex="-1" + const natureTab = container.querySelector('[title*="Animals"]'); + expect(natureTab).toHaveAttribute("tabindex", "-1"); + }); + + it("check tabindex for recent category when recent emojis exist", async () => { + // Mock recent emojis + jest.spyOn(SettingsStore, "getValue").mockImplementation((settingName) => { + if (settingName === "recent_emoji") return ["😀", "🎉"] as any; + return jest.requireActual("../../../../../src/settings/SettingsStore").default.getValue(settingName); + }); + + const { container } = render(); + + await waitFor(() => { + expect(container.querySelector('[data-category-id="recent"]')).toBeInTheDocument(); + }); + + // Recent category should have tabindex="0" + const recentTab = container.querySelector('[title*="Frequently"]'); + expect(recentTab).toHaveAttribute("tabindex", "0"); + expect(recentTab).toHaveAttribute("aria-selected", "true"); + + // People category should have tabindex="-1" + const peopleTab = container.querySelector('[title*="Smileys"]'); + expect(peopleTab).toHaveAttribute("tabindex", "-1"); + }); + + it("should update table position when clicking on a different category tab", async () => { + const { container } = render(); + + await waitFor(() => { + expect(container.querySelector('[data-category-id="people"]')).toBeInTheDocument(); + }); + + // Initially, people category should be visible + const peopleTab = container.querySelector('[title*="Smileys"]') as HTMLButtonElement; + expect(peopleTab).toHaveAttribute("tabindex", "0"); + + // Click on nature category tab + const natureTab = container.querySelector('[title*="Animals"]') as HTMLButtonElement; + await userEvent.click(natureTab); + + // Wait for scroll and visibility update + await waitFor(() => { + const natureCategory = container.querySelector('[data-category-id="nature"]'); + expect(natureCategory).toBeInTheDocument(); + }); + }); + + it("should navigate between category tabs using arrow keys", async () => { + const { container } = render(); + + await waitFor(() => { + expect(container.querySelector('[data-category-id="people"]')).toBeInTheDocument(); + }); + + // Focus on the category header + const peopleTab = container.querySelector('[title*="Smileys"]') as HTMLButtonElement; + peopleTab.focus(); + expect(peopleTab).toHaveFocus(); + + // Press ArrowRight to move to next category + await userEvent.keyboard("[ArrowRight]"); + + // Should focus on next enabled category and trigger scroll + await waitFor(() => { + // Verify focus moved away from people tab + expect(peopleTab).not.toHaveFocus(); + + // Verify some other category tab now has focus + const focusedTab = document.activeElement; + expect(focusedTab?.getAttribute("role")).toBe("tab"); + expect(focusedTab).not.toBe(peopleTab); + }); + }); + + it("should navigate to first/last category using Home/End keys", async () => { + const { container } = render(); + + await waitFor(() => { + expect(container.querySelector('[data-category-id="people"]')).toBeInTheDocument(); + }); + + // Focus on the category header + const peopleTab = container.querySelector('[title*="Smileys"]') as HTMLButtonElement; + peopleTab.focus(); + + // Press End to jump to last category + await userEvent.keyboard("[End]"); + + await waitFor(() => { + const flagsTab = container.querySelector('[title*="Flags"]') as HTMLButtonElement; + expect(flagsTab).toHaveFocus(); + }); + + // Press Home to jump to first category + await userEvent.keyboard("[Home]"); + + await waitFor(() => { + expect(peopleTab).toHaveFocus(); + }); + }); + }); });