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

@@ -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<HTMLButtonElement | null>;
 }
 

src/components/views/emojipicker/EmojiPicker.tsx

@@ -79,71 +79,44 @@ class EmojiPicker extends React.Component<IProps, IState> {
             ...DATA_BY_CATEGORY,
         };
 
-        this.categories = [
+        const hasRecentlyUsed = this.recentlyUsed.length > 0;
-            {
+
-                id: "recent",
+        const categoryConfig: Array<{
-                name: _t("emoji|category_frequently_used"),
+            id: CategoryKey;
-                enabled: this.recentlyUsed.length > 0,
+            name: string;
-                visible: this.recentlyUsed.length > 0,
+        }> = [
-                ref: React.createRef(),
+            { 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: "people",
+            { id: "foods", name: _t("emoji|category_food_drink") },
-                name: _t("emoji|category_smileys_people"),
+            { id: "activity", name: _t("emoji|category_activities") },
-                enabled: true,
+            { id: "places", name: _t("emoji|category_travel_places") },
-                visible: true,
+            { id: "objects", name: _t("emoji|category_objects") },
-                ref: React.createRef(),
+            { id: "symbols", name: _t("emoji|category_symbols") },
-            },
+            { id: "flags", name: _t("emoji|category_flags") },
-            {
-                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(),
-            },
         ];
+
+        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<IProps, IState> {
         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<IProps, IState> {
             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");
             }
         }

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<IProps> {
     }
 
     private changeCategoryRelative(delta: number): void {
-        let current: number;
+        // Move to the next/previous category using the first visible as the current.
-        // As multiple categories may be visible at once, we want to find the one closest to the relative direction
+        const current = this.props.categories.findIndex((c) => c.visible);
-        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);
-        }
         this.changeCategoryAbsolute(current + delta, delta);
     }
 
@@ -104,7 +97,7 @@ class Header extends React.PureComponent<IProps> {
                             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}`}
                         />

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<EmojiPicker>();
+        render(<EmojiPicker ref={ref} onChoose={(str: string) => 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<EmojiPicker>();
+        render(<EmojiPicker ref={ref} onChoose={(str: string) => 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<EmojiPicker>();
         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(<EmojiPicker onChoose={jest.fn()} onFinished={jest.fn()} />);
+
+            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(<EmojiPicker onChoose={jest.fn()} onFinished={jest.fn()} />);
+
+            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(<EmojiPicker onChoose={jest.fn()} onFinished={jest.fn()} />);
+
+            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(<EmojiPicker onChoose={jest.fn()} onFinished={jest.fn()} />);
+
+            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(<EmojiPicker onChoose={jest.fn()} onFinished={jest.fn()} />);
+
+            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();
+            });
+        });
+    });
 });