refactor(ui): let Astryx clickAction own the in-flight button state - #2089
Merged
Conversation
Fourteen buttons and three switches reported their own work by renaming themselves — 导出中… / 测试中… / 删除中… — and hand-rolled the rest of the affordance beside it: an isDisabled flag, a data-pending attribute, an aria-busy attribute, and a mounted-ref guard so the settle didn't setState after unmount. A label swap is the weakest of those signals. A screen reader has to re-read the button to notice it, the text reflows mid-click, and none of it dedupes the same-tick second click that a fire-once action (export, save, delete) most needs to survive. Astryx's clickAction/changeAction already carry the whole contract: a ref guard that dedupes within the tick (which neither isPending nor useOptimistic does), automatic aria-busy, a spinner whose reveal is delayed so a fast action doesn't flash, a live-region "Loading" announcement, and disable-while-pending. Switch adds an optimistic value that reverts if the action rejects. What did NOT move: every cross-control lock stays exactly as it was. configBusy still means "one config operation at a time", detailActionBusy still serializes a connection's actions, pendingSkillAction still tells the skill library which row is busy, and voice's `busy` still freezes the three fields beside its save button. clickAction speaks for one control; those speak for a group, and the two are complementary. Per the same rule, optimistic-settings-draft-controller and use-action-guard are untouched — they hold cross-render save tickets and last-write-wins, which is not something a button can know. mcp-page keeps type="submit" and its form onSubmit so Enter in a field still saves; it takes isLoading, which is the half of the contract that applies to a submit button. Removes seven now-orphaned copy keys across six locale modules (exporting/importing, testing/updating/deleting/importing, opening/saving/copying/deleting, refreshing, saving/importing, recognitionConnectionSaving) in both zh and en. Verified: build, typecheck, format:check, check-dead-css, test:checks, and the desktop + ui workspace suites. storage and cli fail identically on a clean checkout of this commit's parent (Node v25 prints a node:sqlite ExperimentalWarning that those tests assert stderr against) — unrelated to this change, which touches neither package.
Review caught a real regression, and the CI e2e failure was it, not a flake: the three delete buttons converted to clickAction never showed their confirm dialog. clickAction runs its handler inside `startTransition(async …)`. React holds state commits made during an async transition until the action settles. `toast.confirm` is React state — it needs four sequential commits to resolve its promise (mounting → rAF → open → user choice → rAF → resolve). So the dialog could not render until the action settled, and the action was awaiting the dialog. Deadlock; the alertdialog never appeared and the button sat spinning. Reproduced independently before fixing, with a minimal probe that skips the CDP short-viewport step (that step is what makes providers.spec unusable locally): on the previous commit the probe fails with "element(s) not found" on the alertdialog, on this one it passes. The three confirm-gated buttons go back to onClick and take `isLoading` for the affordance. That keeps this PR's actual goal — the label stays 删除 instead of renaming itself to 删除中… — while staying out of the transition. isLoading counts into the button's disabled state, so the spinner, aria-busy, and the disable all still come from the component. Everything else keeps clickAction. Audited every converted handler for a reachable confirm, following the callbacks out of the components: openInFinder / saveAs / copyText, runTest / refreshModels, connect, export / import, voice save, refreshSkills / openSkillsFolder, and the three Switches' onUpdate are all confirm-free. Also applies the review's consistency nit: export / import / save now pass through an arrow like refreshModels already did, so no handler can silently receive the click event as its first argument. Rule this establishes, for the rest of the Astryx migration: never await UI driven by same-tree React state from inside clickAction/changeAction. A confirm-gated action belongs on onClick + isLoading. Verified after rebasing onto main: build, typecheck, format:check, check-dead-css, test:checks, desktop + ui workspace suites, and the delete-confirm probe. providers.spec fails locally at an unrelated step — a network-error toast covers the delete button in the 500px viewport because the sandbox is offline — identically on clean origin/main, same test, same step, same locator. storage and cli remain the known Node v25 node:sqlite ExperimentalWarning failures.
jackwener
force-pushed
the
pr-astryx-click-action-1
branch
from
August 4, 2026 06:18
ca32265 to
6baed50
Compare
Member
Author
|
Review by maka-审美专家 — 通过（第二轮）：3 个 confirm 门控按钮回 onClick + isLoading 的修复复验通过——本地重建后 delete-confirm probe 由挂转过，CI e2e_shard(1/2)（首轮抓到死锁的关卡）全绿。首轮意见全部落实：范围重审、箭头包装 nit、规则入册。合入。 |
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.
Task #137 —【Astryx 落地 ①】clickAction/changeAction 乐观异步替换手写 busy 态.
问题
改造点上的按钮用改自己的名字来汇报进度（
导出中…/测试中…/删除中…），旁边再手写一整套：isDisabled、data-pending、aria-busy、以及一个 mountedRef 守卫防止结束时 setState-after-unmount。标签互换是这几个信号里最弱的一个：读屏必须重新读一遍按钮才会发现；文字在点击瞬间重排；而且它完全不防同 tick 双击——恰恰是 export / save / delete 这类 fire-once 操作最需要防的。
做法
Astryx 的
clickAction/changeAction原生带全套契约（Button.tsx:617-670、Switch.tsx:431-433,490-500已核实）：actionInFlightRef同 tick 去重 —— 官方注释写明isPending和useOptimistic都做不到aria-busy"Loading"播报isInterruptible可豁免）明确没有动的东西
所有跨控件的锁原样保留 —— 它们和 clickAction 是互补的两层，不是替代关系：
configBusydetailActionBusypendingSkillActionbusyrunArtifactAction的 ref按规格要求，
optimistic-settings-draft-controller.ts和use-action-guard.ts一行未动 —— 它们管的是跨渲染的 save ticket / last-write-wins，不是单控件反馈。两处偏离规格，请 review 时确认
mcp-page.tsx:719,772用isLoading而不是clickAction。 这两个是type="submit"，装在<form onSubmit>里。换成 clickAction 就得把 form 的 onSubmit 一起端掉，输入框里按 Enter 提交会失效——这是真实回归。isLoading是 submit 按钮能用的那一半契约：spinner + aria-busy + 播报都在，只是不接管点击。artifact-pane.tsx的 4 个工具栏按钮不再彼此禁用。 规格说「pendingArtifactAction 整套 state 可删」，我照做了：state 删掉，ref 留下继续串行。代价是——action A 跑的时候，B 按钮不再显示为禁用，点它是静默 no-op（ref 挡掉了，不会并发）。考虑到 saveAs 开的是窗口级 modal 原生对话框、其余操作都是亚秒级，我判断可以接受；但如果你要「兄弟按钮可见地禁用」，把 state 加回来只喂isDisabled也行，说一声我改。顺带清理
删掉 6 个 locale 模块里 7 组孤儿 copy key（zh + en 都删）：
exporting/importing、testing/updating/deleting/importing、opening/saving/copying/deleting、refreshing、saving/importing、recognitionConnectionSaving。每个都 grep 确认过全仓无引用。验证
npm run buildnpm run typechecknpm run format:checkcheck-dead-css.mjs --checktest:checks（console / a11y / copy）命令均未接
| tail管道，退出码真实。storage和cli两个 workspace 失败，但在本 commit 的 parent 上 stash 后重跑失败完全一致（Node v25 会打node:sqliteExperimentalWarning，这些测试断言 stderr 为空）。与本 PR 无关，本 PR 一行都没碰这两个包。同一模式还没改的地方（不在本次规格里，留作后续）
data-settings-page的 opening/copying/clearing 三连、artifact-pane的列表重试按钮、general-settings:403、web-search-settings:300、bot-chat-detail:241,246、about-settings:183、password-input:118、memory-settings:344,360,400、permission-center:520、claude-subscription-card:380、bot-wechat-login:188、provider-oauth-section:275、mcp-page:326,615、plan-reminder-panel:575、skills-panel:454,571,572。要不要单开一个 PR 扫干净，你定。@maka-审美专家 请 review。