SD-3294 - add layout-change event for responsive fit-to-container zoom#3548
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31fd208bf8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
cubic analysis
2 issues found across 4 files
Linked issue analysis
Linked issue: SD-3294: Add layout-change event for responsive fit-to-container zoom
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Emit a 'layout-change' event when the container width changes | The PR registers a watcher on the container width and calls emitLayoutChange, which emits the event when fitZoom changes. |
| ✅ | Payload includes containerWidth, documentWidth (base captured at 100% zoom), and fitZoom | The emitted payload contains containerWidth, documentWidth (baseDocumentWidth), and fitZoom; the types file adds SuperDocLayoutChangePayload with those fields. |
| ✅ | Capture document width once at initial load (before any zoom) to avoid feedback loops | baseDocumentWidth is initialized null and set only on the first emitLayoutChange call by measuring the document element, so the base width is captured once. |
| ✅ | Prevent feedback loops by rounding fitZoom and emitting only when it actually changes | fitZoom is computed via Math.round and compared against lastEmittedFitZoom; the event is returned early if unchanged. |
| ✅ | Register the new event in the SuperDoc event map/types so consumers can type-check handlers | The event map in SuperDoc.ts includes 'layout-change' and the payload type is added to the core types file. |
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
This comment was marked as outdated.
This comment was marked as outdated.
4e9e366 to
3025d68
Compare
…er zoom Adds a new `layout-change` event that fires when container dimensions change, enabling customers to implement responsive fit-to-container zoom without manual polling or ResizeObservers. Payload includes containerWidth, documentWidth, and fitZoom (calculated zoom to fit document in container). Base document width is captured once at 100% zoom to avoid feedback loops when setZoom is called. Closes SD-3294 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…idth Defer base width capture until isReady is true to avoid latching stale measurements before DOCX layout resolves (e.g., landscape or multi-section documents). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Verify that: - layout-change is not emitted before isReady - payload includes containerWidth, documentWidth, and fitZoom Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
3025d68 to
fd9f34b
Compare
Reshapes the layout-change contract from the base branch before it
ships, and models zoom as mode + value, the shape document viewers use.
- Rename layout-change to viewport-change: the public surface already
exports LayoutUpdatePayload for document layout passes, and what this
event reports is viewport fit. Payload { availableWidth,
documentWidth, fitZoom } carries pure measurements (sidebar-aware,
policy-free).
- Resolve the base document width from page styles, re-resolved per
evaluation, instead of a one-time DOM capture: the measured element
scales with zoom, so any zoom applied before capture corrupted
fitZoom permanently. Never emit before an editor exists.
- zoom config: initial (seeded before first paint, no flash), mode
(manual | fit-width), fitWidth bounds and padding. Padding and
clamping shape the applied fit only, never the metrics.
- setZoom() switches the mode to manual, so picking a percentage stops
the auto-fit instead of fighting it; setZoomMode('fit-width')
re-enters fitting and applies immediately. The fit application
writes zoom state directly and emits zoomChange with the mode.
- New reads: getZoomState(), getViewportMetrics() (latest metrics
readable any time, so late subscribers cannot miss the first
measurement). New constructor callbacks onZoomChange /
onViewportChange register before the first emit.
Adds onZoomChange and onViewportChange as explicitly plumbed callback props (the callbacksRef pattern), so swapped handler identities stay fresh across rerenders without rebuilding the SuperDoc instance. The zoom config flows through props automatically via SuperDocConfig. Event types re-derive from the core Config so the wrapper cannot drift from the core contract.
…SD-3294) Documents the zoom config (initial, mode, fitWidth), the new setZoomMode / getZoomState / getViewportMetrics methods, the viewport-change event and its pure-metrics payload, the zoomChange mode field, and the React responsive-zoom pattern.
…wport metrics (SD-3294) Two gaps a zoom UI hits in practice: - setZoomMode emitted nothing unless the numeric value later changed, so mode-only transitions (entering fit-width at the clamped value, returning to manual) were invisible to zoomChange subscribers. It now emits zoomChange with the current value on every mode change and no-ops on a same-mode call. - The width resolver required a DOCX activeEditor, so PDF-only instances never produced viewport metrics even though setZoom supports PDFs, and multi-document instances measured only the active editor. The resolver now takes the widest measurable page across all documents: DOCX from per-document page styles, PDF from rendered pages normalized by their actual scale factor back to CSS px at 100% zoom (a 612pt letter page renders 816 CSS px), with a pdf:document-ready re-evaluation hook. HTML documents reflow and contribute nothing; an HTML-only instance reports no metrics.
…D-3294) Custom UI gets first-class zoom: ui.zoom exposes one slice (mode, value, fitZoom, bounds, viewport metrics) recomputed on zoomChange and viewport-change, with set(percent) and setMode passthroughs to the host. Hosts without the zoom surface degrade to a static manual/100 snapshot with no-op mutations. React mirrors it with useSuperDocZoom (slice plus bound actions), and the toolbar registry gains a zoom-fit-width toggle command so custom toolbars can offer Fit width without reaching for the host instance. The numeric zoom command is untouched. The built-in toolbar's Fit width affordance stays a follow-up: the state and command layer it needs ships here.
…ent' into caio/sd-3294-fit-to-container-config # Conflicts: # packages/superdoc/src/core/SuperDoc.ts # packages/superdoc/src/public/index.ts # tests/consumer-typecheck/snapshots/superdoc-root-classification.json # tests/consumer-typecheck/snapshots/superdoc-root-exports.json # tests/consumer-typecheck/snapshots/superdoc-root-exports.md # tests/consumer-typecheck/src/all-public-types.ts
…ommand (SD-3294) Extracts the pdf measurement math into a pure helper (normalizePdfPageMeasurement) and locks it directly: scale-relative conversion back to CSS px at 100%, the zoom-fallback path, and the zoom-desync case where a seeded zoom has not reached the viewer yet. Component tests cover the widest-page rule across mixed-orientation documents and the pdf DOM path with a stubbed scale factor. Registry tests lock zoom-fit-width active/disabled state and the fit-width/manual toggle.
… (SD-3294) The UI host-event comment said three events; viewport-change made it four. Root export snapshots regenerate for the union of this branch's zoom types and the font types the base brought in from main.
The pre-commit format hook ran over the merge commit's full staged set and prettified 15 generated and upstream files this branch never touches (mcp catalog, document-api templates, font-system, sdk dispatch). A clean merge takes the base side verbatim for files only one side changed; restore those bytes so the PR diff carries zoom work only. Committed with hooks disabled so the formatter does not reintroduce the drift.
…dth (SD-3294) The mode-model rework widened the emit condition to any rounded availableWidth change, which the dedup unit test correctly rejected in CI: px-level jitter during a window drag would spam consumers with emits that cannot change any fit decision. Restore the intended key (rounded fitZoom plus rounded documentWidth); meaningful available-width changes already surface through fitZoom.
Five verified issues from the multi-agent and Codex review of #3659: - zoom.initial now reaches every surface at first paint: PdfViewer seeds its scale from a new initialScale prop (the activeZoom watcher never fires for a seeded ref, so a PDF painted 100% while getZoom() said 50, putting overlay math 2x off), and the non-layout-engine CSS fallback applies once from the document/editor ready hooks via the factored style application. - Fit-width targets what the renderer paints: the resolver prefers the widest laid-out page (editor.getPages(), the same source SuperEditor's container sizing uses for landscape sections) with body page styles as the pre-pagination fallback. - setZoom/setZoomMode before init now warn and emit nothing instead of advertising a change that was never persisted. - Stored viewport metrics are always latest (refreshed on any field change, frozen against consumer mutation) while the viewport-change event stays deduped to fit-relevant changes; all five public doc surfaces now state that contract precisely. getZoomState() derives its bounds from the same resolver the policy clamps with. - The applied fit floors at 1 (fractional bounds plus a degenerate container could round to 0, which the presentation engine rejects), and width/pagination evaluations defer a tick so measurement never runs against a mid-flush DOM (also fixes the one-frame sidebar bounce). The PDF page scan is skipped without PDF documents, the sidebar measures through a template ref, and the pt-to-px constant imports from the same module PdfViewerPage writes --scale-factor with.
The geometry 'zoom' latch only arms when the zoom value actually changed (seeded from the host state), so mode-only zoomChange emissions with no repaint to consume the tag no longer mis-label the next unrelated layout notification. useSuperDocZoom memoizes its return so the object identity is stable across unrelated parent renders, matching the controller-side slice memo it sits on.
… described (SD-3294) The d643fb9 message documented two freshness tiers, but a format-hook reformatting made the scripted edit miss silently and the diff never contained them. This commit holds the actual change: stored metrics refresh on any field change (frozen against consumer mutation) so getViewportMetrics() and ui.zoom reads are always latest, while the viewport-change event stays deduped to fit-relevant changes. The ui zoom slice's reference-keyed memo now documents the field-gated replacement invariant it relies on. Also drops two em dashes from comments per repo writing rules.
…D-3294) The F2 fix made the resolver prefer editor.getPages() with page styles as the pre-pagination fallback; the composable overview and the events doc still said page styles only.
…(SD-3294) SuperDoc.vue loads PdfViewer via defineAsyncComponent, so Vue receives the mocked module namespace and interop-probes it; vitest's strict proxy throws on the undeclared __isTeleport access. The __esModule flag routes Vue straight to the default export. Surfaced by the first test that actually renders a PDF document (the scan-gate test needs a real PDF-typed entry). The stub also declares the new initialScale prop.
|
Hey Matt, quick heads up on SD-3294. I stacked #3659 on top of this PR, and it changes the unreleased API a bit, so I wanted to flag it before I merge it into your branch. Main changes:
Your event idea and payload shape are still there. The naming and internals changed to make the feature safer before it ships. #3659 has been reviewed, follow-ups are fixed or ticketed in SD-3390, and CI SuperDoc is green on the current head. One note: stacked PRs do not run that suite automatically, so I dispatched it manually. There is also a small example PR, #3667, stacked on top. That one can wait. |
…ner-config feat(superdoc): zoom modes with viewport metrics and fit-width (SD-3294)
Summary
Payload
Usage