fix(print): window.print() produces a usable page and the Print buttons say what they do (#4462) - #4488
Merged
Merged
Conversation
…ns say what they do (#4462)
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
ACCEPT — step-7 复核 by PM session
Flipping ready + arming auto-merge. #4464 (ListView surface) unlocks when this lands. Generated by Claude Code Generated by Claude Code |
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.
Closes #4462
The defect
Three surfaces expose a Print control backed by a bare
window.print()—plugin-list/src/ListView.tsx,plugin-report/src/ReportViewer.tsxandapp-shell/src/views/DashboardView.tsx. With no print stylesheet the browser printed the whole console (sidebar, top bar, chat rail, toasts) with the data table clipped to a single viewport, because the shell is a viewport-height flex chain:h-svhon the SidebarProvider, thenflex-1 min-h-0 overflow-hiddenall the way down to the grid's scroller. Nothing said what the control was, so it was being accepted at the reporting project against an "export to PDF" requirement.The ruling this implements
Per the ruling on #4462 (option 1 + option 2 as a rider; no removal, no headless detection). A real print/PDF primitive stays out of scope —
objectstack-ai/objectstack#1301, closed NOT_PLANNED — and is not re-litigated here.1. A shared print stylesheet at the app-shell level.
packages/app-shell/src/styles.css— the centralized styles entry host apps import (apps/console/src/index.cssdoes exactly that) — gains one@media printblock serving list, report and dashboard alike. It hides the shell chrome, prints the active content area full-width, releases the viewport-height chain so long tables paginate instead of clipping, repeats table header rows on every sheet, keeps a row from splitting across a page break, and neutralises dark mode (which otherwise prints white-on-white, since browsers drop background fills but keep foreground colours).The un-clipping rules match on Tailwind class substrings (
main [class*='overflow-']and friends). That is not a heuristic: AGENTS.md section 2 bans inlinestyle={{}}and CSS modules, so everyoverflow-*/h-full/max-h-*constraint in the render tree is a utility class. This is why plugin-list needs no print rules of its own — its root, its view container and the grid's scroller are all reached from the shared sheet (measured below).2. The Print controls say what they do. New
common.printDialogHint— "Opens your browser's print dialog (not a PDF export)" — in all ten locale packs, on thetitleand the accessible name of the list and report Print buttons, following the density button's existing two-attribute shape. The list reads it throughLIST_DEFAULT_TRANSLATIONS(mirror row added, byte-identical to theenpack, per the #4409 map-mirror gate); the report reads it throughuseSafeTranslate, which is that package's existing i18n channel (already used byDatasetReportRenderer.tsx) and needs no defaults map.3. No removal, no headless detection. Every call site still calls
window.print(), pinned by a test on each of the three.One deviation from the ruling's premise, reported rather than papered over
The ruling names
DashboardView.tsx:101as the third Print button. It is not one. That line is theexport_dashboard_pdfscript action handler, and its toast readPreparing PDF export…— no PDF was ever produced. There is no tooltip target there, so the honest equivalent of the ruling's item 2 on that surface is the copy itself:dashboardActions.pdfPreparingis replaced bydashboardActions.printDialogOpening, "Opening your browser's print dialog (not a PDF export)", in all ten packs. That string was the single most literal instance of the misreading the issue reports. The action ID staysexport_dashboard_pdf— it is the identifier server-driven dashboard metadata declares, so renaming it is a spec-side change, not a copy fix.Red-first
Reverse verification with the fix committed, then removed with
git checkout origin/main -- ...(nevergit stash) and restored:Ordinary red direction, as predicted in each file's header. The 5 that stay green are the deliberate "no removal" controls — the three
window.print()call-site pins plus the two facts this change does not touch (the console imports the styles entry; the package exports it). They were true before and must stay true.What CANNOT go red in vitest — stated in the test files, not only here
@media printnever applies under happy-dom or jsdom: neither implements a layout engine or media-type emulation, sowindow.matchMedia('print')is a stub andgetComputedStyle()returns the screen cascade whatever the sheet says. So no vitest assertion in this PR observes print rendering. What is pinned is structural — the sheet ships, the@media printblock exists, and each load-bearing rule is present in it, so deleting a rule reds. A rule that merely stops matching (becauseAppShellor the shadcn sidebar primitive changed its markup) cannot be caught from a stylesheet and would stay green; that failure mode is written into the stylesheet's own header.Real-browser verification (Playwright, print-media emulation)
Because the above cannot be tested in vitest, it was measured once for real. Method: the console's own Vite dev server compiled
apps/console/src/index.css(436 KB, the whole Tailwind v4 / Lightning CSS pipeline) — confirming every rule survives compilation, including the.group\/sidebar-wrapperescape. That compiled CSS was paired with a real rendered shell DOM (React-renderedAppShell+ shadcnSidebar+ListView, dumped from a throwaway render), loaded in Chromium, and measured in two arms: the compiled CSS as-is, and the same CSS with the@media printblock stripped out.Under
page.emulateMedia({ media: 'print' }):.group\/sidebar-wrapper > header)flex, 1280x56, visiblenone[data-sidebar="sidebar"])flex, 255x800, visiblenone[data-print-hide])flex, 407x34, visiblenonemainelement)overflow: hiddenoverflow: visibleoverflow: autooverflow: visiblebreak-insideautoavoidAnd the decisive one,
page.pdf({ format: 'A4' }):One clipped page becomes six paginated ones, with no console chrome and no toolbar buttons on any of them. Evidence files (screenshots, PDFs, JSON measurements) were kept out of the repo per AGENTS.md section 9 housekeeping.
Caveat, stated because the numbers deserve it: the harness renders the real
AppShell,SidebarandListView, but its 120-row table is a syntheticTablebuilt from the shadcn primitive's verbatim classes, since the registry-backed grid does not register under the light vitest setup. So the chrome-hiding measurements are of real product markup; the pagination measurement is of real product class shapes carrying synthetic rows.Scope
Touched: the app-shell styles entry, the three named call sites, the ten locale packs, three test files, one changeset. Not touched:
ObjectView.tsx, the app-surface/landing components,console/organizations/manage/**,plugin-calendar/**, the DOM-leak ledger,content/docs/releases/.Verification
pnpm exec vitest run packages/i18n/ packages/plugin-list/ packages/plugin-report/ packages/app-shell/— 444 files, 4864 passed, 1 skippedtype-check(bothtsc --noEmitandtsc -p tsconfig.test.json) on app-shell, plugin-list, plugin-report, i18n — cleaneslinton every touched file — 0 errorscheck:i18n-keys,check:i18n-drift,check:control-bytes,check-changeset-presence— all pass.d.tsdiff against a build oforigin/main: 124 lines, all of them the i18n pack literal types (the addedprintDialogHint, the renamedprintDialogOpening). No.d.tschange in plugin-list or plugin-report. The one other hunk isapp-shell/chrome/ErrorBoundary.d.ts, arender()return union whose 9 members are set-identical and only reordered — tsc declaration-emit nondeterminism, unrelated to this change. Patch grading throughout; no API surface moved.Generated by Claude Code