fix(desktop): announce successful settings save to assistive tech#356
Merged
Conversation
The SettingsPanel failure path announces via role="alert", but a
successful save ("✓ 已保存") was a plain conditionally-mounted span
with no live-region, so screen-reader users got no confirmation the
save worked (WCAG 2.1 SC 4.1.3 status-message parity gap).
Render the success confirmation as an always-present polite status
live-region (role="status" aria-live="polite") whose text toggles on
the existing `saved` flag, so the inserted text is reliably announced.
Visual affordance is unchanged; .settings-saved margin is guarded with
:not(:empty) so the always-rendered empty span adds no layout gap.
Closes #355
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
代码评审报告: fix(desktop): announce successful settings save to assistive tech
风险等级: 中
处理建议: 请求修改
决策摘要: 暂不建议合并：当前把 role="status" 放在 native <button> 内部，按钮后代语义通常会被平台无障碍树扁平化/呈现化，成功消息 live-region 可能无法可靠暴露，直接影响 #355 的核心验收项。
级联分析
- 变更符号:
SettingsPanel的保存按钮成功状态标记；.settings-savedCSS 间距规则 - 受影响流程: 桌面端设置保存成功后的可访问状态反馈；
App → SettingsPanel → save - 变更集外调用方: GitNexus 摘要显示 d=1
App，d=2main.tsx；无 IPC、controller、store 或导出签名变更 - 置信度: medium
问题发现
- [中] 成功 live-region 放在
<button>内，可能无法作为 status 暴露给辅助技术- 证据: diff 将
<span className="settings-saved" role="status" aria-live="polite">作为保存按钮的子节点渲染。native button / button role 的后代在无障碍树中通常会被扁平化为按钮名称文本，子元素语义角色不可靠保留；因此role="status"可能不会成为独立 live region。 - 受影响调用方/流程: 设置保存成功后，屏幕阅读器用户仍可能收不到 #355 要求的 polite status announcement；这正是本 PR 要修复的验收路径。
- 最小可行修复: 将 always-present
role="status" aria-live="polite"节点移出<button>，作为按钮旁边的 sibling 或单独的 visually-hidden status region；如需保留按钮内视觉 “✓ 已保存”，可将视觉副本标为aria-hidden="true"，由按钮外的 live-region 负责公告。
- 证据: diff 将
行级发现
- [apps/desktop/src/renderer/components/SettingsPanel.tsx:59]
role="status"放在 native<button>的子元素上不可靠，按钮后代语义可能被无障碍树扁平化，导致 live-region 不被公告；请把 status live-region 移到按钮外，必要时保留按钮内aria-hidden的视觉副本。
Karpathy 评审
- 假设: 当前实现假设按钮内部的
span[role=status]会作为 live-region 暴露；这是浏览器/AT 组合中不稳的隐式语义假设。 - 简洁性: 变更很小，但把公告语义嵌进按钮标签内会把“按钮名称”和“状态公告”两个概念耦合；按钮外独立 status region 更直接。
- 结构质量: 无导出契约或控制器层级退化；主要问题是 ARIA 语义边界放错层，影响可验证性和可靠性。
- 变更范围: 范围符合 issue 约束，CSS 改动局部；但 issue 写的“single file”与 PR 增加 CSS 文件不完全一致，当前 CSS 是为避免空 span layout gap 的小范围配套改动，不是主要风险。
- 验证: typecheck/test/biome 不能证明实际 AT 公告；修复后需要基于目标浏览器 + SR 的手动验证确认按钮外 live-region 在成功保存时公告。
缺失覆盖
- 修复后补充/更新手动 a11y 验证说明：确认
role="status"节点不在<button>内，保存成功时从空内容变为 “✓ 已保存” 并被 polite announce，失败role="alert"行为不变。
| <button type="button" className="btn btn-primary" onClick={save} disabled={saving}> | ||
| {saving ? '保存中…' : status === 'save-error' ? '重试保存' : '保存设置'} | ||
| {saved ? <span className="settings-saved">✓ 已保存</span> : null} | ||
| <span className="settings-saved" role="status" aria-live="polite"> |
There was a problem hiding this comment.
role="status" 放在 native <button> 的子元素上不可靠，按钮后代语义可能被无障碍树扁平化，导致 live-region 不被公告；请把 status live-region 移到按钮外，必要时保留按钮内 aria-hidden 的视觉副本。
repo-guard (CHANGES_REQUESTED) correctly noted that a role="status" live-region nested inside a native <button> is unreliable: button descendants are flattened into the button's accessible name, so the nested live region may never be exposed to assistive tech — defeating #355's acceptance criterion. Move the always-present role="status" aria-live="polite" region out of the <button> to be its sibling, so it is a top-level live region that reliably announces the "✓ 已保存" success text. The existing .settings-saved:not(:empty) margin rule still spaces it from the button; the role="alert" failure path is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
代码评审报告: fix(desktop): announce successful settings save to assistive tech风险等级: 中 级联分析
问题发现
行级发现
Karpathy 评审
缺失覆盖
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
SettingsPanelannounces a save failure to assistive tech viarole="alert", but a save success ("✓ 已保存") was a plain conditionally-mounted<span>with no live-region — so screen-reader users got no confirmation the save actually worked. This is a WCAG 2.1 SC 4.1.3 (Status Messages) parity gap.Closes #355
Change (single component + minimal CSS)
SettingsPanel.tsx: render the success confirmation as an always-present polite status live-region (role="status" aria-live="polite") whose text toggles on the existingsavedflag. A live region must exist in the DOM before content is inserted to be announced reliably, so the container is always rendered and its text is empty untilsaved. The failurerole="alert"paths are untouched.theme.css: guard.settings-saved'smargin-leftwith:not(:empty)so the now-always-rendered (empty when unsaved) span introduces no layout gap. Visual affordance is otherwise identical.No change to
settingsController, the IPC contract, or any other file. Live-region text is kept equivalent to the existing "✓ 已保存" visual cue (per repo-guard's optional issue-review note).Verification
pnpm --filter @frontagent/desktop typecheck— cleanpnpm --filter @frontagent/desktop test— 96/96 passbiome check .— clean (only the pre-existinguseTemplateinfo in hallucination-guard, unrelated)quality:precommitpre-commit hook — green (lint/typecheck/test/test:workflows 32/32)saved=falsethe status region is empty and adds no visible gap; on a successful save it receives "✓ 已保存" and a polite-live screen reader announces it without moving focus.Follows the desktop no-DOM-test architecture (no
@testing-library/jsdom); semantics verified via typecheck + biome a11y rules + controlled diff + this manual note, consistent with #348/#350.GitNexus Impact Summary
impact(SettingsPanel, upstream)→ risk LOW, d=1App(renders it), d=2main.tsx; 1 process (App → Submit, step 1), moduleComponents.detect_changes(all)→ 1 changed symbol (SettingsPanel, touched), 0 affected processes, risk low. theme.css is non-symbol CSS.