From 9f224710c92e4dfc09963fd00d6e1f8fa1ba4d94 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Wed, 23 Sep 2026 07:48:29 +0000 Subject: [PATCH 1/3] fix(ui-web): state the model picker's name and checked row to a screen reader The picker carried role="dialog" with no accessible name, its rows marked the current model with a tick glyph alone, and #modelChip advertised aria-haspopup without aria-expanded -- so a reader using the markup rather than the pixels was told a dialog had opened, could not hear which of it was chosen, and could not tell whether the chip's popover was up. The popover is named after the slot that opened it, or after the word for a model. Rows become radios carrying aria-checked, one radiogroup per account plus one for Recent, because the same model is listed under both and one set may hold one checked member. The tick is hidden from the accessibility tree now that the state is said, and the show-all row stays out of the set it sits in. Co-authored-by: Claude (claude-opus-5[1m]) --- i18n/messages.json | 4 + ui-tui/src/i18n/messages.generated.ts | 2 + ui-web/src/chrome/ModelChip.test.tsx | 16 +++- ui-web/src/chrome/ModelChip.tsx | 1 + .../src/features/model/ModelPicker.test.tsx | 73 +++++++++++++++++++ ui-web/src/features/model/ModelPicker.tsx | 32 ++++++-- 6 files changed, 122 insertions(+), 6 deletions(-) diff --git a/i18n/messages.json b/i18n/messages.json index e851cbeb0..61d81115f 100644 --- a/i18n/messages.json +++ b/i18n/messages.json @@ -7959,6 +7959,10 @@ "en": "Max", "zh": "全力" }, + "gui.picker.title": { + "en": "Model", + "zh": "模型" + }, "gui.picker.recent": { "en": "Recent", "zh": "最近使用" diff --git a/ui-tui/src/i18n/messages.generated.ts b/ui-tui/src/i18n/messages.generated.ts index 381d081b4..657ee0fb3 100644 --- a/ui-tui/src/i18n/messages.generated.ts +++ b/ui-tui/src/i18n/messages.generated.ts @@ -2206,6 +2206,7 @@ export const UI_TEXT: Record> = { 'gui.tier.medium': 'Light', 'gui.tier.high': 'Standard', 'gui.tier.max': 'Max', + 'gui.picker.title': 'Model', 'gui.picker.recent': 'Recent', 'gui.picker.show_all': 'Show all {n}', 'gui.wd.open_remote': "Browse the host's folders...", @@ -4569,6 +4570,7 @@ export const UI_TEXT: Record> = { 'gui.tier.medium': '轻', 'gui.tier.high': '标准', 'gui.tier.max': '全力', + 'gui.picker.title': '模型', 'gui.picker.recent': '最近使用', 'gui.picker.show_all': '显示全部 {n} 个', 'gui.wd.open_remote': '浏览主机上的文件夹…', diff --git a/ui-web/src/chrome/ModelChip.test.tsx b/ui-web/src/chrome/ModelChip.test.tsx index 171970685..3f1c54b5b 100644 --- a/ui-web/src/chrome/ModelChip.test.tsx +++ b/ui-web/src/chrome/ModelChip.test.tsx @@ -10,7 +10,7 @@ * de-dups by, because a provider's list mixes ids added by hand with ids the * vendor reports. */ -import { cleanup, render } from '@testing-library/react' +import { act, cleanup, render } from '@testing-library/react' import { afterEach, describe, expect, it } from 'vitest' import * as store from '../features/model/store' @@ -111,3 +111,17 @@ describe('the model chip', () => { expect(name()).toBe('My Model') }) }) + +describe('the model chip, to a screen reader', () => { + it('says whether the picker it opens is up', () => { + install([{ id: 'openrouter', name: 'OpenRouter', on: true, models: ['my-model'], configured: ['my-model'] }]) + store.setCurrent('my-model') + mount() + const chip = (): HTMLElement => document.getElementById('modelChip')! + expect(chip().getAttribute('aria-expanded')).toBe('false') + act(() => store.open(chip())) + expect(chip().getAttribute('aria-expanded')).toBe('true') + act(() => store.close()) + expect(chip().getAttribute('aria-expanded')).toBe('false') + }) +}) diff --git a/ui-web/src/chrome/ModelChip.tsx b/ui-web/src/chrome/ModelChip.tsx index 2c79a99e4..b009afb77 100644 --- a/ui-web/src/chrome/ModelChip.tsx +++ b/ui-web/src/chrome/ModelChip.tsx @@ -72,6 +72,7 @@ export function ModelChip(): JSX.Element { id="modelChip" title={current} aria-haspopup="true" + aria-expanded={model.isOpen() ? 'true' : 'false'} onClick={() => { if (openModelsForMissingProvider()) return model.open(null) diff --git a/ui-web/src/features/model/ModelPicker.test.tsx b/ui-web/src/features/model/ModelPicker.test.tsx index 6dfd41663..83317e96e 100644 --- a/ui-web/src/features/model/ModelPicker.test.tsx +++ b/ui-web/src/features/model/ModelPicker.test.tsx @@ -972,3 +972,76 @@ describe('the picker with nothing to offer', () => { expect(h.toasts).toEqual(['gui.picker.no_account']) }) }) + +/* What a reader who cannot see the list is told. The tick is a glyph and the + grouping is a heading, so both are shape alone until the markup states them. */ +describe('the model picker, to a screen reader', () => { + it('names the popover after what is being chosen', () => { + install() + mount() + openIt() + expect(pick()!.getAttribute('aria-label')).toBe('gui.picker.title') + }) + + it('names it after the slot instead, when a slot opened it', () => { + install() + mount() + act(() => { store.open(undefined, undefined, undefined, { kind: 'text', title: 'Planner model' }) }) + expect(pick()!.getAttribute('aria-label')).toBe('Planner model') + }) + + it('states the current model as the checked one of a set', () => { + store.setCurrent('claude-sonnet-5') + install() + mount() + openIt() + expect(rows('models').map((b) => b.getAttribute('role'))).toEqual(['radio', 'radio', 'radio', 'radio']) + expect(rows('models').map((b) => b.getAttribute('aria-checked'))).toEqual(['false', 'false', 'false', 'true']) + }) + + it('gives each account its own set, named after the account', () => { + install() + mount() + openIt() + /* Per account rather than one set for the picker: the same model can be + listed under Recent and under its account, and one set may hold one + checked member. An account with nothing to list owns no set. */ + expect(groups().map((g) => g.getAttribute('role'))).toEqual(['radiogroup', 'radiogroup', null]) + expect(groups().map((g) => g.getAttribute('aria-label'))).toEqual(['MiniMax (Global)', 'Anthropic', null]) + }) + + it('leaves the tick out of the row\'s spoken name, now that the state is said', () => { + /* Chrome builds a radio's name from its contents, so a glyph left visible + to it is read out beside the checked state it duplicates. */ + store.setCurrent('claude-sonnet-5') + install() + mount() + openIt() + const ticked = rows('models').find((b) => b.querySelector('.tick'))! + expect(ticked.querySelector('.tick')!.getAttribute('aria-hidden')).toBe('true') + }) + + it('leaves the show-all row out of the set it sits in', () => { + /* It reveals more of the account's radios rather than being one: a row that + answers to the set's state would be offered as a model to pick. */ + const many = Array.from({ length: 8 }, (_, i) => `router/model-${String(i)}`) + install({}, [{ id: 'router', name: 'Router', on: true, models: many, configured: many }]) + mount() + openIt() + expect(more()).not.toBeNull() + expect(more()!.getAttribute('role')).toBeNull() + expect(more()!.getAttribute('aria-checked')).toBeNull() + }) + + it('gives the recent group a set of its own', async () => { + install() + mount() + openIt() + await act(async () => { rows('models')[3]!.click() }) + openIt() + const recent = document.querySelector('.mpick .model-recent')! + expect(recent.getAttribute('role')).toBe('radiogroup') + expect(recent.getAttribute('aria-label')).toBe('gui.picker.recent') + expect(recents().map((b) => b.getAttribute('aria-checked'))).toEqual(['true']) + }) +}) diff --git a/ui-web/src/features/model/ModelPicker.tsx b/ui-web/src/features/model/ModelPicker.tsx index a7c955cd4..1b2c7fca8 100644 --- a/ui-web/src/features/model/ModelPicker.tsx +++ b/ui-web/src/features/model/ModelPicker.tsx @@ -114,11 +114,17 @@ function Row({ p, m, current, owner, account }: { p: Provider; m: string; curren did before the pick started naming one. */ const ticked = (!owner || owner.id === p.id) && sameModel(p, m, current) return ( - ) } @@ -285,7 +291,14 @@ function Pick(): JSX.Element { } return ( -
+ /* The popover says what is being chosen: the slot's name where one opened it, + the word for a model otherwise. Through `t` and not `lang.attr`, which the + chip popovers name themselves with -- that one withholds the text until a + reader picks a language, because the markup it writes over is served with + words already. Nothing here is served: the picker does not exist until it + is opened, so there is no served text to leave standing and a reader who + never picks a language would get no name at all. */ +
{at.offer.title ? {at.offer.title} : null} @@ -318,7 +331,7 @@ function Pick(): JSX.Element {
{recents.length ? ( -
+
{t('gui.picker.recent')}
{recents.map(({ p, m }) => )}
@@ -330,8 +343,17 @@ function Pick(): JSX.Element { on the way. */ if (q && !list.length) return null const { rows, more } = shown(p, list) + /* A set per account rather than one for the picker: a model listed by + an account is also listed under Recent, and both are marked, so one + set for the whole list would hold two checked members. */ return ( -
+
{p.name} From dd945cd419f19560ce4aff5f0b323377b3e647f8 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Wed, 23 Sep 2026 07:56:37 +0000 Subject: [PATCH 2/3] test(ui-web): assert the show-all row against the radios it sits beside Reverting the source left this one green while the other seven went red: it asserted only that the show-all row has no role and no aria-checked, which is equally true of a picker that marks no row at all. It shares the `row` class with the radios, so the negative half means something only next to the positive one. Co-authored-by: Claude (claude-opus-5[1m]) --- ui-web/src/features/model/ModelPicker.test.tsx | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/ui-web/src/features/model/ModelPicker.test.tsx b/ui-web/src/features/model/ModelPicker.test.tsx index 83317e96e..43ae8cbec 100644 --- a/ui-web/src/features/model/ModelPicker.test.tsx +++ b/ui-web/src/features/model/ModelPicker.test.tsx @@ -8,7 +8,7 @@ import * as pageStore from '../../state/page' import { resetSources, setSources } from '../../state/sources' import * as tier from '../../state/tier' import { domSnapshot } from '../../test/domSnapshot' -import { ModelApp } from './ModelPicker' +import { FOLD, ModelApp } from './ModelPicker' import * as store from './store'; import type { ModelSource, Provider } from './types' @@ -1023,13 +1023,18 @@ describe('the model picker, to a screen reader', () => { it('leaves the show-all row out of the set it sits in', () => { /* It reveals more of the account's radios rather than being one: a row that - answers to the set's state would be offered as a model to pick. */ - const many = Array.from({ length: 8 }, (_, i) => `router/model-${String(i)}`) + answers to the set's state would be offered as a model to pick. Asserted + against its siblings rather than on its own, because it carries their + `row` class: on a picker that marked no row at all, "the show-all row is + not a radio" is true for the wrong reason. */ + const many = Array.from({ length: FOLD + 2 }, (_, i) => `router/model-${String(i)}`) install({}, [{ id: 'router', name: 'Router', on: true, models: many, configured: many }]) mount() openIt() - expect(more()).not.toBeNull() - expect(more()!.getAttribute('role')).toBeNull() + const siblings = [...document.querySelectorAll('.mpick .models .model-group .row')] + expect(siblings.map((b) => b.getAttribute('role'))) + .toEqual([...Array(FOLD).fill('radio'), null]) + expect(siblings.at(-1)).toBe(more()) expect(more()!.getAttribute('aria-checked')).toBeNull() }) From 60d914275e8b7d7e4c54a58ee7e9489dd101b313 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Wed, 23 Sep 2026 09:48:53 +0000 Subject: [PATCH 3/3] fix(ui-web): report the chip expanded only for the picker it opened Review finding on the first head. `isOpen()` answers "a model picker is up somewhere", and one store serves every opener: the composer chip with no anchor, and each settings role pill against its own button. So opening a role such as Planner model from the settings dialog lit up the composer chip too, handing a screen reader a control and a state that do not belong together. The store already knows which control it was opened from -- it keeps the opener as `at.host` -- so it answers the narrower question itself through `openedFrom`, rather than the chip filtering a global. The chip asks about its own node. Reverting the predicate to `isOpen()` reddens the added case and nothing else. Co-authored-by: Claude (claude-opus-5[1m]) --- ui-web/src/chrome/ModelChip.test.tsx | 17 +++++++++++++++++ ui-web/src/chrome/ModelChip.tsx | 6 ++++-- ui-web/src/features/model/store.ts | 9 +++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/ui-web/src/chrome/ModelChip.test.tsx b/ui-web/src/chrome/ModelChip.test.tsx index 3f1c54b5b..3c29a2c90 100644 --- a/ui-web/src/chrome/ModelChip.test.tsx +++ b/ui-web/src/chrome/ModelChip.test.tsx @@ -124,4 +124,21 @@ describe('the model chip, to a screen reader', () => { act(() => store.close()) expect(chip().getAttribute('aria-expanded')).toBe('false') }) + + it('says nothing about a picker another control opened', () => { + /* One store serves every opener, so "a picker is up" is not the same + question as "mine is up": the settings role pills open it against their + own button (features/settings/providers/Roles.tsx). A chip that answered + the first would hand a reader a control and a state that do not belong + together. */ + install([{ id: 'openrouter', name: 'OpenRouter', on: true, models: ['my-model'], configured: ['my-model'] }]) + store.setCurrent('my-model') + mount() + const chip = (): HTMLElement => document.getElementById('modelChip')! + const pill = document.body.appendChild(document.createElement('button')) + + act(() => store.open(pill, undefined, undefined, { kind: 'text', title: 'Planner model' })) + + expect(chip().getAttribute('aria-expanded')).toBe('false') + }) }) diff --git a/ui-web/src/chrome/ModelChip.tsx b/ui-web/src/chrome/ModelChip.tsx index b009afb77..c5cd78b97 100644 --- a/ui-web/src/chrome/ModelChip.tsx +++ b/ui-web/src/chrome/ModelChip.tsx @@ -19,7 +19,7 @@ */ import { ArrowDown01Icon } from '@hugeicons/core-free-icons' -import { useSyncExternalStore } from 'react' +import { useRef, useSyncExternalStore } from 'react' import { Icon } from '../components/Icon' import { ProviderIcon } from '../components/ProviderMark' @@ -63,16 +63,18 @@ function serving(current: string): { provider: Provider; id: string } | null { export function ModelChip(): JSX.Element { useSyncExternalStore(model.subscribe, model.version) useSyncExternalStore(paint.subscribe, paint.get) + const me = useRef(null) const current = model.current() const at = serving(current) const label = at?.provider.labels?.[at.id]?.label || model.short(current) return (