From 071e2adedeedebaad297dab1ce7dd363eb447981 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 28 Jul 2026 10:18:15 -0700 Subject: [PATCH] test+i18n: follow-up for salvaged PR #68969 - Extract the cmdk filter into exported rankSearchOption and cover it, SearchableSelect selection/clear/placeholder, and ConfigField searchable-schema routing with 12 vitest cases (sabotage-verified: the backend schema test fails on unfixed main). - Add searchPlaceholder/noResults/systemDefault strings to ja/ar/zh-hant (zh + en came with the salvaged commits; defineLocale would have fallen back to English otherwise). - Add a backend invariant test: timezone ships as a searchable, clearable select of sorted IANA ids with a UTC fallback. --- .../app/settings/searchable-select.test.tsx | 137 ++++++++++++++++++ .../src/app/settings/searchable-select.tsx | 70 ++++----- apps/desktop/src/i18n/ar.ts | 3 + apps/desktop/src/i18n/ja.ts | 3 + apps/desktop/src/i18n/zh-hant.ts | 3 + tests/hermes_cli/test_web_server.py | 23 +++ 6 files changed, 200 insertions(+), 39 deletions(-) create mode 100644 apps/desktop/src/app/settings/searchable-select.test.tsx diff --git a/apps/desktop/src/app/settings/searchable-select.test.tsx b/apps/desktop/src/app/settings/searchable-select.test.tsx new file mode 100644 index 000000000000..2e87c15c92f0 --- /dev/null +++ b/apps/desktop/src/app/settings/searchable-select.test.tsx @@ -0,0 +1,137 @@ +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest' + +import type { ConfigFieldSchema } from '@/types/hermes' + +import { ConfigField } from './config-field' +import { rankSearchOption, SearchableSelect } from './searchable-select' + +// Radix Popover + cmdk call scrollIntoView / pointer-capture / ResizeObserver +// APIs jsdom lacks. +class TestResizeObserver { + disconnect() {} + observe() {} + unobserve() {} +} + +beforeAll(() => { + vi.stubGlobal('ResizeObserver', TestResizeObserver) + Element.prototype.scrollIntoView = vi.fn() + Element.prototype.hasPointerCapture = vi.fn(() => false) + Element.prototype.releasePointerCapture = vi.fn() +}) + +afterEach(() => { + cleanup() + vi.clearAllMocks() +}) + +describe('rankSearchOption', () => { + it('ranks a final-segment match above a mid-path match', () => { + // "york" hits the city segment of America/New_York (score 2) but only a + // mid-path segment of America/New_York/Special (score 1). + expect(rankSearchOption('America/New_York', 'york')).toBe(2) + expect(rankSearchOption('America/New_York/Special', 'york')).toBe(1) + expect(rankSearchOption('America/New_York', 'york')).toBeGreaterThan( + rankSearchOption('America/New_York/Special', 'york') + ) + }) + + it('is case-insensitive', () => { + expect(rankSearchOption('Asia/Kolkata', 'KOLKATA')).toBe(2) + expect(rankSearchOption('ASIA/KOLKATA', 'kolkata')).toBe(2) + }) + + it('scores a substring match anywhere as 1', () => { + expect(rankSearchOption('America/New_York', 'amer')).toBe(1) + }) + + it('scores a slashless option by plain substring', () => { + expect(rankSearchOption('UTC', 'ut')).toBe(1) + expect(rankSearchOption('UTC', 'xyz')).toBe(0) + }) + + it('scores a non-match as 0', () => { + expect(rankSearchOption('Europe/Berlin', 'tokyo')).toBe(0) + }) +}) + +describe('SearchableSelect', () => { + const options = ['America/New_York', 'Asia/Kolkata', 'Europe/Berlin', 'UTC'] + + it('opens, filters, and selects an option', () => { + const onChange = vi.fn() + + render() + + fireEvent.click(screen.getByRole('combobox')) + fireEvent.change(screen.getByPlaceholderText('Search…'), { target: { value: 'kolkata' } }) + fireEvent.click(screen.getByText('Asia/Kolkata')) + + expect(onChange).toHaveBeenCalledWith('Asia/Kolkata') + }) + + it('renders the clear item when clearLabel is set and selecting it resets to blank', () => { + const onChange = vi.fn() + + render() + + fireEvent.click(screen.getByRole('combobox')) + fireEvent.click(screen.getByText('System default')) + + expect(onChange).toHaveBeenCalledWith('') + }) + + it('omits the clear item without clearLabel', () => { + render() + + fireEvent.click(screen.getByRole('combobox')) + + expect(screen.queryByText('System default')).toBeNull() + }) + + it('shows the placeholder when the value is blank', () => { + render() + + expect(screen.getByRole('combobox').textContent).toContain('Search…') + }) +}) + +describe('ConfigField searchable routing', () => { + const searchableSchema: ConfigFieldSchema = { + type: 'select', + searchable: true, + clearable: true, + options: ['America/New_York', 'UTC'] + } + + it('routes searchable select schemas to SearchableSelect, not a free-text input', () => { + const { container } = render( + + ) + + // The searchable trigger renders; the generic free-text does not. + expect(container.querySelector('[data-slot="searchable-select-trigger"]')).not.toBeNull() + expect(container.querySelector('input[type="text"]')).toBeNull() + }) + + it('keeps plain string schemas on the free-text input', () => { + const { container } = render( + + ) + + expect(container.querySelector('[data-slot="searchable-select-trigger"]')).toBeNull() + expect(screen.getByDisplayValue('hello')).not.toBeNull() + }) + + it('surfaces the clear item via schema.clearable and resets to blank', () => { + const onChange = vi.fn() + + render() + + fireEvent.click(screen.getByRole('combobox')) + fireEvent.click(screen.getByText('System default')) + + expect(onChange).toHaveBeenCalledWith('') + }) +}) diff --git a/apps/desktop/src/app/settings/searchable-select.tsx b/apps/desktop/src/app/settings/searchable-select.tsx index 260d905b985c..8f679b48ba26 100644 --- a/apps/desktop/src/app/settings/searchable-select.tsx +++ b/apps/desktop/src/app/settings/searchable-select.tsx @@ -1,11 +1,33 @@ import { useCallback, useRef, useState } from 'react' -import { Command, CommandEmpty, CommandGroup, CommandInput, CommandItem, CommandList } from '@/components/ui/command' -import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover' import { Codicon } from '@/components/ui/codicon' +import { Command, CommandEmpty, CommandGroup, CommandInput, CommandItem, CommandList } from '@/components/ui/command' import { controlVariants } from '@/components/ui/control' +import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover' import { cn } from '@/lib/utils' +/** + * cmdk filter score for one option. Case-insensitive substring match, with + * the final path segment (after the last "/") ranked above matches anywhere + * else so "york" ranks "America/New_York" over "America/New_York/Special". + * Exported for tests. + */ +export function rankSearchOption(option: string, search: string): number { + const lower = search.toLowerCase() + const itemLower = option.toLowerCase() + const slash = itemLower.lastIndexOf('/') + + if (slash !== -1 && itemLower.slice(slash + 1).includes(lower)) { + return 2 + } + + if (itemLower.includes(lower)) { + return 1 + } + + return 0 +} + /** * Searchable select for large option lists (e.g. ~590 IANA timezones). * Built on Popover + cmdk Command — the same stack as Shadcn's Combobox. @@ -50,6 +72,7 @@ export function SearchableSelect({ - - { - // cmdk's default filter is case-insensitive substring match on - // the item's value. For timezone-like values we also want to - // match segments after "/" so "york" matches "America/New_York". - const lower = search.toLowerCase() - const itemLower = value.toLowerCase() - // Prioritize city/region segment (after last "/") so "york" ranks - // "America/New_York" above "America/New_York/Special". - const slash = itemLower.lastIndexOf('/') - if (slash !== -1 && itemLower.slice(slash + 1).includes(lower)) return 2 - if (itemLower.includes(lower)) return 1 - return 0 - }} - > + + {emptyMessage} {clearLabel && ( - handleSelect('')} - value={clearLabel} - > - + handleSelect('')} value={clearLabel}> + {clearLabel} )} {options.map(option => ( - handleSelect(option)} - value={option} - > - + handleSelect(option)} value={option}> + {option} ))} diff --git a/apps/desktop/src/i18n/ar.ts b/apps/desktop/src/i18n/ar.ts index 0f087a9dd723..4fa1e65e8ddc 100644 --- a/apps/desktop/src/i18n/ar.ts +++ b/apps/desktop/src/i18n/ar.ts @@ -610,6 +610,9 @@ export const ar = defineLocale({ noneParen: '(لا شيء)', notSet: 'غير مضبوط', commaSeparated: 'قيم مفصولة بفواصل', + searchPlaceholder: 'بحث…', + noResults: 'لا توجد نتائج', + systemDefault: 'إعداد النظام الافتراضي', loading: 'جار تحميل إعدادات Hermes...', emptyTitle: 'لا توجد إعدادات', emptyDesc: 'لا يحتوي هذا القسم على إعدادات قابلة للتعديل.', diff --git a/apps/desktop/src/i18n/ja.ts b/apps/desktop/src/i18n/ja.ts index 6ec6c4872001..533e68cd1799 100644 --- a/apps/desktop/src/i18n/ja.ts +++ b/apps/desktop/src/i18n/ja.ts @@ -639,6 +639,9 @@ export const ja = defineLocale({ builtinOnly: '内蔵のみ', notSet: '未設定', commaSeparated: 'カンマ区切りの値', + searchPlaceholder: '検索…', + noResults: '結果が見つかりません', + systemDefault: 'システムのデフォルト', loading: 'Hermes の設定を読み込み中...', emptyTitle: '設定項目がありません', emptyDesc: 'このセクションには調整できる設定がありません。', diff --git a/apps/desktop/src/i18n/zh-hant.ts b/apps/desktop/src/i18n/zh-hant.ts index 10f85b9940e9..bc8b5f83d519 100644 --- a/apps/desktop/src/i18n/zh-hant.ts +++ b/apps/desktop/src/i18n/zh-hant.ts @@ -627,6 +627,9 @@ export const zhHant = defineLocale({ builtinOnly: '僅內建', notSet: '未設定', commaSeparated: '逗號分隔的值', + searchPlaceholder: '搜尋…', + noResults: '找不到結果', + systemDefault: '系統預設', loading: '正在載入 Hermes 設定...', emptyTitle: '無可設定項目', emptyDesc: '此區段沒有可調整的設定。', diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index 24ef3d0f8c40..08c0bfc6f238 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -5327,6 +5327,29 @@ class TestBuildSchemaFromConfig: missing = set(list_memory_provider_names()) - options assert missing == set(), f"discovered providers missing from schema options: {missing}" + def test_timezone_field_is_searchable_select(self): + """timezone must ship as a searchable, clearable select of IANA ids. + + Desktop renders this via SearchableSelect (Popover + cmdk); the old + free-text input let users type invalid timezone strings (#68970). + Invariants, not snapshots: valid IANA entries present, sorted, no + blank entry server-side (the clear item is client-side via + ``clearable``), and never empty even without tzdata (UTC fallback). + """ + from hermes_cli.web_server import CONFIG_SCHEMA, _timezone_options + + entry = CONFIG_SCHEMA["timezone"] + assert entry["type"] == "select" + assert entry.get("searchable") is True + assert entry.get("clearable") is True + options = entry["options"] + assert len(options) >= 1 + assert options == sorted(options) + assert "" not in options + assert "UTC" in options + # Fallback path: never returns an empty list. + assert len(_timezone_options()) >= 1 + def test_dynamic_merge_recomputes_memory_provider_options(self, monkeypatch): """The per-request schema merge re-discovers memory providers.