From 7388c42d519f01fc4ff49731cd7e3e822f4df9c6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 17:48:02 +0000 Subject: [PATCH] Running headers: place each story by its first hit on the page Review fix (CodeRabbit): a story was placed on the page by its highest hit, so a heading at the top of its later column put the whole story ahead of another story whose heading sits higher than the first one's. Each story is now placed by its first hit there in text order. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CgGsJkVe5GYG9qK6VFdECa --- crates/compose/src/vars.rs | 8 ++++--- crates/engine/src/cmd/variables.rs | 37 ++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/crates/compose/src/vars.rs b/crates/compose/src/vars.rs index b633d7a8f..508ac4a3e 100644 --- a/crates/compose/src/vars.rs +++ b/crates/compose/src/vars.rs @@ -66,13 +66,15 @@ struct Hit { /// second column comes after its first, however high its entries sit), and stories by where their /// first entry on the page is (top to bottom, then left to right). fn in_page_order(hits: Vec) -> Vec<(usize, String)> { - let mut first: HashMap<(usize, StoryId), (i64, i64)> = HashMap::new(); + // Each story on a page is placed by its first hit there in text order (not its highest hit, + // which in a later column could put the whole story ahead of another). + let mut first: HashMap<(usize, StoryId), (usize, (i64, i64))> = HashMap::new(); for h in &hits { let (page, y, x) = h.at; - first.entry((page, h.story)).and_modify(|f| *f = (*f).min((y, x))).or_insert((y, x)); + first.entry((page, h.story)).and_modify(|f| *f = (*f).min((h.pos, (y, x)))).or_insert((h.pos, (y, x))); } let mut hits: Vec<_> = - hits.into_iter().map(|h| (h.at.0, first.get(&(h.at.0, h.story)).copied().unwrap_or((h.at.1, h.at.2)), h.story, h.pos, h.text)).collect(); + hits.into_iter().map(|h| (h.at.0, first.get(&(h.at.0, h.story)).map_or((h.at.1, h.at.2), |f| f.1), h.story, h.pos, h.text)).collect(); hits.sort_by_key(|a| (a.0, a.1, a.2.0, a.3)); hits.into_iter().map(|(page, _, _, _, text)| (page, text)).collect() } diff --git a/crates/engine/src/cmd/variables.rs b/crates/engine/src/cmd/variables.rs index 9c5dd20c8..66cee5fb6 100644 --- a/crates/engine/src/cmd/variables.rs +++ b/crates/engine/src/cmd/variables.rs @@ -386,4 +386,41 @@ mod tests { assert_eq!(idx.value("Heading", false, designcraft_doc::vars::Use::FirstOnPage, 0), "Alpha"); assert_eq!(idx.value("Heading", false, designcraft_doc::vars::Use::LastOnPage, 0), "Gamma"); } + /// Stories on one page are ordered by where each one's text starts there: a story whose first + /// heading sits low in its first column comes after another story's heading above it, even + /// though its later column starts higher still. + #[test] + fn running_header_orders_stories_by_their_first_heading() { + use designcraft_doc::story::COLUMN_BREAK; + let mut s = Session::new(); + s.execute("file.new", &json!({"pages": 1, "facingPages": false})).unwrap(); + s.execute("style.paragraph.create", &json!({"name": "Heading"})).ok(); + // Story A, two columns: filler, then Alpha low in column 1; Gamma at the top of column 2. + let a_text = format!("{}Alpha{COLUMN_BREAK}\nGamma\ny", "x\n".repeat(8)); + let a = s.execute("frame.create", &json!({"rect": [72, 72, 300, 400], "content": "text", "text": a_text})).unwrap(); + s.execute("object.textFrameOptions", &json!({"ids": [a["id"]], "columns": 2, "gutter": 12})).unwrap(); + // Story B beside it: Beta between Gamma's height and Alpha's. + let b_text = "Beta".to_string(); + let b = s.execute("frame.create", &json!({"rect": [320, 120, 560, 160], "content": "text", "text": b_text})).unwrap(); + let story = + |s: &Session, id: &Value| s.doc().unwrap().doc.item(designcraft_doc::ItemId(id.as_u64().unwrap())).unwrap().text_frame().unwrap().story; + let (sa, sb) = (story(&s, &a["id"]), story(&s, &b["id"])); + for (sid, text, word) in [(sa, &a_text, "Alpha"), (sa, &a_text, "Gamma"), (sb, &b_text, "Beta")] { + let at = text.find(word).unwrap(); + s.execute("text.select", &json!({"story": sid.0, "anchor": at, "focus": at + word.len()})).unwrap(); + s.execute("style.paragraph.apply", &json!({"name": "Heading"})).unwrap(); + } + s.execute("variables.define", &json!({"name": "Entry", "type": "runningHeader", "style": "Heading"})).unwrap(); + let d = s.doc().unwrap().doc.clone(); + let y = |sid, text: &str, word: &str| { + let cs = s.cache.get(&d, sid, None); + let (fi, _, b, ..) = crate::compose::caret(&cs, text.find(word).unwrap()).unwrap(); + d.item(cs.frames[fi].frame).unwrap().bounds().y0 + b + }; + let (ya, yg, yb) = (y(sa, &a_text, "Alpha"), y(sa, &a_text, "Gamma"), y(sb, &b_text, "Beta")); + assert!(yg < yb && yb < ya, "Gamma {yg} above Beta {yb} above Alpha {ya}"); + let idx = s.cache.running_index(&d); + assert_eq!(idx.value("Heading", false, designcraft_doc::vars::Use::FirstOnPage, 0), "Beta"); + assert_eq!(idx.value("Heading", false, designcraft_doc::vars::Use::LastOnPage, 0), "Gamma"); + } }