Follow-up fixes for the analysis catalog's query controls

__mocks__/platform-bible-react.tsx

@@ -687,9 +687,10 @@ export function RadioGroupItem({
  * Faithful in three ways the extension depends on. The trigger shows `customSelectedText` when the
  * caller supplies one and the bare `placeholder` otherwise — the real component never lists the
  * selection itself, so a caller that wants the selection named has to name it. An entry is resolved
- * by its label, so two entries sharing a label collide, and one whose `value` is empty can never be
- * selected at all. And `id` lands on the root; the stub additionally mirrors it to `data-testid`,
- * since Testing Library has no by-id query and a test needs to scope to one of several controls.
+ * by its label as the command list reports it, which is trimmed, so two entries whose labels agree
+ * once trimmed collide, and one whose `value` is empty can never be selected at all. And `id` lands
+ * on the root; the stub additionally mirrors it to `data-testid`, since Testing Library has no
+ * by-id query and a test needs to scope to one of several controls.
  */
 export function MultiSelectComboBox({
   entries,
@@ -732,11 +733,16 @@ export function MultiSelectComboBox({
         {entries.map((entry) => (
           <button
             aria-selected={selected.includes(entry.value)}
-            key={entry.label}
-            // Resolving the entry by label, as the real component's own select handler does, so a
-            // stub test cannot pass on a collision the real component would drop.
+            // Keyed by value where the real component keys by label, so a caller offering two
+            // choices under one label is left to the assertions rather than buried under React's
+            // own complaint about it.
+            key={entry.value}
+            // Resolving the entry by the label the command list reports back, as the real
+            // component's own select handler does, so a stub test cannot pass on a collision the
+            // real component would drop.
             onClick={() => {
-              const match = entries.find((candidate) => candidate.label === entry.label);
+              const reported = entry.label.trim();
+              const match = entries.find((candidate) => candidate.label === reported);
               if (match?.value) toggle(match.value);
             }}
             role="option"

src/__tests__/components/AnalysisCatalogPanel.test.tsx

@@ -1,6 +1,7 @@
 /// <reference types="jest" />
 /// <reference types="@testing-library/jest-dom" />
 
+import { useSetting } from '@papi/frontend/react';
 import type { SerializedVerseRef } from '@sillsdev/scripture';
 import { act, render, screen, within } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
@@ -22,6 +23,21 @@ declare global {
   var triggerIntersection: (el: Element, isIntersecting: boolean) => void;
 }
 
+/**
+ * Configures `useSetting` to report the given interface languages, most preferred first, for
+ * `platform.interfaceLanguage` — the only setting the panel reads.
+ *
+ * @throws {Error} When `useSetting` is called with any other key (message: `useSetting mock:
+ *   unexpected key "<key>"`).
+ */
+function mockInterfaceLanguage(interfaceLanguage: string[] = ['und']): void {
+  jest.mocked(useSetting).mockImplementation((key: string) => {
+    if (key === 'platform.interfaceLanguage')
+      return [interfaceLanguage, jest.fn(), jest.fn(), false];
+    throw new Error(`useSetting mock: unexpected key "${key}"`);
+  });
+}
+
 /** Builds a link from `tokenRef` to the analysis, approved unless another status is given. */
 function link(
   analysisId: string,
@@ -193,6 +209,7 @@ describe('AnalysisCatalogPanel', () => {
     claimedFocusRequest = undefined;
     editGloss = () => {};
     mockKeyAsValueLocalizedStrings();
+    mockInterfaceLanguage();
   });
 
   describe('rows', () => {
@@ -634,6 +651,112 @@ describe('AnalysisCatalogPanel', () => {
       expect(listedAnalysisIds()).toEqual(['marked']);
     });
 
+    it('offers a value recorded with surrounding whitespace under a name that can be chosen', async () => {
+      const analysis: TextAnalysis = {
+        ...emptyAnalysis(),
+        tokenAnalyses: [
+          // A part of speech is free-form and reaches the draft as its source system recorded it,
+          // so it can carry whitespace the control trims off the name it reports back — leaving a
+          // choice the control cannot resolve unless it was offered under the trimmed spelling.
+          { ...FIXTURE_STAMPS, id: 'padded', surfaceText: 'λόγος', pos: ' noun ' },
+          { ...FIXTURE_STAMPS, id: 'verb', surfaceText: 'ἦν', pos: 'verb' },
+        ],
+        tokenAnalysisLinks: [link('padded', 'GEN 1:1:0'), link('verb', 'GEN 1:2:0')],
+      };
+      renderPanel({ analysis });
+      await openFilters();
+
+      await userEvent.click(screen.getByRole('option', { name: 'noun' }));
+
+      expect(listedAnalysisIds()).toEqual(['padded']);
+    });
+
+    it('offers a marking that pads what it wraps under a name that can be chosen', async () => {
+      mockKeyAsValueLocalizedStrings({
+        '%interlinearizer_analysisCatalog_filter_recordedValue%': '  {value} (recorded value)  ',
+      });
+      const analysis: TextAnalysis = {
+        ...emptyAnalysis(),
+        tokenAnalyses: [
+          // These agree once trimmed, so one has to be marked. Choices are offered in the order the
+          // values sort in, where the leading space sorts first — leaving the padded value under
+          // the plain name and this one under the marking.
+          { ...FIXTURE_STAMPS, id: 'plain', surfaceText: 'λόγος', pos: 'noun' },
+          { ...FIXTURE_STAMPS, id: 'padded', surfaceText: 'ἦν', pos: ' noun ' },
+        ],
+        tokenAnalysisLinks: [link('plain', 'GEN 1:1:0'), link('padded', 'GEN 1:2:0')],
+      };
+      renderPanel({ analysis });
+      await openFilters();
+
+      await userEvent.click(screen.getByRole('option', { name: 'noun (recorded value)' }));
+
+      expect(listedAnalysisIds()).toEqual(['plain']);
+    });
+
+    it('tells a value recorded as whitespace apart from one recorded as empty', async () => {
+      mockKeyAsValueLocalizedStrings({
+        '%interlinearizer_analysisCatalog_filter_empty%': '(empty)',
+        '%interlinearizer_analysisCatalog_filter_recordedValue%': '{value} (recorded value)',
+      });
+      const analysis: TextAnalysis = {
+        ...emptyAnalysis(),
+        tokenAnalyses: [
+          { ...FIXTURE_STAMPS, id: 'blank', surfaceText: 'λόγος', pos: '' },
+          // Nothing is left of this once trimmed, so it has no name of its own to be offered under.
+          { ...FIXTURE_STAMPS, id: 'spaces', surfaceText: 'ἦν', pos: '   ' },
+        ],
+        tokenAnalysisLinks: [link('blank', 'GEN 1:1:0'), link('spaces', 'GEN 1:2:0')],
+      };
+      renderPanel({ analysis });
+      await openFilters();
+
+      await userEvent.click(screen.getByRole('option', { name: '(empty) (recorded value)' }));
+
+      expect(listedAnalysisIds()).toEqual(['spaces']);
+    });
+
+    it('stops marking a value rather than spinning when the marking cannot tell it apart', async () => {
+      // A localization that drops `{value}` leaves the marking spelling whatever name it was given,
+      // so repeating it can never clear a collision. Two choices then share a name and one of them
+      // is unselectable — but the panel renders, where a render that never returns takes the whole
+      // WebView down with it.
+      mockKeyAsValueLocalizedStrings({
+        '%interlinearizer_analysisCatalog_filter_untagged%': '(none)',
+        '%interlinearizer_analysisCatalog_filter_recordedValue%': '{value}',
+      });
+      const analysis: TextAnalysis = {
+        ...emptyAnalysis(),
+        tokenAnalyses: [
+          { ...FIXTURE_STAMPS, id: 'named', surfaceText: 'λόγος', pos: '(none)' },
+          { ...FIXTURE_STAMPS, id: 'untagged', surfaceText: 'ἦν' },
+        ],
+        tokenAnalysisLinks: [link('named', 'GEN 1:1:0'), link('untagged', 'GEN 1:2:0')],
+      };
+      renderPanel({ analysis });
+
+      await openFilters();
+
+      expect(screen.getAllByRole('option', { name: '(none)' })).toHaveLength(2);
+    });
+
+    it('names the language the missing-gloss filter asks about, in the interface language', async () => {
+      mockInterfaceLanguage(['es']);
+      mockKeyAsValueLocalizedStrings({
+        '%interlinearizer_analysisCatalog_filter_missingGloss%': 'Missing gloss in {language}',
+      });
+      renderPanel({ analysis: PER_BOOK, analysisLanguage: 'fr' });
+
+      await openFilters();
+
+      // A reader who never chose the tag has no reason to recognize it, and a name taken from the
+      // host's own locale would read in one language beside a label resolved in another — the
+      // platform's interface language being a setting the host locale does not follow.
+      expect(
+        screen.getByRole('checkbox', { name: 'Missing gloss in francés' }),
+      ).toBeInTheDocument();
+    });
+
     it('narrows the list to the analyses carrying a chosen part of speech', async () => {
       const analysis: TextAnalysis = {
         ...emptyAnalysis(),
@@ -911,6 +1034,13 @@ describe('AnalysisCatalogPanel', () => {
       ),
     };
 
+    /** The scrolling list, reached through the sentinel it holds as its last child. */
+    function rowList(): HTMLElement {
+      const list = screen.getByTestId('catalog-rows-sentinel').parentElement;
+      if (!list) throw new Error('the row sentinel is outside a list');
+      return list;
+    }
+
     /** Reports the end of the list as having come into view. */
     function reachListEnd(): void {
       act(() => {
@@ -948,6 +1078,29 @@ describe('AnalysisCatalogPanel', () => {
       expect(screen.getAllByTestId('catalog-row').length).toBeLessThan(grown);
     });
 
+    it('returns the list to its top when the query changes', async () => {
+      renderPanel({ analysis: MANY });
+      const list = rowList();
+      list.scrollTop = 500;
+
+      // Matches every row, so the listing is the same length as before — only the window resets.
+      await userEvent.type(searchBox(), 'word');
+
+      // The list is the same element throughout, so it holds the offset it was left at until it is
+      // put back, landing a reader who narrowed a deeply scrolled list part way down a new one.
+      expect(list.scrollTop).toBe(0);
+    });
+
+    it('leaves the scroll where it is when the analysis changes under an unchanged query', () => {
+      renderPanelWithGlossEditing({ analysis: MANY });
+      const list = rowList();
+      list.scrollTop = 500;
+
+      act(() => editGloss('GEN 1:1:0', 'word0', 'beginning'));
+
+      expect(list.scrollTop).toBe(500);
+    });
+
     it('keeps the window where it is when the analysis changes under an unchanged query', () => {
       // A gloss approved in the view beside an open catalog rebuilds every row without narrowing
       // anything, and collapsing a deeply scrolled list back to its first chunk on that would throw

src/__tests__/utils/language-tags.test.ts

@@ -1,6 +1,6 @@
 /// <reference types="jest" />
 
-import { collatorForTag } from '../../utils/language-tags';
+import { collatorForTag, languageNameForTag } from '../../utils/language-tags';
 
 describe('collatorForTag', () => {
   it('collates under the tag it is given', () => {
@@ -15,3 +15,36 @@ describe('collatorForTag', () => {
     expect(collatorForTag('en_US').compare('a', 'b')).toBeLessThan(0);
   });
 });
+
+describe('languageNameForTag', () => {
+  it('names the language a tag stands for', () => {
+    // Held against the tag rather than against a spelling: the name comes back in whatever
+    // language the host is running in, which a test cannot pin.
+    expect(languageNameForTag('fr')).not.toBe('fr');
+  });
+
+  it('names the language in the interface languages it is given', () => {
+    expect(languageNameForTag('fr', ['es'])).toBe('francés');
+  });
+
+  it('names the language in a usable interface language behind one Intl rejects', () => {
+    // `Intl` rejects a whole list for any one entry it cannot parse, where the platform resolves a
+    // localized string by walking past the locales it has nothing for — so dropping the list would
+    // read a name in one language beside a label resolved in another.
+    expect(languageNameForTag('fr', ['en_US', 'es'])).toBe('francés');
+  });
+
+  it('names the language anyway when every interface language is one Intl rejects', () => {
+    // Losing the language a name is read in costs less than losing the name.
+    expect(languageNameForTag('fr', ['en_US'])).not.toBe('fr');
+  });
+
+  it('gives back a tag naming no language it knows', () => {
+    // The private-use range every unlisted language is assigned from, so no host has a name for it.
+    expect(languageNameForTag('qaa')).toBe('qaa');
+  });
+
+  it('gives back a tag Intl rejects', () => {
+    expect(languageNameForTag('en_US')).toBe('en_US');
+  });
+});

src/components/AnalysisCatalogPanel.tsx

@@ -1,8 +1,8 @@
-import { useLocalizedStrings } from '@papi/frontend/react';
+import { useLocalizedStrings, useSetting } from '@papi/frontend/react';
 import { Canon } from '@sillsdev/scripture';
 import { X } from 'lucide-react';
 import { Button, EmptyState, TooltipProvider } from 'platform-bible-react';
-import { formatReplacementString } from 'platform-bible-utils';
+import { formatReplacementString, isPlatformError } from 'platform-bible-utils';
 import { useCallback, useMemo, useState } from 'react';
 import { useAnalysisLanguage, useCatalogRows } from './AnalysisStore';
 import CatalogQueryControls, { QUERY_CONTROL_STRING_KEYS } from './CatalogQueryControls';
@@ -17,7 +17,7 @@ import {
   type CatalogSort,
   type CatalogUsage,
 } from '../utils/analysis-query';
-import { collatorForTag } from '../utils/language-tags';
+import { collatorForTag, languageNameForTag } from '../utils/language-tags';
 
 /**
  * Localized string keys the panel needs, the rows' among them so the list resolves once rather than
@@ -133,6 +133,23 @@ export default function AnalysisCatalogPanel({
    */
   const facets = useMemo(() => deriveFacets(catalogRows), [catalogRows]);
 
+  const [interfaceLanguages] = useSetting('platform.interfaceLanguage', ['und']);
+
+  /**
+   * What the analysis language is called, for the filter that asks after a missing gloss to name it
+   * in prose: the question is about a language rather than about a code, and a reader who never
+   * chose the tag has no reason to recognize it.
+   *
+   * Named in the interface's own languages rather than the host's, which the platform's interface
+   * language does not follow — a name resolved against the host would read in one language beside a
+   * label resolved in another.
+   */
+  const analysisLanguageName = useMemo(() => {
+    /* v8 ignore next -- useSetting never returns PlatformError for this key in practice */
+    const locales = isPlatformError(interfaceLanguages) ? undefined : interfaceLanguages;
+    return languageNameForTag(analysisLanguage, locales);
+  }, [analysisLanguage, interfaceLanguages]);
+
   /**
    * The slice of the listing that is actually mounted. A draft accumulates analyses without bound
    * and every row carries its own expander and usage list, so the list grows as it is scrolled
@@ -201,7 +218,7 @@ export default function AnalysisCatalogPanel({
         */}
         {catalogRows.length > 0 && (
           <CatalogQueryControls
-            analysisLanguage={analysisLanguage}
+            analysisLanguageName={analysisLanguageName}
             currentBookName={currentBookName}
             facets={facets}
             filters={filters}