Repository navigation
fix(frontend): name ladder accounts and guard Auto Approve saves - #1379
Conversation
The ladder config table listed each configuration by its internal account UUID, Auto Approve could be saved without any hint that later ladder purchases skip human approval, and a rejected save replaced the server's validation message with a generic failure. The table now loads the registered account list alongside the configs and shows "name (external ID)", keeping the UUID as the cell title and falling back to it when the list is unavailable. Selecting Auto Approve shows an inline warning, and a save that newly turns on unattended purchasing (enabled + Auto Approve) asks for confirmation first, the same pattern the RI exchange Fully Automated mode uses. Save failures now show the API's error text. The table renderer moves to ladder-config-table.ts to keep ladder.ts under 500 lines. Refs LeanerCloud#742 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ladder.ts sits at 494 lines, just under the 500-line rule, and the config modal's form layout needs reworking (review feedback: huge inputs and word-wrapped labels). Moving the static markup out first keeps that change reviewable and leaves ladder.ts room. No markup changes. Refs LeanerCloud#742 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review feedback: the Add/Edit Account Config form laid each label and input side by side, so long labels wrapped word by word, every input stretched to the wrapped label's height, input edges were ragged, the Ramp Schedule textarea was squeezed next to its help text, and on short viewports the title scrolled away with the form. Labels now sit above standard-height inputs, and the short fields pair up in an aligned two-column grid (one column under 640px). Range and unit qualifiers such as "Minimum 10" or "Leave blank for no cap" move to muted hints under the inputs, linked with aria-describedby. The ramp textarea spans the full width with its help below. The header stays fixed while the body scrolls, with Cancel / Save pinned to the bottom. Field IDs, validation and save behaviour are unchanged. Refs LeanerCloud#742 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflicts: - frontend/src/ladder.ts: import block kept both (this branch's ladder-config-table, ladder-config-modal and listAccountsMinimal imports plus main's readLadderNumbers from LeanerCloud#1326). renderConfigTable kept this branch's thin wrapper over renderLadderConfigTable; main's only change to the old inline table, the target_coverage guard (LeanerCloud#1326, Refs LeanerCloud#134: render N/A instead of throwing on a missing value), is re-applied in frontend/src/ladder-config-table.ts, where the table now lives. Main's readLadderNumbers validation in saveLadderConfig merged cleanly and runs before this branch's Auto Approve confirm and server-error toast. - frontend/tests-e2e/ladder-account-selector.spec.ts: kept both. This branch's two tests (account names / Auto Approve confirm / server error text; compact layout) followed by main's cleared Buffer Fraction test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…adder e2e test Main's cleared-Buffer-Fraction test (LeanerCloud#1326) clicks Save again while the validation toast is up. With this branch's compact modal the Save button is pinned to the bottom-right, under the toast stack at the default 1280x720 viewport, so the click was intercepted. Dismiss the toast first, as ladder-config.spec.ts already does between saves. Refs LeanerCloud#742 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflicts: - frontend/src/ladder.ts: import block only. This branch imports confirmDialog (Auto Approve confirm); main (LeanerCloud#731 via LeanerCloud#1372) imports openModal/closeModal to route the ladder config modal through the shared focus-trap helper. Kept both. Main's openModal(modal) / closeModal(modal) calls merged cleanly into this branch's openLadderConfigModal / closeLadderModal. The confirmDialog opened over the ladder modal is appended to body with its own capture-phase key handler, so its Escape does not reach the ladder modal's trap, and it restores focus to Save on settle. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
At 1280x720 the compact ladder config modal puts Save in its bottom-right corner, directly under the bottom-right toast stack. A validation or server-error toast stays up for 30 seconds and swallowed clicks on Save until it was dismissed. While a dialog box (.modal-content or .modal-confirm) is on screen, toast.ts now lays the stack out around it: beside the dialog, narrowed to the gap on its right, when that gap is at least 240px wide (268px at 1280x720 for the ladder modal), otherwise lifted above the dialog's button row (.modal-footer, .modal-buttons, .modal-confirm-actions). The positions are CSS custom properties set through the CSSOM, so no style attributes are added (CSP). A MutationObserver and a resize listener re-run the layout while toasts are showing, so a toast that was already up when a dialog opens moves too; nothing is watched while the stack is empty. This applies to every dialog, not only the ladder modal. The ladder e2e test no longer dismisses the validation toast before saving again; it asserts the toast does not cover Save. A new e2e test at 1280x720 and 390x844 shows a server-error toast and clicks Save again with it still up. Without this change those three e2e checks fail with the toast's box over Save's. Refs LeanerCloud#742 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
css-custom-properties.test.ts (LeanerCloud#745) requires every var() a stylesheet reads to be defined. Declare --toast-bottom and --toast-max-width on .toast-container with the old 1rem / 400px values and read them without fallbacks; toast.ts's inline override still wins while a dialog is open, and removing it restores the defaults. Refs LeanerCloud#742 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): REQUEST_CHANGES Blocking:
|
Bring in f2ab673 (bulk approval confirm totals, LeanerCloud#1362) before fixing the toast placement review finding. It merges cleanly; none of its files overlap this branch. Refs LeanerCloud#742 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…'s buttons The stack was lifted above an open dialog's button row whenever the gap beside the dialog was under 240px, without checking that the toast's corner touched that row. A short centred .modal-wide dialog at 1280x720, or a confirm stacked over the ladder modal, had the toast moved from empty space onto the dialog's content, including a destructive confirm's warning. layoutToastStack() now measures the stack at its default corner and moves it only when that box overlaps an on-screen button row of the topmost dialog (a confirm over a .modal; rows picked by visible rect, not DOM order). It then goes beside the dialog (width capped at 400px), above the whole dialog when there is room, or just above the covered row. Layout also re-runs on scroll inside a dialog, and the watchers are torn down once the last toast is gone. The CSS comment no longer claims more than that. Refs LeanerCloud#742 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Browser coverage for the toast placement fix: at 1280x720 an error toast raised over the short, centred account overrides dialog (.modal-wide) stays in its corner instead of moving onto the dialog, and at 1100x720 a toast that is up when the Auto Approve confirm opens over the ladder modal stays off the confirm and its warning text. Both fail on the previous layout code. Refs LeanerCloud#742 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
On the blocking re-review finding above: fixed in
|
No textual conflicts. frontend/src/ladder.ts and frontend/src/styles/components.css auto-merged: main's LeanerCloud#1387 moved the #ladder-purchase-timeline container out of the laddering fieldset and LeanerCloud#1378 swapped literal font sizes/weights in components.css for the --cudly-fs-*/--cudly-fw-* tokens; this branch's ladder table/modal split and toast stack variables sit in other hunks and are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Main's typography foundation (LeanerCloud#1378) forbids literal font weights that duplicate a --cudly-fw-* token; the ladder config form's labels and section divider still used 500 and 600. Refs LeanerCloud#742 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflicts: - frontend/src/ladder.ts: main's LeanerCloud#1386 (LeanerCloud#751 icon set) imported iconSvg and swapped the ladder modal's close × for iconSvg('x') in the inline modal markup, which this branch moved to ladder-config-modal.ts. Kept this branch's side (confirmDialog import, renderLadderConfigModal() call) and reapplied main's change in ladder-config-modal.ts: it now imports iconSvg and renders the close button as ${iconSvg('x')}. ladder.ts no longer needs the iconSvg import. frontend/src/toast.ts auto-merged: main's SVG kind/close icons (createIcon) sit alongside this branch's layoutToastStack placement. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): APPROVE Blocking:
|
|
Local verification at
|
|
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 |
The per-account ladder table listed each configuration by its internal account UUID, Auto Approve saved silently even though later ladder purchases then skip human approval, and a rejected save showed only "Failed to save ladder config. Check inputs and try again." with the server's reason discarded. The config modal was also hard to use: labels wrapped word by word, inputs stretched to the label height, and the title scrolled away on short screens. This PR names the accounts, warns and confirms on Auto Approve, surfaces the server error, and lays the form out as a compact two-column grid.
Closes #742
What changed
frontend/src/ladder-config-table.tsrenders the table (46c5153). The column is now "Account" and showsname (external ID)from the registered account list, keeps the UUID as the cell'stitletooltip, and falls back to the UUID when the list fails or the account is gone.frontend/src/ladder.ts(46c5153):listAccountsMinimal()in parallel; an account-list failure is treated as an empty list..warning-messageunder Approval Mode shows whenever Auto Approve is selected, including when editing an existing Auto Approve config.confirmDialog: title "Enable Auto Approve?", a red "Enable Auto Approve" button, and Cancel.Failed to save ladder config: <API error text>, with the generic text only when there is no message.renderConfigTablewrapper stays inladder.ts.frontend/src/ladder-config-modal.ts: the modal markup moved out ofladder.tswith no changes (24dea97), leavingladder.tsat 382 lines.frontend/src/styles/ladder-form.css(imported fromstyles/index.css):aria-describedby.frontend/src/toast.ts, e516dcc, 83568cb). This is an app-wide change, but it only matters when a toast would cover an open dialog's button row. Error toasts stay up for 30s, and at 1280x720 the default bottom-right corner sat on the ladder modal's Save, swallowing the retry click. The newlayoutToastStack()measures the stack at its default corner (right 16px, bottom 16px, its real width and height). It changes nothing unless that box overlaps an on-screen button row (.modal-footer,.modal-buttons,.modal-confirm-actions) of the topmost open dialog (.modal-contentor.modal-confirm; a confirm counts over a.modal). In that case the stack moves:.modal-widedialog or a confirm), keep the default corner.MutationObserveron class/hidden/child changes), on scroll inside a dialog (captured ondocument), and on resize, coalesced to one pass per frame. The watchers are removed when the last toast goes away.frontend/src/styles/components.css: the toast container reads--toast-bottomand--toast-max-width(defaults 1rem and 400px), whichtoast.tsoverrides through the CSSOM, so no inline style attributes are needed under the CSP.src/__tests__/ladder.test.ts,src/__tests__/toast.test.ts,tests-e2e/ladder-account-selector.spec.tsandtests-e2e/toast-dialog-placement.spec.ts.The free-text UUID field the issue describes was already replaced on main by a registered-account dropdown (#868, commit
970de3d0), so the modal's account input needed no further change.Why this approach
The Auto Approve safeguard follows the existing RI exchange "Fully Automated" pattern (inline warning banner plus a destructive confirm), so the two automation settings behave the same. The confirm fires only on the change into unattended mode, so editing an already auto-approved config or saving a disabled one does not nag; the inline warning always shows.
The table does its own account lookup instead of sharing the modal selector's fetch. The selector deliberately refetches on every open (tests guard against stale responses), so sharing would have meant reworking it; the cost is one extra
GET /api/accounts/liston page load.The error text comes from
apiRequest, which already copies the JSONerrorfield intoerr.message, so no API change is needed.Moving the modal markup to its own module in a separate, behavior-free commit keeps the layout change reviewable and keeps
ladder.tswell under the 500-line rule.The toast fix lives in
toast.tsrather than in the ladder modal because any dialog with its buttons in the bottom-right corner had the same problem. It moves the stack only when the stack would actually cover a dialog's buttons, so other screens see no change.Screenshots
Before is main (
a14dc8f2), after is this branch. Both are headless Chromium at 1440x900 against the repo's mocked e2e API, with mocked AWS accounts "Production" and "Staging". The last pair is at 1280x720.The screenshots were taken on an earlier head, before main moved to
f2ab6731and before the toast placement change. The changes since then are outside the screenshotted areas except for the "save-error" after shot: on the current head, a toast that would cover Cancel / Save moves beside the modal (or above it on narrow screens), so it no longer sits in the corner shown there.Verification
Run on this branch's head
903a2d5f, which includes mainf2ab6731:npm run lint: 0 errors (141 pre-existing warnings). Typecheck and build pass.toast.test.ts("toast stack and open dialogs", with stubbed geometry): the default corner with no dialog; beside the dialog when the corner would cover its buttons; never wider than 400px beside a wide gap; lifted above the button row when there is no room beside or above; lifted above a short bottom sheet rather than onto it; the default corner under a centred short dialog with room below it, and when the stack is too narrow to reach the buttons; only the topmost dialog counts, so a confirm over the ladder modal keeps the toast off the confirm (and a tall confirm sends it beside, never onto, the confirm); the visible button row is used, not the last one in the DOM; a button row scrolled into view under the stack moves it; open/close re-layout; unrendered dialogs ignored; the watchers stop once the last toast is gone.e2e-local.sh: 238 passed. New e2e coverage: the table name and title, the warning, confirm Cancel (no PUT) and the 400 error text end to end; at 1440x900, inputs under 44px tall, paired fields sharing a row with aligned left edges, the ramp textarea spanning both columns with its hint below; at 1280x720, the body scrolls while the title and Save stay on screen; at 390px, a single column. For the toast:.modal-wide) does not overlap the dialog.scripts/test-git-secrets-allowlist.sh: 137 PASS, 0 FAIL.Notes for review
openModal/closeModal, and the Auto ApproveconfirmDialogstacks over it with its own key handler.Overlaps
N/Aguard for a missingtarget_coveragenow sits inladder-config-table.ts.ladder.ts(callsrefreshLadderTimelineafter a kill-switch toggle or config save). Source merges cleanly; only appended tests inladder.test.tsconflict (keep both).×for an SVG icon inladder.ts; that markup now lives inladder-config-modal.ts, so the change has to be reapplied there when resolving the conflict.AmendLadderTrancheerror mapping) overlaps only on error text. fix(frontend): ladder planned-purchases block has no layout styling and locale date inputs #753 touches the planned-purchases block under the same section, inladder-timeline.tsand CSS, not these files.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