From 3a40ff9b6d782fa699d52931e2897525a0bc7cf0 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sun, 13 Sep 2026 01:25:31 +0200 Subject: [PATCH] fix: the theme reaches the renderer, so changing it changes the deck MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Picking a theme on a saved resource did nothing. The picker saved the choice, the route wrote deck.theme, and the renderer knew how to apply it — the theme was lost in between. attachFigures, the step that puts drawn figures back onto slides, rebuilds the deck as a fresh object: var out = { title, subtitle, date, slides }; A fresh object keeps only the fields it names, and theme was not one. So every export rendered in the default palette, whatever the picker said. Rendering the same deck under clinical-blue, teaching-amber and high-contrast produced three byte-identical files; it now produces three different ones, and they look different. Silent, because nothing downstream could tell the difference between a deck with no theme and a deck whose theme had been dropped — both mean "use the default", which is also the right behaviour for an unknown id. Known and not fixed here: the two tinted cards on a compare slide are hardcoded blue and amber rather than taken from the theme, so those stay the same colour under every palette. The headings, accent, rules and bullets do change. That is a gap in the theme definition, not in this path. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU --- src/utils/documentExport.js | 10 ++++++- test/deck-theme-applied.test.js | 47 +++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 test/deck-theme-applied.test.js diff --git a/src/utils/documentExport.js b/src/utils/documentExport.js index 90563247..2ac50847 100644 --- a/src/utils/documentExport.js +++ b/src/utils/documentExport.js @@ -234,7 +234,15 @@ function attachFigures(deck, files, figureIds) { } return copy; }); - var out = { title: deck.title, subtitle: deck.subtitle, date: deck.date, slides: slides }; + // theme included, and that is not incidental. Rebuilding the deck as a fresh + // object drops every field not named here, and theme was not named — so a + // deck re-skinned in the library rendered in the default palette anyway, and + // the picker looked broken because nothing it did ever reached the renderer. + // Three themes produced byte-identical files. + var out = { + title: deck.title, subtitle: deck.subtitle, date: deck.date, + theme: deck.theme, slides: slides + }; if (deck.title && !slides.some(function (s) { return s.type === 'title'; })) { out.slides = [{ type: 'title', heading: deck.title, subtitle: deck.subtitle, date: deck.date }] .concat(slides); diff --git a/test/deck-theme-applied.test.js b/test/deck-theme-applied.test.js new file mode 100644 index 00000000..f444e8be --- /dev/null +++ b/test/deck-theme-applied.test.js @@ -0,0 +1,47 @@ +// A deck re-skinned in the library rendered in the default palette anyway. The +// picker saved the choice correctly and the renderer could apply it correctly; +// the theme was dropped in between, by the step that puts the drawn figures +// back onto the slides. +// +// attachFigures rebuilds the deck as a fresh object, and a fresh object keeps +// only the fields it names. It named title, subtitle, date and slides. Three +// different themes rendered byte-identical files. +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const read = f => fs.readFileSync(path.join(__dirname, '..', f), 'utf8'); + +test('the theme survives the trip to the renderer', () => { + const src = read('src/utils/documentExport.js'); + const out = src.slice(src.indexOf('var out = {'), src.indexOf('var out = {') + 220); + assert.match(out, /theme: deck\.theme/, + 'attachFigures drops the theme, so every export renders in the default palette'); +}); + +test('every field the renderer reads off the deck is carried', () => { + // The failure was one missing key in an object literal, and it was silent. + // Anything the renderer reads at deck level has to be listed there. + const src = read('src/utils/documentExport.js'); + const out = src.slice(src.indexOf('var out = {'), src.indexOf('var out = {') + 220); + for (const field of ['title', 'subtitle', 'date', 'theme', 'slides']) { + assert.match(out, new RegExp(field + ':'), field + ' is not carried through attachFigures'); + } +}); + +test('the saved theme is what the library writes, and the renderer reads', () => { + // Both ends already worked; this pins that they agree on the field name. + const route = read('src/routes/myResources.js'); + assert.match(route, /deck\.theme = theme/, 'the library writes deck.theme'); + const renderer = read('scripts/render_pptx.py'); + assert.match(renderer, /apply_theme\(/); + assert.match(renderer, /spec\.get\("theme"\)|deck\.get\("theme"\)|data\.get\("theme"\)/, + 'the renderer reads the theme off the deck'); +}); + +test('an unknown theme leaves the default rather than failing the render', () => { + const renderer = read('scripts/render_pptx.py'); + const fn = renderer.slice(renderer.indexOf('def apply_theme')); + assert.match(fn.slice(0, 400), /a deck rendering in the wrong colours beats a deck not rendering/); +});