feat(webapp): read a file with the whole browser window (BEA-195) - #215
Merged
Merged
Conversation
A file page renders inside the app shell — a 264px sidebar, a topbar, and a
768/1200px column — so a wide CSV, a big mermaid diagram, a rendered HTML
file or a PDF is read through a letterbox. `?full=1` on the file route hides
the chrome and gives the content the window; Exit, Esc and browser Back all
leave, and the URL is the state, so a teammate you paste it to lands
fullscreen on the same file.
Fullscreen rides as a query param beside `?v=` rather than as a view route,
for the same reason a version does: the first segment after the project id is
reserved for view names, and a fullscreen file is the same page with
different chrome. The two compose — `?v=<sha>&full=1` reads a past version
fullscreen, banner and all.
Two things the code forced:
* `routeKey` is `withoutFull(useLocationPath())`. That key is
pathname+search, so a `?full=1` push otherwise looks like a brand-new
route and arms a scroll goal of 0 — the reader is thrown to the top of
the document the moment they ask for more of it.
* The Exit control is permanently visible and painted over the content,
not a hover reveal. The HTML renderer is a sandboxed iframe on an opaque
origin and the PDF renderer is the browser's own viewer; once focus is
inside either, `keydown` never reaches us and Esc silently does nothing.
The chrome is hidden with a body class, never unmounted. `<article
id="content">` therefore survives the toggle and keeps its `scrollTop` in
both directions for free, and `display: none` takes the hidden controls out
of the tab order and the accessibility tree together — the same lesson
`syncSidebarInert` already encodes for the off-canvas sidebar.
Markdown keeps the `--page-read` measure: a 2000px line of prose is
unreadable, and what prose gains here is the removed chrome, not a wider
column. Every other renderer gets `.page.bleed` and the HTML/PDF frames take
the viewport height.
Frontend only — no new API, no config, no telemetry, and `full` never
reaches a query key, so a toggle refetches nothing and inflates no read
count.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fullscreen (BEA-195) and the print view (BEA-219) landed hours apart and overlap: #exit-full is fixed over the content, so printing a file opened fullscreen stamped the Exit button onto the page, above a blank strip reserved for it. Added to the print block's hidden chrome, with the reserved padding dropped alongside it. Co-Authored-By: Claude Opus 5 (1M context) <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.
TL;DR
?full=1. Copy it, send it, and a teammate lands fullscreen on the same file — which is what the issue actually asked for.Before / after
A 10-column CSV at 1440px. Today it's clipped at the reading column; with
?full=1every column fits.?full=1Markdown deliberately keeps its 768px measure — a 2000px line of prose is unreadable, so what prose gains here is the removed chrome, not a wider column:
?full=1Why a query param
The first path segment after the project id is reserved for view names, and a file path already occupies that slot.
?v=<sha>is the existing precedent and the reasoning is identical: a fullscreen file is the same page with different chrome, exactly as a pinned version is the same page with older bytes. The two compose —?v=<sha>&full=1reads a past version fullscreen, banner and all.The two traps, both real
1. Entering fullscreen threw the reader to the top of the document.
routeKeyisuseLocationPath()— pathname plus search — so a?full=1push looked like a fresh route and armed a scroll goal of 0. You ask for more of a document and lose your place in it. Fixed by keying the scroll memo onwithoutFull(...), so entering and leaving land in the same slot.2. Esc can never reach the app from inside an HTML or PDF file. Those render in iframes — HTML under
sandbox="allow-scripts"(an opaque origin), PDF as the browser's own viewer. Key events don't cross that boundary. So the Exit control is rendered byAppShelloutside the topbar it hides, painted over the content, always visible and never a hover reveal. It is the only exit that works from inside one.The chrome is hidden by a body class, never unmounted:
<article id="content">survives the toggle and keeps itsscrollTopfor free, anddisplay: nonetakes the hidden controls out of the tab order and the accessibility tree together — the same lessonsyncSidebarInertalready encodes for the off-canvas sidebar.The bug the earlier work could not have known about
Fullscreen and the print view (BEA-219, merged a few hours ago) overlap:
#exit-fullisposition: fixedover the content, so printing a file opened fullscreen stamped the Exit button onto the page, above a blank strip reserved for it. It's now in the print block's hidden-chrome list with its padding dropped, and there's a regression test — verified to fail when the rule is removed.Testing
e2e/fullscreen.spec.ts— 11 specs, one per acceptance criterion: chrome hidden, pasted link with no flash,?v=composition, Esc/Back/Exit, scroll preserved both directions, no refetch on toggle, Exit over the HTML and PDF iframes, tab order and focus, markdown measure vs HTML frame height, below the 900px breakpoint, and folders/view routes ignoring?full=1.router.test.ts—fullparsed, round-tripped, surviving alongside?v=, andwithoutFullidempotent. 138 unit tests pass.main(an org-rename test cascading) — I baselined that with a full run earlier today.go test ./...green.check-dist.shreports the committed bundle fresh.Architecture changes
Browsergains the fullscreen capability androutergainsRoute.full,withoutFull, and a fourthurlForPathparameter.architecture/webapp-frontend.mdis updated on the branch; excerpt of the change:flowchart TB Browser["<div style='text-align:left'><b>Browser</b><br/>folder listing, file view<br/>per-view routes<br/>+moved: /resolve?path= on a tree miss only<br/>+scroll restoration: contentRef, memo, goal from lib/scroll<br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ +fullscreen: body.full-view from route.full, Exit / Esc / Back</span></div>"] router["<div style='text-align:left'><b>router</b><br/>+parseRoute(url, mode) Route<br/>+Route.version ?v= sha, one past version<br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ +Route.full ?full=1, the file page with the chrome hidden</span><br/>+Route.trailingSlash / Route.queryTarget / Route.filters<br/>+historyFilterQuery(filters) / hasHistoryFilters<br/><span style='background:#ef444455;padding:0 4px;border-radius:3px'>❌ <s>+urlForPath(path, projectId, version)</s></span><br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ +urlForPath(path, projectId, version, full)</span><br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ +withoutFull(url) the same URL minus full — the scroll memo key</span><br/>+urlForView(...) / encodePath / decodePath<br/>+projectByName(projects, seg)</div>"] Why["✅ withoutFull exists for the scroll memo.<br/>routeKey is pathname+search, so a ?full=1 push<br/>looked like a fresh route and armed a goal of 0 —<br/>the reader was thrown to the top of the document<br/>the moment they asked for more of it."] Browser --> router Browser -.- Why classDef added fill:#22c55e22,stroke:#22c55e,stroke-width:2px classDef noteBox fill:#88888822,stroke:#888888,stroke-dasharray:2 2 class Why addedCloses BEA-195.
🤖 Generated with Claude Code