Add slide config form in sidebar, and reveal slide types#9300
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Bundle ReportChanges will increase total bundle size by 20.28kB (0.08%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: marimo-esmAssets Changed:
Files in
Files in
Files in
Files in
|
There was a problem hiding this comment.
Pull request overview
This PR expands the slides layout to support slide metadata (slide / sub-slide / fragment / skip), adds a slide-type configuration sidebar to the Reveal.js-based slide renderer, and includes a nested-slides smoke test to exercise the new behavior.
Changes:
- Add slide layout serialization/deserialization (
cells[]) and UI to edit slide types per cell. - Introduce slide composition logic (stacks/subslides/fragments) for Reveal.js rendering + indexing to sync active cell ↔ Reveal indices.
- Add a nested-slides smoke test notebook + layout JSON for vertical stacks, fragments, and skipped cells.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| marimo/_smoke_tests/slides_examples/nested_slides.py | New smoke test notebook exercising nested slides, fragments, and skip cells |
| marimo/_smoke_tests/slides_examples/layouts/nested_slides.slides.json | Serialized slides layout used by the nested smoke test |
| frontend/src/components/slides/slide-form.tsx | New sidebar form for setting slide type per cell |
| frontend/src/components/slides/reveal-slides.css | Reveal-specific CSS updates for nested sections and fragment block stacking |
| frontend/src/components/slides/reveal-component.tsx | Major refactor: compose stacks/subslides/fragments, add config sidebar, sync active cell with Reveal indices |
| frontend/src/components/slides/minimap.tsx | Minimap updates: dynamic thumbnail sizing + skipped-cell affordances |
| frontend/src/components/slides/compose-slides.ts | New composition + indexing utilities for slides/fragments/subslides |
| frontend/src/components/slides/tests/compose-slides.test.ts | Unit tests for composition and index mapping logic |
| frontend/src/components/editor/renderers/types.ts | Clarify serialize/deserialize layout docs |
| frontend/src/components/editor/renderers/slides-layout/types.ts | Define serialized/in-memory slides layout schema + slide config types |
| frontend/src/components/editor/renderers/slides-layout/slides-layout.tsx | Wire layout + setLayout into slides renderer; track skipped cells; default index selection |
| frontend/src/components/editor/renderers/slides-layout/plugin.tsx | Implement slides layout schema validation + (de)serialization |
| const cells = notebookCells(notebook); | ||
| // Fall back to the plugin's initial layout when the user has not yet | ||
| // interacted with this layout — otherwise serializers that expect a | ||
| // structured layout object crash on `undefined`. | ||
| const data = layoutData[selectedLayout] ?? plugin.getInitialLayout(cells); | ||
| return { | ||
| type: selectedLayout, | ||
| data: plugin.serializeLayout(data, notebookCells(notebook)), | ||
| data: plugin.serializeLayout(data, cells), | ||
| }; |
There was a problem hiding this comment.
getSerializedLayout now uses plugin.getInitialLayout(cells) as a fallback, but LayoutState.layoutData is still typed as GridLayout | undefined. This makes layout state/serialization rely on any for non-grid layouts (like slides) and weakens type safety. Consider widening LayoutData to a union of all layout types (e.g. GridLayout | SlidesLayout | undefined) so state and serialization stay type-correct.
There was a problem hiding this comment.
can fix this in a follow-up
| </div> | ||
| )} | ||
| </aside> | ||
| )} |
There was a problem hiding this comment.
might be worth breaking this component up into 3-4 sub components
There was a problem hiding this comment.
moved the sidebar to the slide-form file, I can move out more stuff once we introduce Live editing
![cubic-dev-ai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
peter-gy
left a comment
peter-gy
left a comment
There was a problem hiding this comment.
all in all this looks good!
I believe the unresolved comments are fine to be addtessed in a follow up.
| // Skip cells aren't part of the composed deck. When one is selected in the | ||
| // minimap we render a preview over the deck and park reveal on a neighboring | ||
| // real slide; keyboard nav while parked is handled below. | ||
| const skippedPreviewCell = | ||
| activeCell && layout.cells.get(activeCell.id)?.type === "skip" | ||
| ? activeCell | ||
| : null; |
There was a problem hiding this comment.
I think this leaks skipped cells in read/presentation mode when the selected or initial cell is skipped. The smoke layout starts with type: "skip", but SlidesLayoutRenderer defaults activeIndex to 0, and RevealSlidesComponent builds skippedPreviewCell regardless of mode. So we end up showing the slide that's supposed to be skipped:
Can we gate the skipped-preview behavior to authoring mode, or initialize read-mode navigation to the first non-skipped deck target?
There was a problem hiding this comment.
Oh, good catch, thank you!
kirangadhave
left a comment
kirangadhave
left a comment
There was a problem hiding this comment.
🚀 looks like you have addressed most of the feedback
|
🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.23.3-dev45 |
📝 Summary
Screen.Recording.2026-04-22.at.4.47.06.PM.mov
in read mode
SerializedSlidesLayout now carries optional cells: SlideConfig[] and deck: DeckConfig; both are optional so existing .slides.json files (including bare {}) still deserialize cleanly.
SlideConfig supports type: "slide" | "sub-slide" | "fragment" | "skip"
DeckConfig supports transition: "none" | "fade" | "slide" | "convex" | "concave" | "zoom".
New compose-slides.ts groups the flat cell list into stacks → subslides → blocks, mirroring reveal.js's {h, v, f} indices, with helpers buildSlideIndices and resolveActiveCellIndex.
New tabbed SlidesForm (Slide | Deck):
📋 Pre-Review Checklist
✅ Merge Checklist