fix(spec-builder): keep the phase controls reachable at phone widths - #3859
Conversation
github-actions
Bot
added
the
readiness: checking
Design Review (Fable 5) — ✅ PASSAdvisory design-level review of Design-Verdict: PASS Restores the only route to phase actions and blocking decisions at phone widths; alternatives were measured and rejected with evidence, and the fix stays minimal. [DESIGN-REVIEWED] 928eed0 |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
github-actions
Bot
added
readiness: action required
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsI've verified the candidate. Let me confirm the button-count analysis by checking how The advance button renders only when No grounded behavioral defect exists in this layout-only diff. No findings. [OPUS-REVIEWED] 928eed0 Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
First Principles Review (Fable 5) — 🟡 CONCERNSAdvisory premise-level review of Reading the contract, the intent file, the patch, and the surrounding source is done; I verified the load-bearing claims directly in the repo before judging. Verification notes (evidence for the review below):
First-Principles-Verdict: CONCERNS Every item is a measured fix except Round 3's What this change shipsIntent: restore the ability to advance, build, pause, and unblock a spec on a phone, which #3844 removed. This is a FIX.
Watch
Subtractions
[FIRST-PRINCIPLES-REVIEWED] 928eed0 |
UX Review (Fable 5) — 🟡 CONCERNSAdvisory UX-level review of UX-Verdict: CONCERNS Phase controls are reachable again, but the newly exposed doc tabs switch nothing visible on a phone — a control that reads as dead. Watch
Suggestions
[UX-REVIEWED] 928eed0 |
54410f8 to
77a63b4
Compare
github-actions
Bot
added
readiness: checking
Hiding the document column at phone widths took more than the document with it. `docTabsHeader` is the SOLE host of the phase controls -- Approve → Design, Approve → Tasks, Start building, Pause -- and it lives inside the `sb-doc` element that was hidden. The fullscreen review overlay builds its own header and never calls `docTabsHeader`, and those actions are additionally gated on `!fullscreen`, so there was no second route: at 767px and below a spec could not be advanced, could not start building, and could not be paused. `SpecStatePanel` was hidden the same way. It is the only surface that shows a BLOCKING decision and the only one that can answer it, and the overlay does not render it either -- so a blocked spec was indistinguishable from an idle one. Now only the document BODY steps aside. The header stays and keeps the phase controls, the state panel is visible at every width, and the column itself always renders while narrow rather than being gated on the comment count -- that gate took the phase controls away whenever a spec had no pending comments, which is the normal case. Exposing the header was not sufficient on its own. Measured in the real build at 390px, the header row is 414px wide and the pane's `overflow-hidden` clipped `Approve → Tasks` at left 321 / right 414 with no way to scroll to it, so the control was visible in the tree and still unreachable. The header wraps while narrow, which puts that action on its own line: left 10 / right 155, clipped by nothing, and all five header controls reachable. The stacked column carries a `60vh` cap with internal scrolling, so an accumulating state panel plus a staged comment cannot push the comment tray past the page shell. The bound is in `vh` rather than a percentage because no ancestor in this chain has a definite height, which makes a percentage max-height inert -- measured resolving to 506.4px at an 844px viewport. The body is hidden rather than unmounted because DocView holds an in-progress comment draft.
77a63b4 to
928eed0
Compare
github-actions
Bot
added
readiness: checking
iamwhatever
left a comment
iamwhatever
left a comment
There was a problem hiding this comment.
Tier 1 auto-approve: cosmetic-UI (3 files). Criteria: no conflict, no requested changes, no security surface, AI reviewers green. Category: responsive layout fix — keeps spec-builder phase controls reachable at phone widths.
github-actions
Bot
removed
the
readiness: passed
…irodotdev#3859) Hiding the document column at phone widths took more than the document with it. `docTabsHeader` is the SOLE host of the phase controls -- Approve → Design, Approve → Tasks, Start building, Pause -- and it lives inside the `sb-doc` element that was hidden. The fullscreen review overlay builds its own header and never calls `docTabsHeader`, and those actions are additionally gated on `!fullscreen`, so there was no second route: at 767px and below a spec could not be advanced, could not start building, and could not be paused. `SpecStatePanel` was hidden the same way. It is the only surface that shows a BLOCKING decision and the only one that can answer it, and the overlay does not render it either -- so a blocked spec was indistinguishable from an idle one. Now only the document BODY steps aside. The header stays and keeps the phase controls, the state panel is visible at every width, and the column itself always renders while narrow rather than being gated on the comment count -- that gate took the phase controls away whenever a spec had no pending comments, which is the normal case. Exposing the header was not sufficient on its own. Measured in the real build at 390px, the header row is 414px wide and the pane's `overflow-hidden` clipped `Approve → Tasks` at left 321 / right 414 with no way to scroll to it, so the control was visible in the tree and still unreachable. The header wraps while narrow, which puts that action on its own line: left 10 / right 155, clipped by nothing, and all five header controls reachable. The stacked column carries a `60vh` cap with internal scrolling, so an accumulating state panel plus a staged comment cannot push the comment tray past the page shell. The bound is in `vh` rather than a percentage because no ancestor in this chain has a definite height, which makes a percentage max-height inert -- measured resolving to 506.4px at an 844px viewport. The body is hidden rather than unmounted because DocView holds an in-progress comment draft.
Open PR relationship auditThis is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion. Relationship findings
No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit. |
3 participants
Follow-up to #3844, which merged before this landed. That PR hid the document column
at phone widths — and took two things with it that have no other host.
Capability removed, not just layout
The phase controls.
docTabsHeader(SpecDetail.tsx:277) is the sole host ofApprove → Design, Approve → Tasks, Start building and Pause. Its only call site is
inside the
sb-docelement that #3844 markedisMobile ? 'hidden'. The fullscreenreview overlay builds its own header and never calls
docTabsHeader, and thoseactions are additionally gated on
!fullscreen. So there was no second route: at767px and below a spec could not be advanced, could not start building, and could
not be paused at all.
The decision panel.
SpecStatePanelwas hidden the same way. It is the onlysurface that shows a BLOCKING decision and the only one that can answer it, and the
overlay does not render it either — so a blocked spec looked identical to an idle
one, with no way to find out why it had stopped.
That is a different class from the width defects this series has been fixing. A
squeezed column is hard to read; this removed the ability to act.
The fix
Only the document body steps aside now:
SpecStatePanelis visible at every widthcomments.length > 0— that gate removed the phase controls whenever a spec hadno pending comments, which is the normal case
DocViewholds an in-progresscomment draft
Tests
The existing narrow-viewport suite gains assertions for each half, and one of my own
was too weak until a mutation showed it:
SpecStatePanelagainThe third assertion checked only that the full-width class string was present — and
it survives inside a nested ternary, so the mutation passed. It now also asserts the
absence of any comment-count gate on the column, and the mutation is caught. Worth
recording because a test that cannot fail is worse than no test.
tscclean; 17 Spec Builder suites, 142 tests.Credit and scope
Design and Opus both pointed at the phase controls; UX, Design and Opus all pointed
at the state panel. Two related findings I am not folding in:
truncate/min-w-0. Untouched by these lines, but fix(spec-builder): make the workspace usable at phone widths #3844 made the overlay the only documentroute on a phone, so it deserves its own change.
comments — those live in
SpecDetailstate, which the overlay does not unmount.Only
DocView's uncommitted draft is lost, which is equally true on a desktop andis pre-existing. Its suggested remedy (drop the overlay route) would remove the
only document entry point on a phone.
Round 2 — GPT was right: exposing the header was not enough
Measured in the real build at 390px (isolated pod, seeded spec, DSF=2):
sb-doc … overflow-hiddenThe row measures 414px against a 390px viewport, and
overflow-hiddengives noway to scroll to the overhang — so the control was present in the tree and still
unreachable. My first version of this PR would have shipped inert for its own
stated purpose. The header now wraps while narrow, which drops that action onto
its own line.
The frame is 780×1688 device px = 390×844 CSS at DSF=2.
The BLOCKING finding, with a correction to its suggested fix
The unbounded-section concern is real and now bounded — but
max-h-fullwouldhave been inert. No ancestor in this chain has a definite height, so a
percentage max-height does not resolve; I measured this on a sibling PR where
10%left a 289px list unchanged while10vhcut it to 84px. The cap istherefore
max-h-[60vh] overflow-y-auto, measured resolving to 506.4px at an844px viewport, with
overflow-y: autoconfirmed on the live element.Tests
Two more assertions, both falsified by mutation:
vhbound back to a percentageRound 3 — took the
shrink-0finding, though its premise does not holdThe mechanism is real but not reachable on a phone. Measured in isolation, a column
whose chat is
flex-1 min-h-0(basis 0, so it absorbs only the leftover) against asection capped at
60vh:shrink-0)The cap binds before shrinking is ever needed, so on every reachable geometry the two
forms measure the same and the reported clipping does not occur. The edge that does
bite needs a shell shorter than the cap:
vhis viewport-relative while this row sitsin the viewport minus the app header, so
0.6v > v − 90— a viewport under ~225pxtall, which no device has.
Taking it anyway: it is free where it is reachable, strictly safer at that edge, and
cheaper than arguing. Re-verified in the real build afterwards —
Approve → Tasksstill lands at 10–155 on its own line, clipped by nothing, all five header controls
reachable, no page overflow.