Repository navigation
Highlight any block that is showing work in progress - #260
Merged
juanmaguitar merged 1 commit intoAug 11, 2026
Merged
juanmaguitar merged 1 commit into
juanmaguitar merged 1 commit into
Conversation
juanmaguitar
force-pushed
the
juanmaguitar/highlight-patch-progress
branch
from
August 11, 2026 09:11
88ba147 to
d51da32
Compare
The next-action cue points at the block a contributor should act on; a block that is *doing* work — a trunk update, a patch apply or revert — is where attention belongs too, and it now scrolls into view and glows like any other cue target. The trunk-update tracker was already highlighted; this completes the idea by adding the patch apply/revert tracker, so every in-progress block gets the same treatment. Add an `isApplying` rule to the resolver (id `applying-patch`; one flag covers both apply and revert, since a revert runs the same machinery), placed above the stale-state warnings — a live operation outranks a warning. Anchor the tracker block, which renders only while the operation runs, so the cue clears when it ends. Reorder the ladder so the two tree-owning operations (applying, updating) are checked before the updateIncomplete warning; behaviour is unchanged for the existing cases. Part of #252. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
juanmaguitar
force-pushed
the
juanmaguitar/highlight-patch-progress
branch
from
August 11, 2026 10:47
d51da32 to
44842f9
Compare
juanmaguitar
added a commit
that referenced
this pull request
Aug 11, 2026
## Why
When a contributor pastes a PR or picks a `.diff`/`.patch` file, the app
shows a **staged preview** — "<label> changes N files" with **Apply and
rebuild** / **Cancel**. That block is a clear next action: they set it
up and the decision to apply is theirs. But it was not a cue target, so
nothing pointed at it. This highlights it like the other next-action
blocks.
## What changes
- `next-action.cjs`: a new rule — when `applyPreview` is staged, point
at the `apply-preview` block. Placed **after** the in-flight operations
(`isApplying`, `isUpdating`) and **before** the stale-state warnings and
routine steps: an explicit, waiting choice beats a standing suggestion,
but a live operation still wins. The ordering matters — the preview
block renders only under `applyPreview && !isApplying`, so once applying
starts the cue must move to the progress tracker; checking `isApplying`
first guarantees the id follows the visible block.
- `index.jsx`: passes `applyPreview` to the resolver and spreads
`cueProps('apply-preview')` onto the preview block.
## How to test this
**Platforms:** any (renderer-only). Stacked on #254 + #256 + #258 + #260
— test this branch's Buildkite artifact to exercise the whole stack.
**Starting state:** an initialized site, wizard skipped, *Apply a patch
or PR* panel visible.
1. Paste a PR URL (or number) and click **Apply PR** — or choose a
`.diff`/`.patch` file. → The **preview block** (with *Apply and rebuild*
/ *Cancel*) glows and the view scrolls to center it.
2. Click **Apply and rebuild**. → The preview unmounts and the glow
follows to the **progress tracker** (from #260).
3. Instead, click **Cancel**. → The preview clears and the cue moves on
to the next pending action.
**What must not have happened:**
- The glow must not stay on the preview once applying begins (the
preview block is gone by then).
- No glow on the preview when there is nothing staged.
- Screen reader announces "Next step: A patch is staged — apply and
rebuild, or cancel."
## Risks and limitations
- Renderer-only. Not hand-verified by the author (no local app run — the
EBADENGINE install constraint); `eslint .` clean, `npm test` **799** (3
new). Buildkite artifact is the hand-test path.
## Related
Part of #252. Stacked on #260 (base branch
`juanmaguitar/highlight-patch-progress`) → #258 → #256 → #254. Merge
bottom-up.
---
<details>
<summary>Review outcome (required — see AGENTS.md)</summary>
**No findings across the five dimensions** (0 [fix here] · 0
[follow-up]). Judgement pass ran in a fresh `Explore` subagent given
only the diff and the review standard, per `/self-review`.
Verified: the resolver checks `isApplying` before `apply-preview`, so
when applying starts the preview block unmounts and the cue follows to
the progress tracker in the same render (locked by a new test);
`Boolean(applyPreview)` is the right null-vs-object coercion, and the id
is returned only when the preview block is actually rendered (no
id-without-block window); `cueProps` in scope; full suite green. The
reviewer also confirmed a staged preview + in-flight trunk update can
coexist and `updating` correctly wins — placing `apply-preview` below
`isUpdating` is intentional, and the cue still lands on a rendered
block.
Deterministic layer: `eslint .` clean; `npm test` **799 pass** (3 new).
</details>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
Why
The next-action cue (#252) points at the block a contributor should act on next. A block that is doing work — a trunk update, a patch apply or revert — is where attention belongs too: it should scroll into view and glow like any other cue target, so a contributor who looked away can see where the work is and that it isn't stuck. The trunk-update tracker was already highlighted; this completes the idea so any block displaying work in progress gets the same treatment.
What changes
next-action.cjs: a new high-priority rule — whenisApplyingis true, point at theapplying-patchblock (the patch apply/revert step tracker). It sits above the stale-state warnings: a live operation outranks a warning. One flag covers both operations — a revert runs through the same apply machinery (runApply({ reverse: true })), so there is no separate "reverting" id.isApplying,isUpdating) are checked beforeupdateIncomplete, and dropped the now-redundant!isUpdatingguard. Behaviour-preserving for the pre-existing cases.index.jsx: passesisApplyingto the resolver and spreadscueProps('applying-patch')onto the tracker (the block that only renders while the operation runs).After this, every in-progress block in the site view is a cue target: the trunk-update tracker (wired earlier in the stack) and the patch apply/revert tracker (here). Each glows and — with the assertive scroll from the base PR — is centered in view when it starts, and the cue clears when it ends.
How to test this
Platforms: any (renderer-only). Stacked on #254 + #256 + #258 — test this branch's Buildkite artifact to exercise the whole stack.
Starting state: an initialized site, wizard skipped, with the Apply a patch or PR panel visible.
.diff/.patchfile (paste a PR URL → Apply PR → Apply and rebuild). → While it runs, the step tracker block glows (soft amber) and the view scrolls to center it.What must not have happened:
isApplyingis false).Risks and limitations
isApplyingandisUpdatingown the working tree and never run at once (each disables the other's controls), so the ordering between them is not observable in practice;isApplyingis placed first regardless.eslint .clean,npm test796 (2 new). Buildkite artifact is the hand-test path.Related
Part of #252. Stacked on #258 (base branch
juanmaguitar/setup-step-ready-label) → #256 → #254. Merge bottom-up.Review outcome (required — see AGENTS.md)
No findings across the five dimensions (0 [fix here] · 0 [follow-up]). Judgement pass ran in a fresh
Exploresubagent given only the diff and the review standard, per/self-review.Verified:
isApplyingis true for both apply and revert (runApplysetsapplyStateunconditionally, revert callsrunApply({reverse:true})); the ladder reorder is behaviour-preserving across allupdateIncomplete/isUpdatingcombinations, and dropping the!isUpdatingguard is safe now thatisUpdatingis checked first; the two tree-owning operations are mutually exclusive (both bail on the sharedrunningflag), soisApplyingwinning is moot in practice and reasonable otherwise; and theapplying-patchid and its DOM block appear/disappear together (cuePropsin scope).One change applied from the review (a style note, not a finding): removed a second test that fed the resolver the same input as the first — the apply/revert distinction lives in
runApply, not the resolver, so a resolver test can't prove it. Kept one test documenting the shared-flag behaviour.Deterministic layer:
eslint .clean;npm test796 pass (2 new).Note: reviewed as the apply/revert-specific change; the reframing to "any block showing work in progress" is a title/description change only — no code changed since review.