From c8dc372519a9790cce9fc9cb64e8729800ebbe37 Mon Sep 17 00:00:00 2001 From: Mex <44814932+MMexicann@users.noreply.github.com> Date: Thu, 15 Oct 2026 22:31:32 +0200 Subject: [PATCH] M6.1: preserve PPTX trigger click steps Keep later On Click effects and paragraph builds in separate interactive steps. Add round-trip and headless show regressions. Contribution on behalf of @MMexicann. --- ROADMAP.md | 2 + crates/engine/tests/show_clicks.rs | 28 +++++++++ crates/pptx/src/write/timing.rs | 19 ++---- crates/pptx/tests/trigger_steps.rs | 95 ++++++++++++++++++++++++++++++ docs/animation-parity.md | 3 +- docs/file-format-parity.md | 1 + docs/gaps.md | 10 ++-- docs/parity-checklist.md | 2 +- docs/roadmap.md | 1 + 9 files changed, 139 insertions(+), 22 deletions(-) create mode 100644 crates/pptx/tests/trigger_steps.rs diff --git a/ROADMAP.md b/ROADMAP.md index 8ea976e6..5e33e213 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -131,6 +131,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | What landed | |---|---| +| 2026-10-15 | PPTX trigger sequences keep separate On Click steps, including paragraph builds; synthetic import/edit/save/reopen and headless show regressions | | 2026-10-15 | PDF export and rendering: `anchorCtr` centres the drawn text block — right-aligned text no longer ran past the box edge (#736) | | 2026-10-14 | connected MCP rejects unrelated, malformed, incomplete and oversized replies without replay; buffered unsolicited bytes force a fresh connection before sending (#718) | | 2026-10-14 | Empty bulleted and numbered paragraphs show gray markers while editing, including on first click and after Enter. Markers use the list style and stay out of slide shows and exports. | @@ -226,6 +227,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | Change | Summary | |---|---|---| +| 2026-10-15 | trivial | Progress log: PPTX trigger click-step round trip. Feature scores and milestone names unchanged | | 2026-10-14 | trivial | remote MCP checks reply IDs, envelopes and bounded newline framing (#718) | | 2026-10-14 | minor | Gray list markers on empty paragraphs while editing; caret keeps the list indent before typing | | 2026-10-14 | minor | Community PR wave folded: headline breadth 84% over 191 features (P0 92%, P1 82%, P2 55%) and the feature-area breadths from `cargo xtask parity`; one row per area; Upcoming item 2 merged; per-PR count notes dropped from the progress log. Ready stays ~55%, stage alpha | diff --git a/crates/engine/tests/show_clicks.rs b/crates/engine/tests/show_clicks.rs index 92da2f7f..42c0f701 100644 --- a/crates/engine/tests/show_clicks.rs +++ b/crates/engine/tests/show_clicks.rs @@ -59,6 +59,34 @@ fn start(p: &mut Presentation) { p.slides.clear(); } +#[test] +fn saved_trigger_reveals_then_hides_on_separate_clicks() { + let mut p = defaults::new_presentation(None); + start(&mut p); + let button = shape(&mut p, "Button", "Toggle", 20.0, 20.0, 100.0, 40.0); + let target = shape(&mut p, "Target", "Answer", 200.0, 100.0, 200.0, 80.0); + let animations = vec![ + Animation { + shape: target.id, + trigger: Some(button.id), + effect: "appear".into(), + start: AnimStart::OnClick, + duration_ms: 0, + ..Default::default() + }, + disappear_on_click(button.id, target.id), + ]; + add_slide(&mut p, vec![button, target], animations, false); + let mut show = Show::open(&p); + assert!(!show.shown("Target")); + for _ in 0..2 { + assert_eq!(show.click("Button"), Click::Animated); + assert!(show.shown("Target"), "first click reveals the answer until the next click"); + assert_eq!(show.click("Button"), Click::Animated); + assert!(!show.shown("Target"), "second click hides the answer"); + } +} + /// The slide show window's click handling, on a deck read from a PPTX file. struct Show { pres: Presentation, diff --git a/crates/pptx/src/write/timing.rs b/crates/pptx/src/write/timing.rs index ec4bffd2..3839d992 100644 --- a/crates/pptx/src/write/timing.rs +++ b/crates/pptx/src/write/timing.rs @@ -655,21 +655,10 @@ pub fn timing(w: &mut W, x: &mut Exp, anims: &[Animation], shapes: &[Shape], idm w.close("p:seq"); } for (trig, effs) in triggered { - // Everything a trigger starts plays from its click. - let effs: Vec = effs - .into_iter() - .enumerate() - .map(|(i, e)| Eff { - start: if i == 0 { - AnimStart::OnClick - } else if e.start == AnimStart::OnClick { - AnimStart::AfterPrevious - } else { - e.start - }, - ..e - }) - .collect(); + // The first step starts with the button's click. Later On Click effects must remain + // separate steps; With/After Previous effects still belong to their preceding step. + let effs: Vec = + effs.into_iter().enumerate().map(|(i, e)| Eff { start: if i == 0 { AnimStart::OnClick } else { e.start }, ..e }).collect(); let clicks = group(effs); let trig_cond = |w: &mut W| { w.open("p:cond", A::new().a("evt", "onClick").a("delay", 0)); diff --git a/crates/pptx/tests/trigger_steps.rs b/crates/pptx/tests/trigger_steps.rs new file mode 100644 index 00000000..409b10d6 --- /dev/null +++ b/crates/pptx/tests/trigger_steps.rs @@ -0,0 +1,95 @@ +//! Synthetic interactive sequences must keep one step per click across a save. + +use std::io::{Cursor, Read}; +use std::sync::Arc; + +use deckcraft_geom::Xfrm; +use deckcraft_model::anim::{AnimStart, Animation, TextBuild}; +use deckcraft_model::text::TextBody; +use deckcraft_model::{Presentation, Shape, ShapeId, defaults}; + +fn deck() -> Presentation { + let mut p = defaults::new_presentation(None); + let shapes: Vec<_> = ["Button", "Other button", "Content"] + .into_iter() + .map(|name| Shape { + id: ShapeId(p.alloc_id().expect("id")), + name: name.into(), + xfrm: Some(Xfrm::new(40.0, 40.0, 200.0, 100.0)), + text: Some(TextBody::from_text("First\nSecond\nThird")), + ..Default::default() + }) + .collect(); + Arc::make_mut(&mut p.slides[0]).shapes = shapes; + p +} + +fn round(p: &Presentation) -> Presentation { + deckcraft_pptx::import(&deckcraft_pptx::export(p).expect("export")).expect("import") +} + +fn slide_xml(p: &Presentation) -> String { + let bytes = deckcraft_pptx::export(p).expect("export"); + let mut z = zip::ZipArchive::new(Cursor::new(bytes)).expect("zip"); + let mut xml = String::new(); + z.by_name("ppt/slides/slide1.xml").expect("slide").read_to_string(&mut xml).expect("xml"); + xml +} + +#[test] +fn triggered_click_steps_survive_import_edit_save_reopen() { + use AnimStart::{AfterPrevious, OnClick, WithPrevious}; + let mut p = deck(); + let ids: Vec<_> = p.slides[0].shapes.iter().map(|s| s.id).collect(); + let starts = [OnClick, WithPrevious, AfterPrevious, OnClick, WithPrevious, OnClick]; + for (i, start) in starts.into_iter().enumerate() { + Arc::make_mut(&mut p.slides[0]).animations.push(Animation { + shape: ids[2], + trigger: Some(ids[0]), + start, + effect: "appear".into(), + delay_ms: i as u32 * 100, + ..Default::default() + }); + } + // An independent button and the main sequence must not borrow this button's steps. + for trigger in [Some(ids[1]), None] { + for start in [OnClick, AfterPrevious, OnClick] { + Arc::make_mut(&mut p.slides[0]).animations.push(Animation { shape: ids[2], trigger, start, ..Default::default() }); + } + } + let mut q = round(&p); + for pass in 0..2 { + for trigger in [Some(ids[0]), Some(ids[1]), None] { + let state = |deck: &Presentation| { + deck.slides[0].animations.iter().filter(|a| a.trigger == trigger).map(|a| (a.start, a.delay_ms)).collect::>() + }; + assert_eq!(state(&q), state(&p), "trigger {trigger:?}, pass {pass}"); + } + assert_eq!(q.slides[0].animations.len(), p.slides[0].animations.len()); + Arc::make_mut(&mut q.slides[0]).shapes[2].name = "Edited content".into(); + q = round(&q); + } + let xml = slide_xml(&p); + assert_eq!(xml.matches("nodeType=\"interactiveSeq\"").count(), 2); + assert_eq!(xml.matches("nodeType=\"clickEffect\"").count(), 7); + // The first step in each interactive sequence begins at zero; subsequent steps wait. + assert_eq!(xml.matches("delay=\"indefinite\"").count(), 5); +} + +#[test] +fn triggered_paragraph_build_keeps_a_click_for_each_paragraph() { + let mut p = deck(); + let ids: Vec<_> = p.slides[0].shapes.iter().map(|s| s.id).collect(); + Arc::make_mut(&mut p.slides[0]).animations = + vec![Animation { shape: ids[2], trigger: Some(ids[0]), text_build: TextBuild::ByParagraph, start: AnimStart::OnClick, ..Default::default() }]; + for q in [p.clone(), round(&p)] { + let xml = slide_xml(&q); + assert_eq!(xml.matches("nodeType=\"clickEffect\"").count(), 3); + assert_eq!(xml.matches("delay=\"indefinite\"").count(), 2); + let back = round(&q); + assert_eq!(back.slides[0].animations.len(), 1); + assert_eq!(back.slides[0].animations[0].text_build, TextBuild::ByParagraph); + assert_eq!(back.slides[0].animations[0].trigger, Some(ids[0])); + } +} diff --git a/docs/animation-parity.md b/docs/animation-parity.md index 7cd99abf..9e1fdeec 100644 --- a/docs/animation-parity.md +++ b/docs/animation-parity.md @@ -67,7 +67,7 @@ In, Wipe, Zoom, Float In, Grow/Shrink, Pulse, Spin, motion lines): weighted, ~85 | Effect Options (direction, sequence by paragraph) | yes | yes | | Effect dialog: sound, after-animation dim/hide, animate text by word/letter, smooth start/end, bounce end, repeat, rewind | yes | partly; repeat and bounce end round-trip; rewind lost on PPTX save (#77) | | By-paragraph text builds, sub-bullets follow parents | yes | yes | -| Triggers (on click of a shape) | yes | yes; played in the show: a step per click, the sequence starts over after its last step, a click completes a playing step | +| Triggers (on click of a shape) | yes | yes; played in the show: a step per click, the sequence starts over after its last step, a click completes a playing step. Separate click steps, mixed starts and paragraph builds survive PPTX save (`trigger_steps.rs`) | | Triggers on a media bookmark | yes | no | | Animation Painter | yes | yes (#54) | | Animation Pane: list, reorder, remove | yes | yes | @@ -94,6 +94,7 @@ In, Wipe, Zoom, Float In, Grow/Shrink, Pulse, Spin, motion lines): weighted, ~85 | Date | Change | Summary | |---|---|---| +| 2026-10-15 | trivial | PPTX interactive sequences preserve later On Click effects and one click per paragraph; no change to playback or feature scores | | 2026-10-14 | trivial | Community PR wave folded: the Animation Pane timeline row says yes (it landed), preview split into canvas (yes) and from the selected effect (no); estimates drop the timeline and transition sounds (~15–27 h) | | 2026-10-14 | trivial | Repeat and bounce end round-trip through PPTX; rewind is still lost (#77) | | 2026-10-13 | minor | Animations ▸ Preview and Animation Pane ▸ Play All play the current slide's animations on the editor canvas; preview from the selected effect is still open | diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index a5f72fe7..80f6a8fc 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -103,6 +103,7 @@ the round trip even though DeckCraft doesn't model it. | Date | Change | Summary | |---|---|---| +| 2026-10-15 | trivial | PPTX interactive timing sequences keep later On Click steps and paragraph click builds (`trigger_steps.rs`); mixed starts and independent buttons survive import/edit/save/reopen | | 2026-10-14 | minor | Community PR wave folded: one row each for `.pptx` (seven owned corpus files), outline, 3-D models, embedded fonts, text, effects, 3-D (extrusion sides draw now), SmartArt (written as a diagram) and VBA (tags and custom XML are kept) | | 2026-10-12 | minor | SmartArt import lays each shape's text out on its `dsp:txXfrm` text rectangle instead of the shape box | | 2026-10-13 | minor | EMF/WMF rectangle clip regions limit later drawing; EMF+ records remain | diff --git a/docs/gaps.md b/docs/gaps.md index 4e564951..755872e3 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -212,11 +212,10 @@ of the effect gallery. Detail in [animation-parity.md](animation-parity.md). 8 counters): a click on a shape whose action is "play sound" (`ppaction://media`, 126 and 486 shapes in two decks) does nothing; triggers that start on `onStopAudio` or `begin` conditions are not read as triggers; click boxes are the shape's bounding box (a hexagon's corners count) and do - not follow a shape that a motion path has moved; the PPTX writer turns every "on click" after the - first effect of a trigger into "after previous", so a trigger with several click steps saved from - DeckCraft plays all of them at once. -- **Impact:** games built around sound effects on buttons, or around a button that counts up with - repeated clicks, lose that part when they are saved from DeckCraft or played in it. + not follow a shape that a motion path has moved. Trigger sequences now keep their separate + click steps on PPTX save, including paragraph builds (`crates/pptx/tests/trigger_steps.rs`). +- **Impact:** games built around sound effects on buttons, audio-triggered effects or moving + clickable shapes do not play as authored. - **Estimate:** 4–8 h. ### Notes and Handout masters @@ -355,6 +354,7 @@ Detail in [localization-parity.md](localization-parity.md). | Date | Change | Summary | |---|---|---| +| 2026-10-15 | trivial | PPTX trigger sequences keep later On Click steps instead of converting them to After Previous; mixed starts, independent buttons and paragraph builds have synthetic round-trip regressions | | 2026-10-14 | trivial | Fixed #718: remote MCP no longer accepts unrelated or buffered stale success; loopback regressions cover invalid envelopes, incomplete and oversized frames | | 2026-10-14 | minor | Community PR wave folded: one entry each for the soak tests (engine and UI), the PPTX corpus (seven owned corpus files), edit points / text warp / 3-D, proofing (Hunspell spelling, thesaurus, AutoCorrect), the smaller missing rows, dialogs (41 vs ~60; Print and Insert Object left) and pen/trackpad input; ranked rows 3, 4 and 10 de-duplicated (comment threads #70 and the pane timeline are done); the fixed command-palette entry (#79) removed | | 2026-10-13 | minor | EMF/WMF rectangle clip regions (`INTERSECTCLIPRECT`, `EXCLUDECLIPRECT`, `EXTSELECTCLIPRGN`) limit later drawing. EMF+ records remain | diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 94928d8c..7f129fea 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -206,7 +206,7 @@ Open P0 items: Contextual tabs (Shape Format, Picture Format, Table Design/Layou | Animations | Effect options, start (on click/with/after), duration, delay | P0 | D | | Animations | Animation pane (reorder, timing) | P0 | P | | Animations | By-paragraph text builds | P1 | D | -| Animations | Triggers | P2 | D | +| Animations | Triggers (separate click steps and paragraph builds round-trip; `crates/pptx/tests/trigger_steps.rs`) | P2 | D | | Animations | Animation Painter | P2 | D | | Animations | Preview in editor | P1 | D | | Slide show | Full-screen show from start / current | P0 | D | diff --git a/docs/roadmap.md b/docs/roadmap.md index caf868de..f6115ebf 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -115,6 +115,7 @@ plan above. | Date | Change | Summary | |---|---|---| +| 2026-10-15 | trivial | PPTX trigger click steps survive save, including by-paragraph builds. Interactive-deck gaps remain media actions, audio triggers and click-box geometry; milestone scope and feature scores unchanged | | 2026-10-14 | trivial | M13 reliability: remote MCP validates IDs and envelopes, caps replies and rejects incomplete frames; #718 | | 2026-10-14 | minor | Community PR wave folded: one Review gate row, one Current focus item 2, one beta row each for items 9 and 10 (timeline and timer controls are in), one after-beta paragraph, one M9 and M10 row, one corpus row in the 2026-10-07 history | | 2026-10-13 | trivial | Beta item 3: embedded fonts are in, except MicroType Express subsets |