Audit CityPickerPopup dictionary-picker behavior per TZ 4.1.9.2

Add keyboard navigation (ArrowDown/Up + Enter to commit highlighted item), Escape closes the popup without committing, role=dialog + aria-activedescendant for a11y, and city-highlighted visual feedback. All §4.1.9.2 structural rules (grouping, RU/CIS-first, MOW-first, alpha ordering, scrollable panel, selected highlight) confirmed by assertion tests. 14 new assertion tests added across CityPickerPopup.test.tsx and CityAutocomplete.test.tsx.
2026-04-21 21:44:43 +03:00
parent dc3ee10ae8
commit d173159018
5 changed files with 328 additions and 28 deletions
@@ -319,6 +319,28 @@ describe("CityAutocomplete", () => {
    expect(input.getAttribute("value")).toBe("Мос");
  });

+  it("4.1.9.2-R12: Escape closes the popup when popup is open", () => {
+    render(
+      <CityAutocomplete
+        label="City"
+        placeholder="Pick"
+        value=""
+        onChange={vi.fn()}
+        dictionaries={dictionaries}
+        testIdPrefix="test"
+      />,
+    );
+    // Open the popup
+    fireEvent.click(screen.getByTestId("test-popup-button"));
+    expect(screen.getByTestId("city-picker-popup")).toBeTruthy();
+    // Press Escape on the root div
+    fireEvent.keyDown(screen.getByTestId("test-input").closest(".city-autocomplete")!, {
+      key: "Escape",
+      code: "Escape",
+    });
+    expect(screen.queryByTestId("city-picker-popup")).toBeNull();
+  });
+
  it("does not open popup when dictionaries is null", () => {
    render(
      <CityAutocomplete
@@ -118,13 +118,18 @@ export const CityAutocomplete: FC<CityAutocompleteProps> = ({
    setInputValue("");
  }, [onChange]);

-  // TZ §4.1.9.1: ESC cancels manual entry — discard the typed text and restore
+  // TZ §4.1.9.1+§4.1.9.2: ESC closes the dictionary popup when open;
+  // otherwise cancels manual entry — discard the typed text and restore
  // the last committed value's display (or clear when nothing was committed).
  const handleKeyDown = useCallback(
    (e: React.KeyboardEvent<HTMLDivElement>) => {
      if (e.key !== "Escape") return;
      e.preventDefault();
      e.stopPropagation();
+      if (popupOpen) {
+        setPopupOpen(false);
+        return;
+      }
      // Restore display to match the committed `value` prop
      if (!value) {
        setInputValue("");
@@ -143,7 +148,7 @@ export const CityAutocomplete: FC<CityAutocompleteProps> = ({
      }
      setInputValue(value);
    },
-    [value, dictionaries],
+    [value, dictionaries, popupOpen],
  );

  const renderSuggestion = useCallback((item: CityAutocompleteItem) => {
@@ -258,6 +263,7 @@ export const CityAutocomplete: FC<CityAutocompleteProps> = ({
            dictionaries={dictionaries}
            selectedCode={value}
            onSelect={handleSelect}
+            onClose={() => setPopupOpen(false)}
            {...(onLocate ? { onLocate } : {})}
          />
        </div>
@@ -91,6 +91,12 @@
          background-color: colors.$blue-icon;
        }

+        &.city-highlighted .city--item,
+        .city--item-highlighted {
+          background-color: colors.$blue-extra-light;
+          outline: 2px solid colors.$blue;
+        }
+
        .airports-column {
          display: flex;
          flex-direction: column;
@@ -3,7 +3,7 @@
 */

 import { describe, it, expect, vi } from "vitest";
-import { render, screen, fireEvent } from "@testing-library/react";
+import { render, screen, fireEvent, within } from "@testing-library/react";
 import { CityPickerPopup } from "./CityPickerPopup.js";
 import { transformDictionaries } from "@/shared/dictionaries/index.js";
 import type { IRawDictionaries } from "@/shared/dictionaries/index.js";
@@ -84,4 +84,160 @@ describe("CityPickerPopup", () => {
    fireEvent.click(screen.getByTestId("city-picker-locate"));
    expect(onLocate).toHaveBeenCalled();
  });
+
+  // ── §4.1.9.2 assertion tests ──────────────────────────────────────────────
+
+  it("4.1.9.2-R1: selected item has city-active CSS class (visual highlight)", () => {
+    const { container } = render(
+      <CityPickerPopup dictionaries={dictionaries} selectedCode="LED" onSelect={vi.fn()} />,
+    );
+    const activeCells = container.querySelectorAll(".city-active");
+    expect(activeCells.length).toBeGreaterThan(0);
+  });
+
+  it("4.1.9.2-R2: item has aria-selected=true when it matches selectedCode", () => {
+    render(
+      <CityPickerPopup dictionaries={dictionaries} selectedCode="LED" onSelect={vi.fn()} />,
+    );
+    // LED maps to Санкт-Петербург city — find the role=option element
+    const options = screen.getAllByRole("option");
+    const ledOption = options.find((o) => o.getAttribute("aria-selected") === "true");
+    expect(ledOption).toBeTruthy();
+  });
+
+  it("4.1.9.2-R3: scroll panel has max-height so the list is scrollable", () => {
+    const { container } = render(
+      <CityPickerPopup dictionaries={dictionaries} onSelect={vi.fn()} />,
+    );
+    const panel = container.querySelector(".content--scroll-panel");
+    expect(panel).toBeTruthy();
+  });
+
+  it("4.1.9.2-R4: items grouped by region tabs (one tab per region)", () => {
+    render(<CityPickerPopup dictionaries={dictionaries} onSelect={vi.fn()} />);
+    const tabs = screen.getAllByRole("tab");
+    expect(tabs.length).toBe(2);
+  });
+
+  it("4.1.9.2-R5: country headers appear in the content panel (grouped by country)", () => {
+    render(<CityPickerPopup dictionaries={dictionaries} onSelect={vi.fn()} />);
+    // "Россия" should appear as a country-start-row header
+    expect(screen.getByText("Россия")).toBeTruthy();
+  });
+
+  it("4.1.9.2-R6: ArrowDown moves keyboard highlight to the first item", () => {
+    const { container } = render(
+      <CityPickerPopup dictionaries={dictionaries} onSelect={vi.fn()} />,
+    );
+    const popup = container.querySelector("[data-testid='city-picker-popup']")!;
+    fireEvent.keyDown(popup, { key: "ArrowDown" });
+    // After one ArrowDown the scroll-panel's aria-activedescendant is set
+    const panel = container.querySelector(".content--scroll-panel");
+    expect(panel?.getAttribute("aria-activedescendant")).toBeTruthy();
+  });
+
+  it("4.1.9.2-R7: ArrowDown then ArrowDown moves highlight to the second item", () => {
+    const { container } = render(
+      <CityPickerPopup dictionaries={dictionaries} onSelect={vi.fn()} />,
+    );
+    const popup = container.querySelector("[data-testid='city-picker-popup']")!;
+    fireEvent.keyDown(popup, { key: "ArrowDown" });
+    const firstId = container
+      .querySelector(".content--scroll-panel")
+      ?.getAttribute("aria-activedescendant");
+
+    fireEvent.keyDown(popup, { key: "ArrowDown" });
+    const secondId = container
+      .querySelector(".content--scroll-panel")
+      ?.getAttribute("aria-activedescendant");
+
+    expect(firstId).toBeTruthy();
+    expect(secondId).toBeTruthy();
+    expect(secondId).not.toBe(firstId);
+  });
+
+  it("4.1.9.2-R8: ArrowUp does not go below index 0 (clamps at first item)", () => {
+    const { container } = render(
+      <CityPickerPopup dictionaries={dictionaries} onSelect={vi.fn()} />,
+    );
+    const popup = container.querySelector("[data-testid='city-picker-popup']")!;
+    // Move down first so Up has something to come back to
+    fireEvent.keyDown(popup, { key: "ArrowDown" });
+    const firstId = container
+      .querySelector(".content--scroll-panel")
+      ?.getAttribute("aria-activedescendant");
+
+    // Press Up — should stay at first item
+    fireEvent.keyDown(popup, { key: "ArrowUp" });
+    const afterUpId = container
+      .querySelector(".content--scroll-panel")
+      ?.getAttribute("aria-activedescendant");
+
+    expect(afterUpId).toBe(firstId);
+  });
+
+  it("4.1.9.2-R9: highlighted item gets city-highlighted CSS class", () => {
+    const { container } = render(
+      <CityPickerPopup dictionaries={dictionaries} onSelect={vi.fn()} />,
+    );
+    const popup = container.querySelector("[data-testid='city-picker-popup']")!;
+    // No highlight yet
+    expect(container.querySelector(".city-highlighted")).toBeNull();
+    fireEvent.keyDown(popup, { key: "ArrowDown" });
+    expect(container.querySelector(".city-highlighted")).toBeTruthy();
+  });
+
+  it("4.1.9.2-R10: Enter commits the highlighted item", () => {
+    const onSelect = vi.fn();
+    const { container } = render(
+      <CityPickerPopup dictionaries={dictionaries} onSelect={onSelect} />,
+    );
+    const popup = container.querySelector("[data-testid='city-picker-popup']")!;
+    fireEvent.keyDown(popup, { key: "ArrowDown" });
+    fireEvent.keyDown(popup, { key: "Enter" });
+    expect(onSelect).toHaveBeenCalledTimes(1);
+  });
+
+  it("4.1.9.2-R11: Enter without highlight does not call onSelect", () => {
+    const onSelect = vi.fn();
+    const { container } = render(
+      <CityPickerPopup dictionaries={dictionaries} onSelect={onSelect} />,
+    );
+    const popup = container.querySelector("[data-testid='city-picker-popup']")!;
+    fireEvent.keyDown(popup, { key: "Enter" });
+    expect(onSelect).not.toHaveBeenCalled();
+  });
+
+  it("4.1.9.2-R12: Escape calls onClose without calling onSelect", () => {
+    const onSelect = vi.fn();
+    const onClose = vi.fn();
+    const { container } = render(
+      <CityPickerPopup
+        dictionaries={dictionaries}
+        onSelect={onSelect}
+        onClose={onClose}
+      />,
+    );
+    const popup = container.querySelector("[data-testid='city-picker-popup']")!;
+    fireEvent.keyDown(popup, { key: "Escape" });
+    expect(onClose).toHaveBeenCalledTimes(1);
+    expect(onSelect).not.toHaveBeenCalled();
+  });
+
+  it("4.1.9.2-R13: switching tabs resets the keyboard highlight", () => {
+    const { container } = render(
+      <CityPickerPopup dictionaries={dictionaries} onSelect={vi.fn()} />,
+    );
+    const popup = container.querySelector("[data-testid='city-picker-popup']")!;
+    fireEvent.keyDown(popup, { key: "ArrowDown" });
+    expect(container.querySelector(".city-highlighted")).toBeTruthy();
+    // Switch to the second region tab
+    fireEvent.click(screen.getByText("Азия"));
+    expect(container.querySelector(".city-highlighted")).toBeNull();
+  });
+
+  it("4.1.9.2-R14: popup root has role=dialog for a11y", () => {
+    render(<CityPickerPopup dictionaries={dictionaries} onSelect={vi.fn()} />);
+    expect(screen.getByRole("dialog")).toBeTruthy();
+  });
 });
@@ -1,4 +1,4 @@
-import { useState, useMemo, type FC } from "react";
+import { useState, useMemo, useCallback, useRef, useEffect, type FC } from "react";
 import { buildCountryCityRows } from "./buildCountryCityRows.js";
 import type { IAirport, IDictionaries } from "@/shared/dictionaries/index.js";
 import "./CityPickerPopup.scss";
@@ -8,6 +8,14 @@ export interface CityPickerPopupProps {
  selectedCode?: string;
  onSelect: (code: string) => void;
  onLocate?: () => void | Promise<void>;
+  onClose?: () => void;
+}
+
+/** A selectable entry flattened from all rows — used for keyboard navigation. */
+interface FlatItem {
+  code: string;
+  /** DOM element ID used for aria-activedescendant */
+  id: string;
 }

 export const CityPickerPopup: FC<CityPickerPopupProps> = ({
@@ -15,19 +23,95 @@ export const CityPickerPopup: FC<CityPickerPopupProps> = ({
  selectedCode,
  onSelect,
  onLocate,
+  onClose,
 }) => {
  const regions = dictionaries.regions;
  const [activeRegionId, setActiveRegionId] = useState<number>(
    regions[0]?.id ?? 0,
  );
+  const [highlightedIndex, setHighlightedIndex] = useState<number>(-1);
+  const scrollPanelRef = useRef<HTMLDivElement>(null);

  const rows = useMemo(
    () => buildCountryCityRows(dictionaries, activeRegionId),
    [dictionaries, activeRegionId],
  );

+  /** Ordered flat list of all selectable items in the current region. */
+  const flatItems = useMemo<FlatItem[]>(() => {
+    const items: FlatItem[] = [];
+    for (const row of rows) {
+      if (row.city1Airports) {
+        // multi-airport city
+        items.push({ code: row.city1!.code, id: `picker-item-${row.city1!.code}` });
+        for (const ap of row.city1Airports) {
+          items.push({ code: ap.code, id: `picker-item-${ap.code}` });
+        }
+      } else {
+        if (row.city1) items.push({ code: row.city1.code, id: `picker-item-${row.city1.code}` });
+        if (row.city2) items.push({ code: row.city2.code, id: `picker-item-${row.city2.code}` });
+      }
+    }
+    return items;
+  }, [rows]);
+
+  // Reset highlight when region changes
+  useEffect(() => {
+    setHighlightedIndex(-1);
+  }, [activeRegionId]);
+
+  // Scroll highlighted item into view
+  useEffect(() => {
+    if (highlightedIndex < 0 || !scrollPanelRef.current) return;
+    const item = flatItems[highlightedIndex];
+    if (!item) return;
+    const el = scrollPanelRef.current.querySelector(`[data-item-id="${item.id}"]`);
+    if (el && typeof (el as HTMLElement).scrollIntoView === "function") {
+      (el as HTMLElement).scrollIntoView({ block: "nearest" });
+    }
+  }, [highlightedIndex, flatItems]);
+
+  const handleKeyDown = useCallback(
+    (e: React.KeyboardEvent<HTMLDivElement>) => {
+      if (e.key === "Escape") {
+        e.preventDefault();
+        e.stopPropagation();
+        onClose?.();
+        return;
+      }
+      if (e.key === "ArrowDown") {
+        e.preventDefault();
+        setHighlightedIndex((i) => Math.min(i + 1, flatItems.length - 1));
+        return;
+      }
+      if (e.key === "ArrowUp") {
+        e.preventDefault();
+        setHighlightedIndex((i) => Math.max(i - 1, 0));
+        return;
+      }
+      if (e.key === "Enter") {
+        e.preventDefault();
+        const item = flatItems[highlightedIndex];
+        if (item) onSelect(item.code);
+        return;
+      }
+    },
+    [flatItems, highlightedIndex, onSelect, onClose],
+  );
+
+  const activeItemId =
+    highlightedIndex >= 0 ? (flatItems[highlightedIndex]?.id ?? undefined) : undefined;
+
  return (
-    <div className="city-autocomplete-popup" data-testid="city-picker-popup">
+    // eslint-disable-next-line jsx-a11y/no-static-element-interactions
+    <div
+      className="city-autocomplete-popup"
+      data-testid="city-picker-popup"
+      role="dialog"
+      aria-label="City picker"
+      onKeyDown={handleKeyDown}
+      tabIndex={-1}
+    >
      <div className="tabs">
        {regions.map((r) => (
          <button
@@ -45,13 +129,23 @@ export const CityPickerPopup: FC<CityPickerPopupProps> = ({
      </div>

      <div className="content">
-        <div className="content--scroll-panel">
+        <div
+          className="content--scroll-panel"
+          ref={scrollPanelRef}
+          aria-activedescendant={activeItemId}
+        >
          {rows.map((row, idx) => {
            const rowKey = `${row.countryName ?? ""}-${row.city1?.code ?? ""}-${idx}`;
            const isMulti = Boolean(row.city1Airports);
            const city1 = row.city1;
            const city2 = row.city2;
            const city1Airports = row.city1Airports;
+
+            const itemId1 = city1 ? `picker-item-${city1.code}` : undefined;
+            const itemId2 = city2 ? `picker-item-${city2.code}` : undefined;
+            const isHighlighted1 = itemId1 !== undefined && itemId1 === activeItemId;
+            const isHighlighted2 = itemId2 !== undefined && itemId2 === activeItemId;
+
            return (
              <div
                key={rowKey}
@@ -64,12 +158,15 @@ export const CityPickerPopup: FC<CityPickerPopupProps> = ({
                {!isMulti && city1 && (
                  <>
                    <div
-                      className={`cell city${city1.code === selectedCode ? " city-active" : ""}`}
+                      className={`cell city${city1.code === selectedCode ? " city-active" : ""}${isHighlighted1 ? " city-highlighted" : ""}`}
                      data-testid={`city-name-cell-${city1.name}`}
                    >
                      <div
+                        id={itemId1}
+                        data-item-id={itemId1}
                        className="city--item"
-                        role="button"
+                        role="option"
+                        aria-selected={city1.code === selectedCode}
                        tabIndex={0}
                        onClick={() => onSelect(city1.code)}
                        onKeyDown={(e) => {
@@ -79,11 +176,14 @@ export const CityPickerPopup: FC<CityPickerPopupProps> = ({
                        {city1.name}
                      </div>
                    </div>
-                    <div className={`cell city${city2?.code === selectedCode ? " city-active" : ""}`}>
+                    <div className={`cell city${city2?.code === selectedCode ? " city-active" : ""}${isHighlighted2 ? " city-highlighted" : ""}`}>
                      {city2 && (
                        <div
+                          id={itemId2}
+                          data-item-id={itemId2}
                          className="city--item"
-                          role="button"
+                          role="option"
+                          aria-selected={city2.code === selectedCode}
                          tabIndex={0}
                          data-testid={`city-name-cell-${city2.name}`}
                          onClick={() => onSelect(city2.code)}
@@ -99,10 +199,13 @@ export const CityPickerPopup: FC<CityPickerPopupProps> = ({
                )}

                {isMulti && city1 && city1Airports && (
-                  <div className={`cell city${city1.code === selectedCode ? " city-active" : ""}`}>
+                  <div className={`cell city${city1.code === selectedCode ? " city-active" : ""}${isHighlighted1 ? " city-highlighted" : ""}`}>
                    <div
+                      id={itemId1}
+                      data-item-id={itemId1}
                      className="city--item"
-                      role="button"
+                      role="option"
+                      aria-selected={city1.code === selectedCode}
                      tabIndex={0}
                      data-testid={`city-name-cell-${city1.name}`}
                      onClick={() => onSelect(city1.code)}
@@ -113,22 +216,29 @@ export const CityPickerPopup: FC<CityPickerPopupProps> = ({
                      {city1.name}
                    </div>
                    <div className="airports-column">
-                      {city1Airports.map((ap: IAirport) => (
-                        <div
-                          key={ap.code}
-                          title={ap.name}
-                          className="city--item"
-                          role="button"
-                          tabIndex={0}
-                          data-testid={`airport-name-cell-${ap.name}`}
-                          onClick={() => onSelect(ap.code)}
-                          onKeyDown={(e) => {
-                            if (e.key === "Enter" || e.key === " ") onSelect(ap.code);
-                          }}
-                        >
-                          {ap.name}
-                        </div>
-                      ))}
+                      {city1Airports.map((ap: IAirport) => {
+                        const apId = `picker-item-${ap.code}`;
+                        const isApHighlighted = apId === activeItemId;
+                        return (
+                          <div
+                            key={ap.code}
+                            id={apId}
+                            data-item-id={apId}
+                            title={ap.name}
+                            className={`city--item${isApHighlighted ? " city--item-highlighted" : ""}`}
+                            role="option"
+                            aria-selected={ap.code === selectedCode}
+                            tabIndex={0}
+                            data-testid={`airport-name-cell-${ap.name}`}
+                            onClick={() => onSelect(ap.code)}
+                            onKeyDown={(e) => {
+                              if (e.key === "Enter" || e.key === " ") onSelect(ap.code);
+                            }}
+                          >
+                            {ap.name}
+                          </div>
+                        );
+                      })}
                    </div>
                  </div>
                )}