close
Skip to content

fix: label the prune merged-scan as scanning and drop danger styling while it runs (#2451) - #2482

Merged
iamwhatever merged 1 commit into
mainfrom
fix/prune-scan-label-2451
Aug 10, 2026
Merged

fix: label the prune merged-scan as scanning and drop danger styling while it runs (#2451)#2482
iamwhatever merged 1 commit into
mainfrom
fix/prune-scan-label-2451

Conversation

@bolichen97

@bolichen97 bolichen97 commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes the ambiguous affordance on the Dev Fleet Prune merged button: while
busy['__prune'] is set the button now says what it is actually doing —
"Scanning for merged…" — instead of showing a spinner under the destructive
label, and the danger styling is suppressed for the scan's whole window.

Nothing is deleted during that window: pruneShipped only builds the candidate
list, and the review dialog's checkboxes remain the mandatory confirmation step.
A spinner on a danger-styled "Prune merged" read as "deletion already running",
which is exactly when a nervous user reloads and throws away the in-flight
git + gh walk.

Answering the issue's open question: yes, the danger styling is suppressed
during the scan.
The scan is read-only, so the destructive affordance is
misleading for its entire duration. This uses Btn's existing danger boolean
prop (danger={!busy['__prune']}) — no new prop, no new variant. The button
stays visibly disabled (disabled + the component's disabled opacity) and
aria-busy is unchanged for assistive tech.

Changes

  • website/src/pages/DevFleetPage.tsx — label swaps to the new
    pages.devFleetPage.scanning_merged key while the scan runs; danger becomes
    danger={!busy['__prune']}. No behaviour change to pruneShipped or the
    review-dialog confirmation flow.
  • website/src/i18n/locales/en.manual.json — new hand-authored key
    scanning_merged: "Scanning for merged…" (ellipsis style matches the
    neighbouring busy strings Building… / Loading…). en.json is
    codemod-generated and correctly untouched.
  • All 11 translated catalogs (bn, de, es, fr, hi, it, ja, ko, pt, ru, zh-CN) —
    real translations consistent with each file's existing prune_merged and
    busy-string wording, inserted in each file's sorted key position.
  • en-XA.json — regenerated via npm run i18n:pseudo (not hand-edited).
  • website/src/test/DevFleetPage.test.tsx — the existing in-flight spinner test
    now also locks the label swap, the danger suppression during the scan, and the
    restoration of both after it resolves.

Screenshots

Idle — destructive affordance on (hovered):

idle

Scanning — read-only label, spinner kept, danger suppressed, visibly disabled:

scanning

Full-page context: idle · scanning

Testing

  • npx tsc -b — clean
  • npx vitest run — full website suite: 876 files, 11794 passed
  • npm run i18n:check — full chain green (catalog parity, DNT, manifest sync)
  • Live browser verification via a throwaway Playwright harness (not committed):
    in-flight button reports aria-busy=true, label Scanning for merged…, and
    no danger classes; both states captured in the screenshots above.

Closes #2451

@bolichen97
bolichen97 requested a review from a team August 10, 2026 05:20
@bolichen97
bolichen97 requested a review from a team as a code owner August 10, 2026 05:20
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Aug 10, 2026
@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed bb3ef8c7fe41eb4927ddb257bf3a29fa67261ebb — this comment is updated in place on each push.

Review details

The diff is a small, self-consistent UX change: the button's danger prop and label swap based on busy['__prune'], plus the new scanning_merged i18n key across en.manual.json and all 11 translated catalogs + en-XA. Logic is correct in both states (idle → danger + "Prune merged" + Trash2; in-flight → no danger + "Scanning for merged…" + spinner, disabled). en.json correctly untouched (codemod-generated). No behavioral change to pruneShipped or the confirmation flow.

No findings.

[OPUS-REVIEWED] bb3ef8c

Verdict parsed from the review's SHA-scoped output markers for commit bb3ef8c7fe41eb4927ddb257bf3a29fa67261ebb.

False positive or not applicable? A repository writer can comment:
/ai-review override fable bb3ef8c7fe41eb4927ddb257bf3a29fa67261ebb: <one-sentence reason>

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — ✅ PASS

Advisory design-level review of bb3ef8c7fe41eb4927ddb257bf3a29fa67261ebb — updated in place on each push; does not block merge.

Design-Verdict: PASS

Real misleading-affordance harm, fixed at the right layer with existing state and the Btn prop — no new surface, fully reversible.

[DESIGN-REVIEWED] bb3ef8c

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — ✅ PASS

Advisory UX-level review of bb3ef8c7fe41eb4927ddb257bf3a29fa67261ebb — updated in place on each push; does not block merge.

UX-Verdict: PASS

Busy label now truthfully names the read-only scan; confirmation dialog still guards deletion — the screenshots confirm both states render as claimed.

Suggestions

  • scanning-toolbar.png: "Scanning for merged…" renders at disabled opacity — near-ghost beside full-contrast "Refresh" — yet it is the page's only progress feedback; exempt the in-flight label (and spinner) from the disabled dimming so the state it exists to communicate is legible at a glance.
  • The wider busy label pushes "3 rows" and the "Sort: status" control ~50px left for the scan's duration, then snaps back (compare idle-full.png vs scanning-full.png); give the Btn a min-width spanning both labels so the toolbar holds still while the user waits.

[UX-REVIEWED] bb3ef8c

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of bb3ef8c7fe41eb4927ddb257bf3a29fa67261ebb and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] bb3ef8c

False positive or not applicable? A repository writer can comment:
/ai-review override gpt bb3ef8c7fe41eb4927ddb257bf3a29fa67261ebb: <one-sentence reason>

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 10, 2026
…while it runs (#2451)

While `busy['__prune']` is set, the Prune merged button now renders a
dedicated "Scanning for merged…" label (new `pages.devFleetPage.scanning_merged`
key, translated into all 12 catalogs) instead of keeping the destructive label
under a spinner, and the `danger` variant is suppressed for the scan's whole
window — the scan is read-only (`pruneShipped` only builds the candidate list;
the review dialog remains the mandatory confirmation step), so the destructive
affordance was misleading exactly when a nervous user might reload and throw
away the in-flight git+gh walk.

Answering the reporter's open question: yes, the danger styling is suppressed
during the scan, via the existing `danger` boolean prop on `Btn` (no new prop
or variant). The button stays visibly disabled and `aria-busy` is unchanged.

Closes #2451
@bolichen97
bolichen97 force-pushed the fix/prune-scan-label-2451 branch from 5d221b7 to bb3ef8c Compare August 10, 2026 05:55
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 10, 2026
@iamwhatever
iamwhatever merged commit 041c32e into main Aug 10, 2026
58 of 60 checks passed
@iamwhatever
iamwhatever deleted the fix/prune-scan-label-2451 branch August 10, 2026 16:38
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Aug 10, 2026
encomjp pushed a commit to encomjp/kirocrew-customapi that referenced this pull request Aug 22, 2026
…while it runs (kirodotdev#2451) (kirodotdev#2482)

While `busy['__prune']` is set, the Prune merged button now renders a
dedicated "Scanning for merged…" label (new `pages.devFleetPage.scanning_merged`
key, translated into all 12 catalogs) instead of keeping the destructive label
under a spinner, and the `danger` variant is suppressed for the scan's whole
window — the scan is read-only (`pruneShipped` only builds the candidate list;
the review dialog remains the mandatory confirmation step), so the destructive
affordance was misleading exactly when a nervous user might reload and throw
away the in-flight git+gh walk.

Answering the reporter's open question: yes, the danger styling is suppressed
during the scan, via the existing `danger` boolean prop on `Btn` (no new prop
or variant). The button stays visibly disabled and `aria-busy` is unchanged.

Closes kirodotdev#2451
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dev Fleet: a spinner on the danger-styled Prune merged button reads as "deleting", not "scanning"

2 participants