Uh oh!
There was an error while loading. Please reload this page.
fix(desktop): point link token at solid accent - #2701
Conversation
7f5022d to
e2d50baCompare@Astro-Han This is ready for review. It follows the exact #2505 scope, includes rendered light/dark regression coverage and before/after screenshots. CI baseline is now isolated in clean worktrees: at da9f948, settings.spec.ts passes 3/3; at current-main 94e6775 (#2700), it fails 0/3 in the shared fixture before assertions, with the onboarding screen visible and the composer hidden. The fixture seeds only the legacy llm-connections path, while #2700 removes the legacy runtime-policy migration, leaving the fresh Runtime Host catalog empty. @jackwener, could you advise on the current-main fixture/bootstrap follow-up? The typecheck job is independently stopped by the same 62-file formatter baseline present on main. |
e2d50ba to
214773fCompare@Astro-Han Thank you for the review. Your requested rebase and smaller standalone rendered contract are now complete: the removed Settings walkthrough stays deleted, the real documentation link is checked in Tokyo Night light/dark, and fresh CI is fully green on final SHA 089d5b5. Could you please confirm the final SHA when convenient? — posted by Codex for @MoonOld |
Astro-Han
left a comment
There was a problem hiding this comment.
Approved — I found no P0–P3 issues in the production change.
This fixes the token at the right authority boundary:
- link text now derives from
--accent-solid, which owns the cross-palette text-contrast guarantee; - the two washes and two borders remain derived from the raw interaction accent;
- genuine link and copied-state text continues to follow
--link.
The rendered before/after evidence also matches the intended result: the light Tokyo Night link becomes clearly readable without changing the surrounding interaction hierarchy.
One friendly suggestion for the rebase: the branch is currently conflicting because the large Settings E2E block extended by this PR has since been removed from main. I would avoid restoring that deleted omnibus test. The useful regression can be preserved as a smaller, standalone rendered contract that:
- opens a real Settings documentation link;
- selects Tokyo Night;
- verifies in light and dark modes that the link's computed color equals
--accent-solidand differs from--accent.
That would retain the meaningful browser-level coverage while following the current, smaller test seam.
The previous red CI was consistent with the old base, but current main is green. Please rebase and obtain a fresh complete CI result before merging. None of this changes my approval of the implementation itself.
Please feel free to push back if there is a newer rendered-theme seam that would be a better home for the regression.
AI-assisted review disclosure
Codex inspected the issue contract, current PR diff, current main, token consumers, rendered screenshots, test ownership, and CI/merge state. Astro-Han reviewed the evidence and made the final decision to approve the production change while recommending a smaller test after rebase.
中文对照(默认折叠)
批准。生产改动中没有发现 P0–P3。
这次修改落在正确的 token authority 上:链接文本由保证跨 palette 文本对比度的 --accent-solid 派生;两个 wash 和两个 border 继续使用 raw interaction accent;真正的链接和复制成功文本继续使用 --link。渲染前后截图也符合预期,浅色 Tokyo Night 下链接明显更清晰,同时没有改变周围的交互层级。
有一个友好建议:当前分支发生冲突,是因为本 PR 扩展的那段大型 Settings E2E 已经被 main 删除。Rebase 时建议不要恢复这段被删除的综合测试。可以保留一个更小、独立的 rendered contract:打开真实 Settings documentation link,选择 Tokyo Night,然后在 light/dark 下验证 computed color 等于 --accent-solid 且不同于 --accent。
这样既保留真正有价值的浏览器渲染覆盖,也符合 current main 更小的测试边界。
此前红色 CI 与旧 base 一致,但 current main 已经恢复绿色,因此合并前仍需 rebase 并取得新的完整 CI。这个前置条件不改变对生产实现本身的批准。
如果仓库现在已有更合适的 rendered-theme 测试 seam,也欢迎直接 push back。
本次由 Codex 检查 issue contract、PR diff、current main、token consumers、渲染截图、测试 owner 和 CI/merge 状态;Astro-Han 复核证据,并决定批准生产修改,同时建议在 rebase 后缩小测试。
Generated-by: Codex
214773f to
089d5b5CompareUh oh!
There was an error while loading. Please reload this page.
Summary
Fixes#2505
Verification
npm run lintandnpm run format:checknpm run astryx:theme -- --checknpx knip --workspace apps/desktopandnpx knip --workspace packages/uinpm run buildandnpm run typecheckmainworktreeChecklist
Does this PR entail a change in behavior?
AI-assisted implementation and review are disclosed by the
Generated-by: Codexcommit trailer. Independent human review is requested because this changes user-visible link colors.