Stage warning actions; defer sort to explicit click
Add/remove/swap warning-action handlers no longer auto-fire /api/sort. They mutate the input textarea idempotently; the sort button gets a pending cue when current input != last-sorted input. Branch-picker (/api/resort, cheap) keeps instant behavior. Spec lives in docs/specs/2026-05-04-staged-warning-actions.md.
This commit is contained in:
137
docs/specs/2026-05-04-staged-warning-actions.md
Normal file
137
docs/specs/2026-05-04-staged-warning-actions.md
Normal file
@@ -0,0 +1,137 @@
|
||||
# Spec — Staged warning actions
|
||||
|
||||
**Date:** 2026-05-04
|
||||
**Status:** Draft (awaiting review)
|
||||
**Lineage:** Sits on top of Spec C §3 (Dep Add) and Spec A (multi-branch picker). Modifies the post-click behavior of warning action buttons; does not change the warnings themselves, the backend, or the picker contract.
|
||||
|
||||
## 1. Summary
|
||||
|
||||
Today, clicking `[add modId]`, `[✕ remove]`, or `[↔ swap]` in a warning row mutates the input textarea **and immediately fires a fresh `/api/sort`**. This spec defers the sort: those three handlers become pure textarea mutators, the user accumulates as many add/remove/swap edits as they want, and the existing `sort` button is the explicit "apply now" trigger. The branch-picker (cheap `/api/resort`) keeps its current instant behavior.
|
||||
|
||||
Visual feedback is twofold: each warning row whose action target is now satisfied by the current input gets a strikethrough + `✓ pending` button label; the main sort button gets a "pending" cue when `current input ≠ last-sorted input`.
|
||||
|
||||
## 2. Problem
|
||||
|
||||
The current "instant resort on click" loop punishes the user when they want to triage warnings in a batch:
|
||||
|
||||
- `/api/sort` is the **slow** endpoint — every wsid hits Steam's metadata lookup; uncached mods queue a DepotDownloader pull (~30s cold per mod). Three sequential clicks = three slow round-trips.
|
||||
- Users who reconsider mid-flight (add X, change mind, remove X) eat two full sort cycles for net-zero state change.
|
||||
- The chained reflows feel jumpy: each click rerenders the result panel and the warnings list, which can shuffle the row the user was about to click next.
|
||||
|
||||
Branch-picker doesn't have this problem — `/api/resort` uses the cached MOD_DB and returns in ~100ms — so it's left alone.
|
||||
|
||||
## 3. Behavior
|
||||
|
||||
### 3.1 Action handlers
|
||||
|
||||
`onAddWsid` / `onRemoveWsid` / `onSwapWsid` (sortof-app.jsx ~1348 / ~1363 / ~1381) drop their trailing `onSort(newInput)` call. They become pure mutators of `input`. The `setBranchSelections + runResort` handler `onPickBranch` is **untouched**.
|
||||
|
||||
These handlers also become **idempotent toggles**: if the action's effect is already present in the current input (e.g., `add-wsid` for a wsid already in input, or `remove-wsid` for a wsid already absent), the second click reverses the first. This is what powers the per-warning "click to undo" UX without a separate undo affordance.
|
||||
|
||||
| Action | First click | Second click |
|
||||
|---|---|---|
|
||||
| `add-wsid X` | append `X` to textarea | remove `X` from textarea |
|
||||
| `remove-wsid X` | remove `X` from textarea | append `X` back to textarea |
|
||||
| `swap-wsid A→B` | replace `A` with `B` | replace `B` with `A` |
|
||||
|
||||
### 3.2 Warning row "staged" derivation (stateless)
|
||||
|
||||
For each warning row with at least one mutating action, derive a boolean `staged` from current `input`:
|
||||
|
||||
- `add-wsid` action targeting `wsid W` → `staged = (W ∈ parseWorkshopInput(input))`
|
||||
- `remove-wsid` action targeting `wsid W` → `staged = (W ∉ parseWorkshopInput(input))`
|
||||
- `swap-wsid` action `from A, to B` → `staged = (A ∉ input ∧ B ∈ input)`
|
||||
|
||||
A warning may have multiple actions; if any one is satisfied, the row is staged.
|
||||
|
||||
When `staged === true`:
|
||||
- Row gets `.staged` class — strikethrough text + reduced opacity (~0.55).
|
||||
- Mutating action button label flips to `✓ pending` and `title` changes to `click to undo` (or per-type analogue).
|
||||
- Non-mutating action buttons on the row (`search-workshop`, branch-picker expand) are unchanged.
|
||||
|
||||
This derivation runs every render against `parseWorkshopInput(input)`. No new state, no syncing logic. Free benefit: if the user manually edits the textarea, all stage indicators update on the next render automatically.
|
||||
|
||||
### 3.3 Sort button "pending" cue
|
||||
|
||||
A new `lastSortedInputRef` (React `useRef`) tracks the input string at the moment the most recent successful `onSort` completed.
|
||||
|
||||
- On `onSort` success path (existing fetch handler): `lastSortedInputRef.current = inputThatWasSent`.
|
||||
- On `onSort` failure path: do **not** update — preserves the pending cue so the user knows the resort didn't land.
|
||||
- Initial value: `null`. The cue only appears once the user has done at least one sort and *then* drifted from it.
|
||||
|
||||
The main sort button reads `pending = (input !== lastSortedInputRef.current && lastSortedInputRef.current !== null)` and applies a visual modifier (CSS class `.sort-pending`) when true. Concrete treatment: a small filled dot suffix or pill (`var(--info)` or `var(--warning)`, exact pick during impl).
|
||||
|
||||
### 3.4 Multi-warning resolution
|
||||
|
||||
Because `staged` is derived purely from `input`, a single textarea mutation marks **every** warning whose action target matches. Example: three mods all missing dep `tsarslib`; clicking `[add 2392709985]` on warning A marks warnings A, B, and C as `✓ pending` simultaneously, since all three have `add-wsid` actions targeting the same wsid. No event-broadcasting code needed; it's a free property of the stateless derivation.
|
||||
|
||||
### 3.5 Branch-picker (unchanged)
|
||||
|
||||
`onPickBranch` continues to call `runResort(updated)` synchronously. Rationale: `/api/resort` is fast, doesn't hit Steam, and has no slow path; the current "instant" feel is correct for it. The pending cue/strikethrough machinery does not apply to branch warnings.
|
||||
|
||||
## 4. Components touched
|
||||
|
||||
All changes are in **`/opt/sortof/frontend/sortof-app.jsx`** (one file). No backend, no schema, no CSS file outside the inline `<style>` blocks already in `index.html` / co-located in the JSX.
|
||||
|
||||
| Symbol | Change |
|
||||
|---|---|
|
||||
| `onAddWsid` (~1348) | Drop `onSort(newInput)`. Add idempotent toggle: if `wsid` already in input, remove it instead of appending. |
|
||||
| `onRemoveWsid` (~1363) | Drop `onSort(newInput)`. Add idempotent toggle: if `wsid` already absent, re-append it. |
|
||||
| `onSwapWsid` (~1381) | Drop `onSort(newInput)`. Add idempotent toggle: if input is in `from→to` swapped state already, swap back to `to→from`. |
|
||||
| `onPickBranch` (~1408) | **No change.** |
|
||||
| `onSort` success branch | Set `lastSortedInputRef.current = <the input string just sorted>`. |
|
||||
| `lastSortedInputRef` | New `useRef(null)`. Lives next to existing `latestResortSeqRef` (~964). |
|
||||
| Warning render helper (~564–611) | Compute `staged` per warning. Apply `.staged` class to the row, swap mutating action button labels/titles when `staged`. |
|
||||
| Main sort button (~1809) | Append `.sort-pending` class when `input !== lastSortedInputRef.current` (and ref is non-null). |
|
||||
| CSS (`index.html`) | Add `.warn-row.staged { opacity: .55; text-decoration: line-through; }` (or equivalent on the existing warning-row selector — exact selector verified during impl). Add `.sort-pending` indicator rule (small dot or border accent using `var(--info)` / `var(--warning)`). |
|
||||
|
||||
`parseWorkshopInput` is already used elsewhere in the file (e.g., `onAddWsid` itself); no new helper needed.
|
||||
|
||||
## 5. Out of scope
|
||||
|
||||
- **Per-op undo affordance separate from the action button.** The action button's idempotent toggle behavior is the undo. No "discard all pending" button.
|
||||
- **Predictive cascade.** We don't simulate the sort to predict which warnings would be resolved by a staged change beyond a direct match against the action target. If adding mod X would *also* resolve a missing-dep on a different mod Y, that warning's row stays un-staged until the next real sort surfaces fresh warnings.
|
||||
- **Result-panel staleness overlay.** The right panel keeps showing the prior sort. The sort-button cue + strikethroughs are deemed sufficient signal. (Considered and rejected during brainstorming.)
|
||||
- **Pending-changes summary panel** (e.g., "+3 / −1 / 1 swap" badge). YAGNI; the strikethroughs already enumerate exactly what will change.
|
||||
- **Backend changes.** The `/api/sort` and `/api/resort` contracts, warning shapes, and action descriptors are all unchanged. Pure frontend.
|
||||
- **Branch-picker staging.** Out of scope per §3.5; reconsider only if `/api/resort` becomes slow.
|
||||
|
||||
## 6. Acceptance criteria
|
||||
|
||||
- [ ] Clicking `[add modId]` on a missing-dep warning appends the wsid to the textarea and does **not** fire `/api/sort` or `/api/resort`.
|
||||
- [ ] Clicking `[✕ remove]` on a build-mismatch warning removes the wsid from the textarea and does not fire any sort.
|
||||
- [ ] Clicking `[↔ swap A→B]` on a build-mismatch warning replaces A with B in the textarea and does not fire any sort.
|
||||
- [ ] Clicking the same `[add X]` button twice leaves the textarea in its pre-click state (idempotent toggle).
|
||||
- [ ] Clicking branch-picker still triggers `/api/resort` immediately (unchanged).
|
||||
- [ ] When a warning's action target is satisfied by the current input, the warning row renders with reduced opacity + line-through, and the action button label changes to `✓ pending`.
|
||||
- [ ] When two warnings share an action target (e.g., two missing-dep warnings both pointing at the same wsid), clicking the action on one stages **both**.
|
||||
- [ ] When `current input !== last-sorted input` (and at least one prior sort has happened), the main sort button shows a pending visual cue.
|
||||
- [ ] After a successful `/api/sort` completes, `lastSortedInputRef` is updated and the pending cue disappears.
|
||||
- [ ] After a *failed* `/api/sort`, the pending cue persists.
|
||||
- [ ] Manually editing the textarea (typing) updates the staged-warning indicators on the next render without explicit dispatch.
|
||||
- [ ] No backend file is modified; `app.py`, `worker.py`, `mlos_sort.py`, `adapters.py` untouched.
|
||||
|
||||
## 7. Test cases
|
||||
|
||||
1. **Single add, no sort.** Input has 3 mods producing one missing-dep warning for `tsarslib`. Click `[add 2392709985]`. Expect: textarea now shows 4 wsids; right panel still shows 3-mod result; warning row strikethrough; sort button has pending cue. No network request fired.
|
||||
2. **Two-warning shared-target stage.** Input produces two missing-dep warnings, both for `tsarslib`. Click `[add 2392709985]` on either. Expect: both warning rows strike through.
|
||||
3. **Idempotent add.** Click `[add X]`, then click it again. Expect: textarea returns to original; warning row un-stages; sort button pending cue clears (assuming this was the only edit).
|
||||
4. **Add-then-manual-delete.** Click `[add X]`. Manually delete `X` from textarea. Expect: warning row un-stages on next render; sort-button cue may stay (textarea still differs from `lastSortedInput` if user added/removed extra whitespace, otherwise clears).
|
||||
5. **Swap then sort.** Two B41↔B42 swap warnings on different wsids. Click swap on both. Expect: both rows stage. Click sort. Expect: `/api/sort` fires once with both swaps applied; on success, both rows now reflect fresh post-sort state (warnings either gone or different).
|
||||
6. **Branch-picker untouched.** Open the picker on a multi-branch wsid; tick a different branch. Expect: `/api/resort` fires immediately; result panel updates; no pending cue persists.
|
||||
7. **Failed sort preserves cue.** Stage one add. Force `/api/sort` to fail (e.g., simulate 500). Expect: result panel doesn't update, warning rows remain staged, sort-button cue remains.
|
||||
8. **Sort with no pending changes.** No prior edits, click sort. Expect: normal sort fires (existing behavior); cue does not appear before nor after.
|
||||
|
||||
## 8. Implementation notes
|
||||
|
||||
- The idempotent-toggle logic for `onAddWsid` reuses the existing `parseWorkshopInput(input).includes(wid)` check; today that path early-returns (silent dedupe), the spec replaces the early return with a remove-and-set.
|
||||
- The `staged` derivation can be a small helper inside the warnings renderer (`deriveStaged(actions, inputWsids)`) to avoid duplicating the per-action-type logic across the three button types. `inputWsids` should be computed once per render via `useMemo(() => new Set(parseWorkshopInput(input)), [input])` to avoid quadratic cost when there are many warnings.
|
||||
- The `.sort-pending` styling lives in `frontend/index.html`'s `<style>` block alongside the other `.icon-btn.report` / sort-button rules. Use existing tokens (`var(--info)`, `var(--warning)`); no new color literals.
|
||||
|
||||
## 9. Locked decisions (do not relitigate)
|
||||
|
||||
- **Stateless derivation** is the data model — no separate "pending ops" list. (§3.2)
|
||||
- **Branch picker stays live** — not staged. (§3.5)
|
||||
- **No result-panel overlay.** Sort button + strikethroughs are the only visual cues. (§5)
|
||||
- **Action button is the undo.** No separate undo button per warning. (§3.1)
|
||||
- **No predictive cascade** beyond direct action-target matches. (§5)
|
||||
Reference in New Issue
Block a user