mirror of
https://github.com/nexu-io/open-design.git
synced 2026-10-27 09:51:12 +01:00
fix(desktop): reveal every deck-stage slide in multi-page PPT/PDF export (#5645)
* fix(desktop): reveal every deck-stage slide in multi-page PPT/PDF export
Exporting a multi-page deck to PDF/PPTX produced one populated page
followed by blank pages. Root cause is in the desktop slide-capture path
(apps/desktop/src/main/deck-capture.ts): `showSlide` selects the page to
screenshot by toggling active *classes* and setting *non-important* inline
styles.
That loses the CSS cascade for `<deck-stage>` decks. The custom element and
its injected fallback (packages/contracts/src/runtime/deck-stage-fallback.ts)
hide every slotted slide with `::slotted(*){visibility:hidden!important}` and
reveal only the one carrying the `data-od-deck-active` attribute. A plain
inline `visibility:visible` cannot override a shadow `!important` rule, and
the export never set that attribute — so only the initially-active slide 0
stayed visible and every other slide captured blank. `restackActiveSlide`
and `showAllSlides` already used `!important`; `showSlide` was the lone
non-important selector, which is why the screenshot PPTX/PDF path regressed.
Fix: `showSlide` now sets opacity/visibility/pointer-events/z-index with
`!important` priority (an inline `!important` beats a shadow `::slotted()
!important` rule) and toggles the `data-deck-active` / `data-od-deck-active`
attributes deck runtimes key their reveal rule on. Verified in a real
browser: showing slide index 1 flips its computed visibility from `hidden`
to `visible`.
Red spec: apps/desktop/tests/main/scroll-stitch-geometry.test.ts
"per-slide selection wins the deck-stage !important visibility cascade" —
red on the pre-fix body, green after.
Source: https://plane.powerformer.net/open-design/projects/49832a02-3158-4faf-bf2f-d0e39c40c7e6/issues/3cfcfe64-295c-4a1c-ac6a-156549f599e6
Generated-By: looper 0.0.0-dev (runner=worker, agent=claude-code)
* test(desktop): tighten deck-stage regression oracle to the exact fallback contract
Address review feedback on #5645: the regression test's `slideRenders` oracle
was looser than the deck-stage fallback contract it cites. The fallback
(packages/contracts/src/runtime/deck-stage-fallback.ts) reveals ONLY
`data-od-deck-active`, but the oracle also accepted `data-deck-active` (which
belongs to the real deck-stage.js runtime), so a relapse that dropped
`data-od-deck-active` and the `!important` visibility could have kept the test
green while fallback-backed exports went blank again.
Tighten it: the oracle now honors only `data-od-deck-active`, and the test adds
explicit assertions that the selected slide is revealed via inline
`visibility:visible` at `!important` priority AND carries `data-od-deck-active`,
while non-selected slides are `hidden !important` and unmarked. Either half
regressing now fails the test.
Behavior unchanged; test-only. `pnpm --filter @open-design/desktop typecheck`
and the deck-capture suite (27/27) pass.
Generated-By: looper 0.0.0-dev (runner=worker, agent=claude-code)
* fix(desktop): stop toggling data-deck-active on export; correct the cascade rationale
Address the CHANGES_REQUESTED review on #5645 (two valid points, both verified
in a real browser).
1) Blocking: showSlide toggled BOTH `data-od-deck-active` and the real
deck-stage.js runtime's `data-deck-active`. Setting `data-deck-active` can
replay authored `[data-deck-active]` entrance animations on a slide's
descendants, which `prepareDeckStage`'s slide-level `animation:none` does not
suppress (animation is not inherited) — risking a mid-transition capture for
slide 2..N. It is also unnecessary: the real runtime hides slides with a
NON-important `::slotted(*)`, so the inline `!important` visibility/opacity
overrides already reveal those slides outright (importance beats non-important
regardless of context — confirmed in-browser). Drop the `data-deck-active`
toggle; keep only `data-od-deck-active` (the fallback's reveal attribute).
2) The prior comment/test claimed inline `visibility:visible !important` beats
the fallback's shadow `::slotted(*){visibility:hidden !important}`. That is
false: for `!important` declarations the inner (shadow) context wins before
inline precedence is considered, so inline `!important` alone leaves a
fallback slide hidden — the `data-od-deck-active` attribute is what reveals
it. Verified in a real browser (inline-important-only → hidden;
attribute-only → visible). Correct the code comment and rewrite the regression
oracle to model the true cascade, focusing the unit test on the
`data-od-deck-active` toggle (and asserting `data-deck-active` is NOT set).
Reveal still works for every runtime: fallback via `data-od-deck-active`; real
deck-stage.js and class/light-DOM decks via the inline `!important` overrides +
class toggle.
Validation: apps/desktop typecheck; deck-capture suite 27/27 (red-verified by
neutralizing the attribute toggle); full desktop suite 211/212 (the one failure
is the pre-existing updater.test.ts baseline flake); pnpm guard 86/86.
Generated-By: looper 0.0.0-dev (runner=worker, agent=claude-code)
* test(desktop): relax over-constrained deck-active assertion to the fallback contract
Address non-blocking review feedback on #5645. The regression test asserted
`expect(slides[1].hasAttribute('data-deck-active')).toBe(false)`, which pins an
implementation detail rather than the fallback contract. The fallback
(packages/contracts/src/runtime/deck-stage-fallback.ts) only requires the
selected slide to carry `data-od-deck-active`; it does not care whether
`data-deck-active` is present. And the real export freezes descendant
animations in prepareDeckStage() (`*{animation-duration:0s!important;
transition-duration:0s!important}`) before slide selection, so an implementation
that also set `data-deck-active` would still be a correct fix — the assertion
would fail a valid future change for a reason the runtime does not care about.
Drop that assertion; keep the test focused on the fallback-specific contract
(selected slide has `data-od-deck-active`, others do not) plus the inline
`!important` override mechanism. Also soften the showSlide code comment: not
setting `data-deck-active` is primarily because it is unnecessary for reveal —
prepareDeckStage's freeze already neutralizes any `[data-deck-active]` motion —
not because setting it would be unsafe.
Validation: apps/desktop typecheck; deck-capture suite 27/27; pnpm guard 86/86.
Generated-By: looper 0.0.0-dev (runner=worker, agent=claude-code)
This commit is contained in:
@@ -1315,7 +1315,8 @@ export function restoreActiveSlideCapture(): void {
|
||||
|
||||
// Returns a Promise that resolves after the style change has settled for two
|
||||
// animation frames, so the caller can show + wait in a single round trip.
|
||||
function showSlide(slideSelector: string, index: number): Promise<{ x: number; y: number; w: number; h: number } | null> {
|
||||
// Exported so focused tests can drive the real per-slide selection.
|
||||
export function showSlide(slideSelector: string, index: number): Promise<{ x: number; y: number; w: number; h: number } | null> {
|
||||
restoreActiveSlideCapture();
|
||||
const slides = Array.prototype.slice
|
||||
.call(document.querySelectorAll(slideSelector))
|
||||
@@ -1324,16 +1325,35 @@ function showSlide(slideSelector: string, index: number): Promise<{ x: number; y
|
||||
// the slide (incl. visibility:hidden->visible and reveal animations), plus
|
||||
// inline overrides as a backstop for decks that hide via opacity/visibility.
|
||||
const activeClasses = ["active", "visible", "is-active", "current"];
|
||||
// The injected <deck-stage> fallback (packages/contracts/src/runtime/
|
||||
// deck-stage-fallback.ts) hides slotted slides with an `!important` shadow rule
|
||||
// and reveals ONLY the one carrying `data-od-deck-active`. We toggle exactly that
|
||||
// attribute. We do NOT also set the real deck-stage.js runtime's
|
||||
// `data-deck-active`: it is unnecessary for reveal (mechanism 1 below already
|
||||
// reveals that runtime's slides), and skipping it keeps the export from depending
|
||||
// on the prepareDeckStage() animation freeze to neutralize any authored
|
||||
// `[data-deck-active]`-keyed entrance motion.
|
||||
const activeAttributes = ["data-od-deck-active"];
|
||||
slides.forEach((node, k) => {
|
||||
const el = node as HTMLElement;
|
||||
const on = k === index;
|
||||
el.style.transition = "none";
|
||||
el.style.animation = "none";
|
||||
el.style.opacity = on ? "1" : "0";
|
||||
el.style.visibility = on ? "visible" : "hidden";
|
||||
el.style.pointerEvents = on ? "auto" : "none";
|
||||
el.style.zIndex = on ? "999" : "0";
|
||||
// Reveal the captured slide through the two mechanisms real decks actually use:
|
||||
// 1. Inline `!important` styles beat a deck's own NON-important hide rules —
|
||||
// the real <deck-stage> runtime's `::slotted(*){visibility:hidden}` and
|
||||
// class-based `.slide` decks — because importance wins outright there.
|
||||
// 2. The `data-od-deck-active` attribute is the ONLY thing that reveals the
|
||||
// fallback, whose hide rule is `::slotted(*){visibility:hidden!important}`
|
||||
// in its shadow root: a shadow-tree `!important` declaration beats an outer
|
||||
// inline `!important` one (for `!important`, the inner context wins), so
|
||||
// inline styles alone cannot reveal a fallback slide — the attribute can.
|
||||
el.style.setProperty("transition", "none", "important");
|
||||
el.style.setProperty("animation", "none", "important");
|
||||
el.style.setProperty("opacity", on ? "1" : "0", "important");
|
||||
el.style.setProperty("visibility", on ? "visible" : "hidden", "important");
|
||||
el.style.setProperty("pointer-events", on ? "auto" : "none", "important");
|
||||
el.style.setProperty("z-index", on ? "999" : "0", "important");
|
||||
activeClasses.forEach((c) => el.classList.toggle(c, on));
|
||||
activeAttributes.forEach((a) => el.toggleAttribute(a, on));
|
||||
});
|
||||
// Report where the active slide actually landed after two frames, so the
|
||||
// capturer can detect a slide that the deck keeps off-screen (e.g. a
|
||||
|
||||
@@ -17,6 +17,7 @@ import {
|
||||
runDomToPptx,
|
||||
scrollStitchGeometry,
|
||||
scrollStitchRowOffset,
|
||||
showSlide,
|
||||
shouldCapturePageAsJpeg,
|
||||
shouldCaptureAsDeck,
|
||||
solidBgraBuffer,
|
||||
@@ -174,6 +175,18 @@ describe('deck capture DOM prep', () => {
|
||||
runtimeFrame: symbol | undefined;
|
||||
style = new FakeStyle();
|
||||
classList = { toggle: () => true };
|
||||
private readonly attributes = new Set<string>();
|
||||
|
||||
toggleAttribute(name: string, force?: boolean): boolean {
|
||||
const on = force === undefined ? !this.attributes.has(name) : force;
|
||||
if (on) this.attributes.add(name);
|
||||
else this.attributes.delete(name);
|
||||
return on;
|
||||
}
|
||||
|
||||
hasAttribute(name: string): boolean {
|
||||
return this.attributes.has(name);
|
||||
}
|
||||
|
||||
constructor(
|
||||
private rect: { x: number; y: number } = { x: 32, y: 24 },
|
||||
@@ -361,6 +374,121 @@ describe('deck capture DOM prep', () => {
|
||||
Object.assign(globalThis, { document: previousDocument });
|
||||
}
|
||||
});
|
||||
|
||||
// Regression for issue #990 ("导出PPT多页时后续页面内容丢失"): the injected
|
||||
// <deck-stage> fallback (packages/contracts/src/runtime/deck-stage-fallback.ts)
|
||||
// hides every slotted slide with `::slotted(*){visibility:hidden!important}` and
|
||||
// reveals only the one carrying the `data-od-deck-active` attribute. showSlide
|
||||
// picks the page to capture, so it must set that attribute on the selected slide;
|
||||
// the pre-fix code toggled classes and set non-important inline styles only, so
|
||||
// every slide but the first captured blank (one populated page followed by blanks).
|
||||
test('per-slide selection marks the deck-stage fallback active attribute', async () => {
|
||||
class FakeStyle {
|
||||
private readonly values = new Map<string, { priority: string; value: string }>();
|
||||
setProperty(name: string, value: string, priority = ''): void {
|
||||
this.values.set(name, { priority, value });
|
||||
}
|
||||
getPropertyValue(name: string): string {
|
||||
return this.values.get(name)?.value ?? '';
|
||||
}
|
||||
getPropertyPriority(name: string): string {
|
||||
return this.values.get(name)?.priority ?? '';
|
||||
}
|
||||
// Plain `el.style.foo = x` assignments (how the pre-fix code showed slides)
|
||||
// are non-`!important`, mirrored here by recording an empty priority.
|
||||
set animation(value: string) {
|
||||
this.setProperty('animation', value);
|
||||
}
|
||||
set opacity(value: string) {
|
||||
this.setProperty('opacity', value);
|
||||
}
|
||||
set pointerEvents(value: string) {
|
||||
this.setProperty('pointer-events', value);
|
||||
}
|
||||
set transition(value: string) {
|
||||
this.setProperty('transition', value);
|
||||
}
|
||||
set visibility(value: string) {
|
||||
this.setProperty('visibility', value);
|
||||
}
|
||||
set zIndex(value: string) {
|
||||
this.setProperty('z-index', value);
|
||||
}
|
||||
}
|
||||
class Slide {
|
||||
style = new FakeStyle();
|
||||
classList = { toggle: () => true };
|
||||
private readonly attributes = new Set<string>();
|
||||
closest(): null {
|
||||
return null;
|
||||
}
|
||||
getBoundingClientRect(): DOMRect {
|
||||
return { x: 0, y: 0, width: 1920, height: 1080 } as DOMRect;
|
||||
}
|
||||
toggleAttribute(name: string, force?: boolean): boolean {
|
||||
const on = force === undefined ? !this.attributes.has(name) : force;
|
||||
if (on) this.attributes.add(name);
|
||||
else this.attributes.delete(name);
|
||||
return on;
|
||||
}
|
||||
hasAttribute(name: string): boolean {
|
||||
return this.attributes.has(name);
|
||||
}
|
||||
}
|
||||
|
||||
// The fallback (packages/contracts/src/runtime/deck-stage-fallback.ts) hides
|
||||
// slotted slides with `::slotted(*){visibility:hidden!important}` in its shadow
|
||||
// root and reveals ONLY `::slotted([data-od-deck-active])`. Because a shadow
|
||||
// `!important` declaration beats an outer inline `!important` one (for
|
||||
// `!important`, the inner tree wins BEFORE inline precedence is considered —
|
||||
// verified in a real browser), inline `visibility:visible !important` on a
|
||||
// slotted slide does NOT reveal it. Under the fallback a slide renders iff it
|
||||
// carries `data-od-deck-active` — that attribute toggle is the mechanism this
|
||||
// unit test guards. (The inline `!important` override handles the OTHER runtimes
|
||||
// — real deck-stage.js with non-`!important` `::slotted`, and class-based decks
|
||||
// — where importance wins outright; that path is covered separately below.)
|
||||
const revealedByFallback = (slide: Slide): boolean => slide.hasAttribute('data-od-deck-active');
|
||||
|
||||
const slides = [new Slide(), new Slide(), new Slide()];
|
||||
const previousDocument = globalThis.document;
|
||||
const previousRaf = globalThis.requestAnimationFrame;
|
||||
Object.assign(globalThis, {
|
||||
document: { getElementById: () => null, querySelectorAll: () => slides },
|
||||
requestAnimationFrame: (cb: FrameRequestCallback) => {
|
||||
cb(0);
|
||||
return 0;
|
||||
},
|
||||
});
|
||||
try {
|
||||
// Export the SECOND page (index 1) — the one the bug left blank.
|
||||
await showSlide('.slide, [data-screen-label], .deck-slide, .ppt-slide', 1);
|
||||
// Fallback mechanism: exactly the selected slide is marked, so only it renders.
|
||||
expect(revealedByFallback(slides[1])).toBe(true);
|
||||
expect(revealedByFallback(slides[0])).toBe(false);
|
||||
expect(revealedByFallback(slides[2])).toBe(false);
|
||||
// NB: we intentionally do NOT assert `data-deck-active` is absent — the
|
||||
// fallback contract only requires `data-od-deck-active` on the selected
|
||||
// slide, and the real export freezes descendant animations in
|
||||
// prepareDeckStage() before selection, so an implementation that also set
|
||||
// `data-deck-active` would still be correct. Constraining it here would fail
|
||||
// a valid future fix for a reason the fallback runtime does not care about.
|
||||
// Inline override for the non-shadow / non-`!important`-hide runtimes: the
|
||||
// selected slide is forced visible and the rest hidden. This is a DIFFERENT
|
||||
// mechanism from the fallback attribute above — it wins for those runtimes by
|
||||
// importance, and does not (need to) beat the fallback's shadow `!important`.
|
||||
expect(slides[1].style.getPropertyValue('visibility')).toBe('visible');
|
||||
expect(slides[1].style.getPropertyPriority('visibility')).toBe('important');
|
||||
for (const off of [slides[0], slides[2]]) {
|
||||
expect(off.style.getPropertyValue('visibility')).toBe('hidden');
|
||||
expect(off.style.getPropertyPriority('visibility')).toBe('important');
|
||||
}
|
||||
} finally {
|
||||
Object.assign(globalThis, {
|
||||
document: previousDocument,
|
||||
requestAnimationFrame: previousRaf,
|
||||
});
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('readDomToPptxBundleFile', () => {
|
||||
|
||||
Reference in New Issue
Block a user