[IAmJSD]

This tracker lists seven small UI problems; this PR fixes five of them. The header said "Logout", the Purchasing policies tab was headed "Purchasing Settings", help text cited a "Settings → Purchasing" path that no longer exists, claimed overrides were AWS-only and described Azure WIF as an RSA certificate login; toasts kept counting down while being read, the Approval-queue Cancel and History Retry confirms didn't name the purchase, and the ladder timeline stayed stale after a kill-switch toggle or config save until reload.

Refs #728
Partial: the full sentence-case sweep, shorter success toasts with sticky errors, and UX-30 (approval links ignoring purchase status) are left out; UX-37 did not reproduce on a14dc8f2.

What changed

One commit per item, so each can be taken or dropped on its own (2dcbcb5, aacd3ae, 674a94e, aa37ff0). Also on the branch: 5 merges of main (1c8489c, ed4b1e6, 4710317, 56a2219, a006d14) and 3 follow-ups (8b68eec clicks Cancel in the Approval queue in the e2e; c85b919 uses "Log out" in tests merged from main; 150979e applies #181's no-commitments guards to the confirm summary):

• VIS-23 (specific strings) and UX-32, frontend/src/index.html and frontend/src/settings.ts: "Log out", panel heading "Purchasing policies", overrides help path "Admin → Purchasing policies" (auth.test.ts, mobile-nav.test.ts, tests-e2e/mobile-header-drawer.spec.ts and tests-e2e/crosstab-storage-clear.spec.ts are updated). The add-override help no longer says AWS-only, and the Azure WIF tooltip now describes the OIDC federated credential with no secret or certificate stored.
• VIS-25, frontend/src/toast.ts: the countdown pauses on hover and on focus inside the toast, and resumes with the time left (at least 1 s) when both leave. In settings.ts the override Undo toast goes from 5 s to 10 s.
• UX-29, new frontend/src/history-confirm-body.ts: builds a summary of the row (provider, service, resource × count, region; account, term, upfront, created date) that history.ts puts above the existing text in the Approval-queue Cancel and History Retry confirms. The row is looked up from the already loaded History / Approval-queue data. It follows fix(frontend): impossible/placeholder values surfaced for scheduled and pending purchases #181's row guards (lib/purchase-row-guards.ts): a row with no commitments reads "No recommendations attached" with no upfront, service or region; a "multiple" provider or region is left out; and a term is shown only if it is a real one.
• UX-35, frontend/src/ladder-timeline.ts: exports refreshLadderTimeline(configs, enabled?), which updates the mounted timeline's inputs and reloads it while keeping provider, account and filters. ladder.ts calls it after a successful kill-switch toggle and after a config save.

Why this approach

The 30 s default toast timeout for all kinds was an explicit owner choice in the original toast commit, so only the uncontroversial WCAG 2.2.1 pause on hover/focus is added here, not shorter success toasts or sticky errors. The Cancel and Retry confirms reuse data already on screen instead of fetching details, so Cancel and Retry stay instant and still work when the details fetch is forbidden. The ladder refresh updates the mounted timeline instead of re-mounting it, so selections survive.

Screenshots

Screenshots were taken before this branch was merged with main; before = main at a14dc8f. After is this branch. Both are headless Chromium at 1440x900 against the repo's mocked e2e API.

Before	After

• Purchasing policies: before, "Logout" in the header and the tab headed "Purchasing Settings"; after, "Log out" and a heading that matches the tab.
• Add override for an Azure account: before, "Only AWS services are supported today" with "Settings → Purchasing" in the dialog behind; after, accurate help text and "Admin → Purchasing policies".

The named Cancel and Retry confirms, the toast pause (behavior over time) and the ladder refresh (behind the laddering flag) have no screenshots here. The earlier Cancel confirm shot is not shown because it put Cancel in the History list, and main now shows Cancel only in the Approval queue. All three are covered by the e2e and jest tests below.

Verification

Run on this branch's head a006d14d, on top of main da896f42, from frontend/:

• Merge conflicts were resolved in the merge commits: ladder-timeline.ts and __tests__/ladder-timeline.test.ts in 1c8489c, and the history.ts utils import and __tests__/ladder-timeline.test.ts in 56a2219. The last merge (a006d14) was clean.
• lint: 0 errors (existing warnings only); typecheck and build pass.
• jest: 157 suites, 4210 of 4211 passed, 1 skipped. Earlier, --randomize over the toast, ladder, ladder-timeline, history-cancel/retry and settings-accounts tests: 176/176.
• New jest tests: toast.test.ts (4: hover pause/resume, focus pause across buttons, 1 s minimum on resume, sticky stays sticky); named confirms in history-cancel-button.test.ts and history-retry-button.test.ts; purchases-placeholders-181.test.ts (4: Cancel and Retry confirms on a no-commitments row show "No recommendations attached" and no placeholders, a real row's confirm shows "Upfront: $4,200" with the real formatter, a "multiple" region is left out); ladder-timeline.test.ts (refresh after config save, then kill switch); ladder.test.ts asserts the refresh on toggle and save; the settings-accounts Undo TTL assertion is now 10 s.
• Playwright: new tests-e2e/history-confirm-and-toast.spec.ts (2: named Cancel confirm in the Approval queue; a toast survives 20 s of hover under page.clock and expires after the pointer leaves). Full suite: 247 passed.
• Secrets scan clean (planted key flagged); allowlist test: 137 PASS.

Notes for review

• Done: VIS-23 (only the specific strings: "Log out", the Purchasing policies heading, the stale "Settings → Purchasing" path), VIS-25 (pause on hover/focus and a 10 s Undo), UX-29, UX-32, UX-35.
• UX-37 did not reproduce: on a14dc8f2 a direct load of /admin/general (and /admin) sets the title CUDly — Admin · General, checked live against the before build with Playwright. switchSettingsSubTab sets the title in frontend/src/navigation.ts:334 before its self-switch return. Since the reviewed b50ca383, the only document.title change is fix(frontend): show an error state when init() fails, not a silent Home #1338, which sets an error title when init() fails; that is a separate path. The reviewer may have hit a different path (for example a permission-gated admin load); worth a second look.
• Left out:
  • The full sentence-case sweep of headings and buttons in index.html: a large diff over lines many other UI branches edit, and it needs a decision on the style rule.
  • VIS-25's shorter success/info default and sticky errors: this contradicts the documented 30 s default, so it's an owner decision.
  • UX-30 (approval links ignoring purchase status): needs the deeplink flow to gate Approve on details.status and show the requester. The issue marks it unverified end to end, so it is better done on its own with a live check.
• Copy to check: the add-override help ("The services listed are the ones available for this account's cloud provider."), the Azure WIF tooltip, and the confirm summary format. The confirm shows the account ID when the account-name cache isn't loaded on the History page (as in the screenshot), and the friendly name otherwise.

Overlaps

Textual conflicts with the open branches for #742 (__tests__/ladder.test.ts; it also changes saveLadderConfig in ladder.ts next to the refresh call here) and #730 (the line next to this one in tests-e2e/mobile-header-drawer.spec.ts). ladder.ts is 503 lines on this branch (main alone is 501), so it is already past 500 before #742 lands. #753, #739 and #731 are merged; their conflicts are resolved in the merges of main. Other open branches edit index.html (#749, #751), history.ts (#737, #751) and settings.ts (#751) without touching the same lines. #181 is merged, and the confirm summary here applies its no-commitments guards.

Note for the UI owner

This changes UI code that @cristim owns. IAmJSD reviewed the before/after screenshots and asked for it to be merged; any design point you disagree with can be a follow-up.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 19 billable files and costs up to $4.75.}

Or wait 59 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: LeanerCloud/cloud-commitments-platform
• Branch: main
• File: .coderabbit.yaml
• Commit: 10f2c8a
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 41b45c78-1b39-4474-9fe6-f4ebb3cd3540

Commits

Reviewing files that changed from the base of the PR and between 10f2c8a and 20a9b80.

Files selected for processing: 19

• frontend/src/__tests__/auth.test.ts
• frontend/src/__tests__/history-cancel-button.test.ts
• frontend/src/__tests__/history-retry-button.test.ts
• frontend/src/__tests__/ladder-timeline.test.ts
• frontend/src/__tests__/ladder.test.ts
• frontend/src/__tests__/mobile-nav.test.ts
• frontend/src/__tests__/purchases-placeholders-181.test.ts
• frontend/src/__tests__/settings-accounts.test.ts
• frontend/src/__tests__/toast.test.ts
• frontend/src/history-confirm-body.ts
• frontend/src/history.ts
• frontend/src/index.html
• frontend/src/ladder-timeline.ts
• frontend/src/ladder.ts
• frontend/src/settings.ts
• frontend/src/toast.ts
• frontend/tests-e2e/crosstab-storage-clear.spec.ts
• frontend/tests-e2e/history-confirm-and-toast.spec.ts
• frontend/tests-e2e/mobile-header-drawer.spec.ts

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): APPROVE 8b68eecd6086a944ddb40d40236921c3f80a3738

Blocking:

• none
  Non-blocking:
• frontend/src/tests/history-cancel-button.test.ts:254: the new test sits under "History inline Cancel button" and uses an unscoped .history-cancel-btn query. Since main's feat(frontend): make the Approval queue the one approval surface #1331, Cancel renders only in the Approval queue (renderPendingActionButtons, history.ts:1887). The assertion is still valid, but the name misleads.
• frontend/src/history-confirm-body.ts:38: the account name comes from getAccountName, which falls back to the raw ID when the recs cache is cold. The body notes this, and it matches the queue row on main.
• dashboard.ts:649/690 and history.ts:1396-1692 keep 5 s success toasts. Only the override Undo moves to 10 s, which matches the stated scope.
  Attacked: Read the full origin/main...HEAD diff. toast.ts is unchanged on main since the branch point. Walked the pause/resume state machine: sticky toasts stay sticky, focusout within the toast is ignored, the timer is cleared on dismiss, and nothing auto-focuses toasts. The ladder-timeline merge vs both parents: main's disposeLadderTimeline, the hidden-section early return and chart = undefined are kept, and the branch's updateTimeline = undefined is added in the same closure. There is no await before assignment, so there is no dispose race. The test merge keeps the fix(frontend): low-severity copy, toast, confirm and stale-state fixes #728 refresh test and main's fix(ladder): timeline editor 409 recovery, focus and chart cleanup #667 teardown. Follow-up 8b68eec: Cancel lives only in #purchases-approval-queue, findHistoryRow falls back to lastPendingForQueue, and confirmDialog accepts an HTMLElement body. The copy was checked against main: the sub-tab is "Purchasing policies", and overrides are per-provider. Jest (toast, history-cancel, history-retry, ladder, ladder-timeline, settings-accounts): 192/192.

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): REQUEST_CHANGES 56a2219562c7219252e9a95256451cd7c3d9b634

Blocking:

• frontend/src/history-confirm-body.ts:15-18,38: the new confirm summary ignores main's fix(frontend): impossible/placeholder values surfaced for scheduled and pending purchases #181 guards (fix(frontend): stop rendering placeholder values on pending purchases #1329, lib/purchase-row-guards.ts hasNoCommitments), which landed after the last review. Main deliberately stopped printing the placeholder values of a no-commitments execution: count 0, resource_type "0 commitment(s)", provider/service/region "multiple", upfront $0. Scenario: an admin clicks Cancel on such a pending row in the Approval queue. The row reads "No recommendations attached" with dashes, but the destructive confirm reads "multiple 0 commitment(s) in multiple / Account: … · Upfront: $0 · Created: …". I reproduced this in a scratch copy by calling historyConfirmBody with purchases-placeholders-181.test.ts's EMPTY_PENDING shape. The same applies to Retry on a failed empty row. Fix: when hasNoCommitments(p), use NO_RECOMMENDATIONS_LABEL instead of the description and skip upfront, service and region. Treat region 'multiple' like provider 'multiple'. Add a test with the EMPTY_PENDING fixture.
  Non-blocking:
• frontend/src/tests/history-cancel-button.test.ts:255: the test sits under "History inline Cancel button" while Cancel now renders only in the Approval queue (feat(frontend): make the Approval queue the one approval surface #1331). It also asserts "Upfront: $4200", which only passes because formatCurrency is mocked to $${val}. The real output is "$4,200", so the assertion does not pin the shipped format.
• frontend/src/ladder.ts: 503 lines (main 501). The note in the body anticipated crossing 500. That threshold has now passed, before fix(frontend): ladder config asks for a raw account UUID and hides auto-approve impact #742 even lands.
  Attacked: The two-dot and three-dot diffs vs origin/main are identical. Merges: 1c8489c combines the disposeTimeline closure (chart = undefined from fix(ladder): dispose the timeline chart when leaving its section #1357 plus updateTimeline = undefined) and keeps disposeLadderTimeline and the hidden-section early return. ed4b1e6 and 4710317 are clean. 56a2219 resolves the history.ts import with main's NO_VALUE kept and ladder-timeline.test.ts with main's fix(frontend): ladder planned-purchases block has no layout styling and locale date inputs #753 describe block kept. Follow-ups 8b68eec (queue Cancel e2e) and c85b919 (Log out renames) leave no "Logout" in tests or markup. I walked the toast pause/resume state machine again, and toast.ts is unchanged on main. refreshLadderTimeline uses load()'s generation guard. findHistoryRow still covers feat(frontend): add Load more to History and queue from all pages #1358's paged lastPurchases/lastPendingForQueue. Jest toast, history-cancel/retry, ladder, ladder-timeline, settings-accounts, auth, mobile-nav, purchases-placeholders-181: 286/286.

[IAmJSD]

On the blocking re-review finding above: fixed in 150979e7, with main merged at a006d14d.

• Confirm text for empty rows: when hasNoCommitments(p), the Cancel and Retry confirm summary reads "No recommendations attached", the same label the row shows, and leaves out upfront, service and region.
• Placeholders: a "multiple" region is dropped the same way as a "multiple" provider, and the term only shows when isRealTerm accepts it.
• New tests in purchases-placeholders-181.test.ts use the real utils:
  • Cancel (Approval queue) and Retry (failed row) clicked on the EMPTY_PENDING shape. Both fail on the old code.
  • A real row's confirm, which pins "Upfront: $4,200" from the real formatter.
  • The "multiple" region case.
• Test rename: the history-cancel-button.test.ts block is now "Approval queue Cancel button". Its upfront check no longer relies on the mocked formatter's output.
• Checks at a006d14d: jest 4210 passed, Playwright 247 passed, and lint, typecheck, build and git-secrets all pass.

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): APPROVE a006d14d4d44d2cf35aaa84e9fcc9ec2b058c77e

Blocking:

• none
  Non-blocking:
• frontend/src/history-confirm-body.ts:24 (pinned by tests/purchases-placeholders-181.test.ts:231): on a multi-rec execution the API sets resource_type to "N commitment(s)" and count to N (internal/api/handler_history.go:386,560), so the confirm reads "AWS ec2 2 commitment(s) × 2". When the service collapses to "multiple" it reads "multiple 3 commitment(s) × 3". The words are accurate and the row shows the same values, but the text repeats itself. A fix: skip the "× N" suffix when resource_type already ends in "commitment(s)", and treat a "multiple" service like provider and region. The line 17 comment says "multiple" applies to provider or region only, yet the API also sends "multiple" for service.
• Main moved after this head: chore(frontend): replace mixed Unicode glyphs with one SVG icon set #1386 (chore(frontend): replace mixed Unicode glyphs with one SVG icon set #751, 049c617) landed and touches toast.ts, confirmDialog.ts, history.ts, ladder.ts, settings.ts and index.html. git merge-tree of HEAD with 049c617 is clean. On that merged tree (read-tree in a scratch copy, no commit), tsc passes, and jest passes 256/256 on icons (including the new Unicode glyph guard; "×" is not banned), toast, purchases-placeholders-181, history-cancel/retry, ladder, ladder-timeline, row-action-icons and settings-accounts. Merging main again is optional, not required.
  Attacked: The blocking finding is resolved in 150979e. When hasNoCommitments(p) is true, the summary uses NO_RECOMMENDATIONS_LABEL, and upfront, service and region are skipped. A "multiple" region is dropped and the term needs isRealTerm. This matches the row renderers on main (history.ts:654-665, 1395, 2023). I put back the pre-fix history-confirm-body.ts in a scratch copy, and 3 of the 4 new fix(frontend): impossible/placeholder values surfaced for scheduled and pending purchases #181/fix(frontend): low-severity copy, toast, confirm and stale-state fixes #728 tests failed (empty Cancel, empty Retry, multiple region). They pass at HEAD. "Upfront: $4,200" now comes from the real formatter. The summary is built with textContent and createElement only, so there is no XSS path, even for a provider that is not whitelisted.
  Merges: a006d14's tree is byte-identical to the automatic git merge-tree of its parents, so it is a clean merge with nothing hidden in it. I checked 56a2219 against its auto-merge: only the conflict hunks differ, and both keep main's NO_VALUE import and the fix(frontend): ladder planned-purchases block has no layout styling and locale date inputs #753 describe block. origin/main...HEAD covers 19 files. Jest at HEAD (placeholders-181, history-cancel/retry, toast, ladder, ladder-timeline, settings-accounts, auth, mobile-nav): 290/290. No "Logout", "Purchasing Settings" or user-visible "Settings → Purchasing" text is left in the markup or the tests.

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): APPROVE 20a9b804805b9c681f8bc9df4134831ae2c888af

Blocking:

• none
  Non-blocking:
• frontend/src/toast.ts:317-341: the toast has tabIndex=0, so clicking its body with the mouse focuses it, and the countdown then stays paused after the pointer leaves, until focus moves elsewhere. That is consistent with WCAG 2.2.1, but it means a clicked toast effectively becomes sticky. Worth a line in the body.
• frontend/src/history-confirm-body.ts:24: carried over from the last review and still present. A multi-rec execution reads "AWS ec2 2 commitment(s) × 2", and a "multiple" service is still printed ("multiple 3 commitment(s) × 3"). The text is accurate and matches the row, but it repeats itself.
• dashboard/history success toasts keep 5 s; only the override Undo moves to 10 s. This matches the stated scope.
  Attacked: The previous blocking item (fix(frontend): impossible/placeholder values surfaced for scheduled and pending purchases #181 placeholders in the Cancel/Retry confirm) is resolved by 150979e: hasNoCommitments uses NO_RECOMMENDATIONS_LABEL, upfront/service/region are skipped, a "multiple" region is dropped, and the term goes through isRealTerm. The summary is built with textContent/createElement only, so there is no XSS path. Read the full origin/main...HEAD diff (19 files; origin/main == merge^2). For merge 20a9b80, diffed against both parents. Against main, toast.ts carries only the branch's pause/resume block plus startTimer() after main's fix(frontend): ladder config asks for a raw account UUID and hides auto-approve impact #742 appendChild/layoutToastStack/startWatching; the duplicate appendChild is gone and main's layout and icon code are untouched. ladder.test.ts keeps main's confirmDialog import and adds the refreshLadderTimeline mock and assertions. mobile-header-drawer.spec.ts keeps main's 'navigation' role (fix(frontend): make sidebar plain links and move focus on navigation #1371) and only renames to "Log out".
  Ladder: refreshLadderTimeline goes through load()'s generation guard. dispose clears updateTimeline, and the hidden-section early return runs after disposeLadderTimeline, so no stale closure survives. No user-visible "Logout", "Purchasing Settings" or "Settings → Purchasing" is left; the remaining hits are only in code comments and test fixtures. Jest toast, history-cancel/retry, ladder, ladder-timeline, purchases-placeholders-181, settings-accounts, auth, mobile-nav: 382/382.

[IAmJSD]

Local verification at 20a9b804 merged onto current main (c582cfd4; merge-tree ee0a6278; Linux, Node, headless Chromium):

• This branch's own checks passed at this head: lint, typecheck, build, the full jest suite, the full Playwright e2e suite, and the git-secrets checks (see Verification in the PR body).
• The combined tree of every UI PR in this merge wave (tree ee0a6278, main plus 3 PRs) passes npm ci, lint, typecheck, build, jest (1 skipped, 4400 passed, 4401 total), and the full Playwright suite (264 passed).
• CI is green on 20a9b804 (CI Success, Run pre-commit hooks).
• The independent review above approves this exact head.
• Nothing was applied to any cloud.

[IAmJSD]

Marked ready and merged by IAmJSD's agent, at IAmJSD's request (IAmJSD reviewed the before/after screenshots; cristim is traveling), after the independent review and local verification above, all at 20a9b804.