feat(desktop): the shell — a location that can hold a pane, and overlays that are not one - #31
Merged
Conversation
…ays that are not one Group 3 of plans/desktop-ui.md, as `specs/desktop-shell/` — requirements, design and tasks, all four artifacts validating. It is a spec rather than a checklist because the routing model genuinely changes. `Location` was one flat `view` that also carried `settings` and `history`, which made opening the settings a place you had gone: Back after closing it landed on the pane *before* the one you opened it from, the sheet having eaten the press that should have taken you there. - `Location` is now a pane and a selection within it; `Overlay` is a separate type that is deliberately not a location. `Shell` owns the three facts that only make sense together — where you are, what each pane was left showing, and which overlay is open — and the rules between them: an overlay is never recorded, nothing navigates while one is open, and a pane is re-entered at what it was last showing. - `Titlebar`, `Rail` and `StatusBar`, built from group 2's primitives. The recording indicator and its pause/stop sit in the titlebar because the moment you need them is the moment you are looking at something else. - Settings becomes a sheet, history a drawer, and what a citation opens an overlay — none of the three in the back stack. - The status bar's findings count is handed up from the checks pane's own load. `ow check` walks the whole project, and running it again to fill in a number in the frame is how a status bar becomes the slowest thing in the window. Until it has run once the bar says so rather than showing a confident zero: "no findings" and "not looked yet" are not the same sentence. Plan amendment: 8.4 and 8.5 become references to this spec rather than tasks of their own. The titlebar's recording controls and the status bar are this spec's R3 and R5 — they cannot be assembled empty and filled in later, and two records of one fact disagree. Verified: `pnpm test:coverage` (278 desktop tests, the 76% floor per package), `pnpm lint`, `pnpm run typecheck`, `build:renderer`, `scc validate` — clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HBt5LRVJd5Z7nagJGRwvSC
…n panel is modal Both review findings, and the first is a regression I introduced rather than a gap I left: task 2.4 replaced "the flat row of text buttons" and took the ←/→ buttons with it, so `Shell.back()` and `forward()` were implemented, tested, and reachable from nothing at all. The spec's own purpose says Back is half of how a wiki is read; a shell with no way to return had removed that quietly, while every screenshot still looked right. - Back and Forward are `IconButton`s at the left of the titlebar, disabled when there is nowhere to go. **Not in the draft** — the draft moves around the wiki with its tree — and here anyway, recorded as R3.6 rather than smuggled in. - The provenance viewer is not a modal, and `navigation.ts` and `design.md` both said every overlay was. It is deliberately beside the page: you open a citation to check a claim you are in the middle of reading, and a panel that makes that paragraph unreadable has answered the question by removing the thing that raised it. Recorded as R2.6, and the "R2.4 falls out of the modal for free" claim is corrected — it holds for two of the three by accident, which is why `Shell` asserts it. - `Findings` guards its in-flight load. Two overlapping `reloadKey` bumps could land a stale count in the status bar, which this change is what made reachable. - `Shell.select()` had no caller and is gone; so is the `.nav` rule the removed button row left behind. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HBt5LRVJd5Z7nagJGRwvSC
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
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.
Group 3 of
plans/desktop-ui.md, delivered asspecs/desktop-shell/— requirements, design and tasks, all validating.Why a spec and not a checklist
The routing model genuinely changes.
Locationwas one flatviewthat also carriedsettingsandhistory— which made opening the settings a place you had gone. Back after closing it landed on the pane before the one you opened it from, the sheet having eaten the press that should have taken you there.Locationis now a pane and a selection within it.Overlayis a separate type that is deliberately not a location.Shellowns the three facts that only make sense together — where you are, what each pane was left showing, which overlay is open — and the rules between them: an overlay is never recorded, nothing navigates while one is open, and a pane is re-entered at what it was last showing.The frame
Titlebar,RailandStatusBar, built from group 2's primitives. Settings becomes a sheet, history a drawer, and what a citation opens stays beside the page. The recording indicator and its pause/stop sit in the titlebar because the moment you need them is the moment you are looking at something else.The status bar's findings count is handed up from the checks pane's own load.
ow checkwalks the whole project, and running it again to fill a number in the frame is how a status bar becomes the slowest thing in the window. Until it has run once the bar says so rather than showing a confident zero — "no findings" and "not looked yet" are not the same sentence, and the second dressed as the first is the more dangerous one.Two places this departs from the draft, on purpose
specs/mcp-pane/), and a rail entry leading to a pane that can say nothing is worse than no entry.Plan amendment
8.4 and 8.5 stop being tasks and become references to this spec. The titlebar's recording controls and the status bar are this spec's R3 and R5 — they cannot be assembled empty and filled in later, and two records of one fact disagree. That is
routing.md's own rule for a plan item that references a spec.How it was verified
pnpm test:coverage(278 desktop tests, whole workspace, 76% floor, exit 0) ·pnpm lint·pnpm run typecheck·build:renderer·npx @protonspy/scc validate— no findings.code-reviewandsecurity-review, then fixes. Three findings, all closed:back()andforward()were implemented, tested, and reachable from nothing at all. Fixed, and recorded as a requirement so the affordance is not silently optional next time.navigation.tsanddesign.mdboth claimed every overlay was a modal, and the provenance viewer is not. The claim was the problem, not the panel: it is beside the page on purpose. Corrected in both, recorded as R2.6, and the "R2.4 falls out of the modal for free" reasoning replaced — it held for two of the three by accident.Findingshad no in-flight guard, and this change is what made a stale value reach the status bar as well as the screen.Left open
"Undo last write" opens the history drawer rather than undoing in one click. R5.4 says offer undoing, and the drawer's top row is the same operation the bar is reporting — so the bar points at it rather than performing an unrecoverable action from a strip of text. Worth a second opinion if you read R5.4 as one click.
🤖 Generated with Claude Code
https://claude.ai/code/session_01HBt5LRVJd5Z7nagJGRwvSC