Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,28 @@ const BACKWARDS_COMPAT_SNAPSHOTS: BackwardsCompatCase[] = [
cellEntries: [["a", { type: "slide" }]],
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
},
},
{
// `speakerNotes` was added to SlideConfig. The validator must
// know about it (so it isn't silently stripped), the deserializer must
// carry it through, and serialize β†’ deserialize must round-trip it.
label: "speakerNotes round-trips through validate + (de)serialize",
input: {
cells: [
{ type: "slide", speakerNotes: "intro" },
{ type: "fragment", speakerNotes: "" },
{ type: "fragment", speakerNotes: "multi\n\nline\n\nnotes" },
],
},
expected: {
deck: {},
cellIds: ["a", "b", "c"],
cellEntries: [
["a", { type: "slide", speakerNotes: "intro" }],
["b", { type: "fragment", speakerNotes: "" }],
["c", { type: "fragment", speakerNotes: "multi\n\nline\n\nnotes" }],
],
},
},
];

describe("SlidesLayoutPlugin backwards compatibility", () => {
Expand All @@ -223,15 +245,16 @@ describe("SlidesLayoutPlugin backwards compatibility", () => {
parsed.success,
`validator rejected: ${JSON.stringify(input)}`,
).toBe(true);
if (!parsed.success) {
return;
}

// 2. Deserialize must succeed and reflect the user-set fields.
const layout = SlidesLayoutPlugin.deserializeLayout(
// Use the raw input (not the validator output) because that is what
// `deserializeLayout` actually receives in production today.
// oxlint-disable-next-line typescript/no-explicit-any
input as any,
cells,
);
// 2. Deserializing the *validator output* (not the raw input) must
// preserve every field listed in `expected.cellEntries`. This is what
// catches a schema regression: if the validator silently strips a
// known field, the deserialized config won't carry it and the
// assertion below fails.
const layout = SlidesLayoutPlugin.deserializeLayout(parsed.data, cells);
if (expected.deck !== undefined) {
expect(layout.deck).toEqual(expected.deck);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,16 @@ export const SlidesLayoutPlugin: ICellRendererPlugin<
type: "slides",
name: "Slides",

// All fields are optional so layouts saved by older marimo versions will work
// All fields are optional so layouts saved by older marimo versions will work.
// NOTE: every property of `SlideConfig` must be listed here β€” `z.object`
// strips unknown keys by default, so omitting a field silently drops it on
// any code path that runs the input through `validator.parse`.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe for future PR (and other plugins/validations), but would be nice for our zod schemas to be very flexible but .transform() into more strict types for the internals (wide/backward-compatible external, to narrow/strict internal type).

validator: z.object({
cells: z
.array(
z.object({
type: z.enum(["slide", "sub-slide", "fragment", "skip"]).optional(),
speakerNotes: z.string().optional(),
}),
)
.optional(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import React, { useMemo, useState } from "react";
import { useAtomValue } from "jotai";
import { numColumnsAtom } from "@/core/cells/cells";
import type { CellId } from "@/core/cells/ids";
import { kioskModeAtom } from "@/core/mode";
import type { ICellRendererProps } from "../types";
import type { SlidesLayout } from "./types";
import { computeSlideCellsInfo } from "./compute-slide-cells";
Expand All @@ -21,7 +22,10 @@ export const SlidesLayoutRenderer: React.FC<Props> = ({
cells,
mode,
}) => {
const isReading = mode === "read";
// Kiosk clients (e.g. reveal.js's speaker-view iframes) are read-only and
// shouldn't show authoring chrome, so we collapse to the read-mode layout.
const kioskMode = useAtomValue(kioskModeAtom);
const isReading = mode === "read" || kioskMode;
const numColumns = useAtomValue(numColumnsAtom);
const isMultiColumn = numColumns > 1;
const [activeCellId, setActiveCellId] = useState<CellId | null>(null);
Expand Down Expand Up @@ -52,14 +56,23 @@ export const SlidesLayoutRenderer: React.FC<Props> = ({
activeIndex={resolvedIndex}
onSlideChange={handleSlideChange}
configWidth={300}
mode={mode}
isEditable={mode !== "read"}
mode={isReading ? "read" : mode}
isEditable={!isReading}
/>
);

if (isReading) {
// Cap the deck height and derive width from height via aspect-video so it stays 16:9 without
// ballooning to the full viewport on wide screens.
// In kiosk mode (e.g. reveal.js's speaker-view iframes), anchor to the
// iframe viewport with `dvh`/`dvw` so the deck resizes with the popup
// window. The non-kiosk read mode keeps its 16:9 cap so the deck doesn't
// balloon to the full viewport on wide screens.
if (kioskMode) {
return (
<div className="flex h-dvh w-dvw overflow-hidden bg-background">
{slides}
</div>
);
}
return (
<div className="p-4 flex flex-1 items-center justify-center min-h-0">
<div className="h-full max-h-[95vh] aspect-video max-w-full flex">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export interface SlidesLayout extends Omit<
export type SlideType = "slide" | "sub-slide" | "fragment" | "skip";
export interface SlideConfig {
type?: SlideType;
speakerNotes?: string;
Comment thread
Light2Dark marked this conversation as resolved.
Outdated
}
Comment thread
Light2Dark marked this conversation as resolved.
Outdated

export type DeckTransition =
Expand Down
130 changes: 130 additions & 0 deletions frontend/src/components/slides/__tests__/slide-notes.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
/* Copyright 2026 Marimo. All rights reserved. */

import { describe, expect, it } from "vitest";
import type {
SlideConfig,
SlideType,
} from "@/components/editor/renderers/slides-layout/types";
import type { CellId } from "@/core/cells/ids";
import { composeSlides } from "../compose-slides";
import { buildSubslideNotes, collectBlockNotes } from "../slide-notes";

interface Cell {
id: CellId;
type?: SlideType;
}

const cell = (id: string, type?: SlideType): Cell => ({
id: id as CellId,
Comment thread
Light2Dark marked this conversation as resolved.
Outdated
type,
});

const configs = (
notes: Record<string, string>,
): ReadonlyMap<CellId, SlideConfig> =>
new Map(
Object.entries(notes).map(([id, speakerNotes]) => [
id as CellId,
{ speakerNotes } satisfies SlideConfig,
]),
);

const firstSubslide = (cells: Cell[]) =>
composeSlides({ cells, getType: (c) => c.type ?? "slide" }).stacks[0]
.subslides[0];

describe("collectBlockNotes", () => {
it("concatenates non-empty notes with paragraph spacing", () => {
const result = collectBlockNotes(
[cell("a"), cell("b"), cell("c")],
configs({ a: "first", b: "", c: "third" }),
);
expect(result).toBe("first\n\nthird");
});

it("returns an empty string when no cell has notes", () => {
expect(collectBlockNotes([cell("a")], configs({}))).toBe("");
});

it("ignores whitespace-only notes", () => {
expect(
collectBlockNotes([cell("a"), cell("b")], configs({ a: " ", b: "x" })),
).toBe("x");
});
});

describe("buildSubslideNotes", () => {
it("returns empty notes when no cell has any", () => {
const subslide = firstSubslide([cell("a"), cell("b", "fragment")]);
expect(buildSubslideNotes(subslide, configs({}))).toEqual({
slideLevel: "",
cumulativeByBlock: new Map(),
});
});

it("returns only slide-level notes when there are no fragments", () => {
const subslide = firstSubslide([cell("a")]);
expect(buildSubslideNotes(subslide, configs({ a: "intro" }))).toEqual({
slideLevel: "intro",
cumulativeByBlock: new Map(),
});
});

it("accumulates fragments below the slide-level notes with a divider", () => {
const subslide = firstSubslide([
cell("a"),
cell("b", "fragment"),
cell("c", "fragment"),
]);
const { slideLevel, cumulativeByBlock } = buildSubslideNotes(
subslide,
configs({ a: "intro", b: "step one", c: "step two" }),
);
expect(slideLevel).toBe("intro");
expect(cumulativeByBlock.get(1)).toBe("intro\n\n---\n\nstep one");
expect(cumulativeByBlock.get(2)).toBe(
"intro\n\n---\n\nstep one\n\n---\n\nstep two",
);
});

it("accumulates fragments with no slide-level notes", () => {
const subslide = firstSubslide([
cell("a"),
cell("b", "fragment"),
cell("c", "fragment"),
]);
const { slideLevel, cumulativeByBlock } = buildSubslideNotes(
subslide,
configs({ b: "first reveal", c: "second reveal" }),
);
expect(slideLevel).toBe("");
expect(cumulativeByBlock.get(1)).toBe("first reveal");
expect(cumulativeByBlock.get(2)).toBe(
"first reveal\n\n---\n\nsecond reveal",
);
});

it("skips empty fragments without leaving dangling dividers", () => {
const subslide = firstSubslide([
cell("a"),
cell("b", "fragment"),
cell("c", "fragment"),
]);
const { cumulativeByBlock } = buildSubslideNotes(
subslide,
configs({ a: "intro", c: "third" }),
);
expect(cumulativeByBlock.get(1)).toBe("intro");
expect(cumulativeByBlock.get(2)).toBe("intro\n\n---\n\nthird");
});

it("returns no cumulative entries when fragments and slide have no notes", () => {
const subslide = firstSubslide([
cell("a"),
cell("b", "fragment"),
cell("c", "fragment"),
]);
const { cumulativeByBlock } = buildSubslideNotes(subslide, configs({}));
expect(cumulativeByBlock.size).toBe(0);
});
});
Loading
Loading