mirror of
https://github.com/storytold/deckcraft.git
synced 2026-11-09 11:59:36 -06:00
Keep Source Formatting resolves colours as the renderer does
Review follow-up on #1658 (CodeRabbit), both from storing the slide's
mapped scheme in 30ccc38e:
- Copying in Slide Master view used the current slide's colour map. Use
`ctx_for`, so the theme is that of what is being edited.
- `ColorScheme` shares an entry between `dk1` and `tx1`, so a mapped scheme
changed direct `dk1` colours too: a dark fill pasted white. `SourceTheme`
keeps the theme's colours and the map apart, and `bake_theme` maps only
`tx1`, `bg1`, `tx2` and `bg2`, as `Ctx::color` does.
The browser image test now checks that non-picture pastes keep their text.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
46a15a4684
commit
070f12cc9c
@@ -127,11 +127,12 @@ fn copy_inner(s: &mut Session, p: &Value) -> Result<String> {
|
||||
let mids = crate::media_refs::shapes_media(&shapes);
|
||||
let media = st.doc.media.iter().filter(|m| mids.contains(&m.id)).cloned().collect();
|
||||
let plain = shapes.iter().filter_map(|x| x.text.as_ref().map(|t| t.text())).collect::<Vec<_>>().join("\n");
|
||||
// The scheme the slide draws with: its or its layout's colour map, else the master's.
|
||||
let source_theme = st
|
||||
.current_slide()
|
||||
.and_then(|sl| deckcraft_model::resolve::Ctx::for_slide(&st.doc, sl))
|
||||
.map(|ctx| (ctx.scheme.clone(), ctx.master.theme.fonts.clone()));
|
||||
// The theme of what is being edited: a slide's or layout's colour map, else the master's.
|
||||
let source_theme = super::text::ctx_for(&st.doc, &st.selection).map(|ctx| crate::SourceTheme {
|
||||
colors: ctx.master.theme.colors.clone(),
|
||||
color_map: ctx.color_map.clone(),
|
||||
fonts: ctx.master.theme.fonts.clone(),
|
||||
});
|
||||
s.clipboard = crate::Clipboard { shapes, placeholders, media, plain: plain.clone(), source_theme, ..Default::default() };
|
||||
Ok(plain)
|
||||
}
|
||||
@@ -430,9 +431,9 @@ fn paste_special(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
"text" | "keepTextOnly" => paste_text(s, &json!({})),
|
||||
"keepSource" | "keepSourceFormatting" => {
|
||||
let saved = s.clipboard.clone();
|
||||
if let Some((scheme, fonts)) = &saved.source_theme {
|
||||
bake_theme(&mut s.clipboard.shapes, scheme, fonts);
|
||||
bake_theme(&mut s.clipboard.placeholders, scheme, fonts);
|
||||
if let Some(theme) = &saved.source_theme {
|
||||
bake_theme(&mut s.clipboard.shapes, theme);
|
||||
bake_theme(&mut s.clipboard.placeholders, theme);
|
||||
}
|
||||
let r = paste(s, &json!({}));
|
||||
let pastes = s.clipboard.pastes;
|
||||
@@ -447,8 +448,8 @@ fn paste_special(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
|
||||
/// Replace theme colours and theme fonts with the values they have in `scheme` / `fonts`, so the
|
||||
/// shapes keep their look under another theme.
|
||||
pub(crate) fn bake_theme(shapes: &mut [Shape], scheme: &deckcraft_color::ColorScheme, fonts: &deckcraft_model::theme::FontScheme) {
|
||||
fn rec(v: &mut Value, scheme: &deckcraft_color::ColorScheme, fonts: &deckcraft_model::theme::FontScheme, depth: usize) {
|
||||
pub(crate) fn bake_theme(shapes: &mut [Shape], theme: &crate::SourceTheme) {
|
||||
fn rec(v: &mut Value, theme: &crate::SourceTheme, depth: usize) {
|
||||
if depth > 256 {
|
||||
return;
|
||||
}
|
||||
@@ -457,7 +458,7 @@ pub(crate) fn bake_theme(shapes: &mut [Shape], scheme: &deckcraft_color::ColorSc
|
||||
if o.get("kind").and_then(Value::as_str) == Some("scheme")
|
||||
&& let Some(slot) = o.get("slot").and_then(|x| serde_json::from_value::<deckcraft_color::SchemeSlot>(x.clone()).ok())
|
||||
&& slot != deckcraft_color::SchemeSlot::PhClr
|
||||
&& let Ok(rgb) = serde_json::to_value(scheme.get(slot))
|
||||
&& let Ok(rgb) = serde_json::to_value(theme.colors.get(deckcraft_model::map_slot(&theme.color_map, slot)))
|
||||
{
|
||||
o.clear();
|
||||
o.insert("kind".into(), json!("rgb"));
|
||||
@@ -465,14 +466,14 @@ pub(crate) fn bake_theme(shapes: &mut [Shape], scheme: &deckcraft_color::ColorSc
|
||||
return;
|
||||
}
|
||||
for x in o.values_mut() {
|
||||
rec(x, scheme, fonts, depth + 1);
|
||||
rec(x, theme, depth + 1);
|
||||
}
|
||||
}
|
||||
Value::Array(a) => a.iter_mut().for_each(|x| rec(x, scheme, fonts, depth + 1)),
|
||||
Value::Array(a) => a.iter_mut().for_each(|x| rec(x, theme, depth + 1)),
|
||||
Value::String(t) if t.starts_with('+') => {
|
||||
let set = match t.get(1..3) {
|
||||
Some("mj") => &fonts.major,
|
||||
Some("mn") => &fonts.minor,
|
||||
Some("mj") => &theme.fonts.major,
|
||||
Some("mn") => &theme.fonts.minor,
|
||||
_ => return,
|
||||
};
|
||||
let face = match t.get(3..) {
|
||||
@@ -490,7 +491,7 @@ pub(crate) fn bake_theme(shapes: &mut [Shape], scheme: &deckcraft_color::ColorSc
|
||||
}
|
||||
for sh in shapes.iter_mut() {
|
||||
let Ok(mut v) = serde_json::to_value(&*sh) else { continue };
|
||||
rec(&mut v, scheme, fonts, 0);
|
||||
rec(&mut v, theme, 0);
|
||||
if let Ok(baked) = serde_json::from_value::<Shape>(v) {
|
||||
*sh = baked;
|
||||
}
|
||||
@@ -503,8 +504,8 @@ fn paste_picture(s: &mut Session) -> Result<Value> {
|
||||
if clip.shapes.is_empty() {
|
||||
return Err(bad("edit.pasteSpecial", "copy shapes first to paste them as a picture"));
|
||||
}
|
||||
if let Some((scheme, fonts)) = &clip.source_theme {
|
||||
bake_theme(&mut clip.shapes, scheme, fonts);
|
||||
if let Some(theme) = &clip.source_theme {
|
||||
bake_theme(&mut clip.shapes, theme);
|
||||
}
|
||||
let st = s.doc()?;
|
||||
let mut doc = (*st.doc).clone();
|
||||
|
||||
@@ -454,6 +454,17 @@ pub struct MediaStatus {
|
||||
}
|
||||
|
||||
/// Clipboard contents copied within the app.
|
||||
/// The theme a copy drew with. The colours and the map stay apart, as in
|
||||
/// `deckcraft_model::resolve::Ctx::color`: the map renames only `tx1`, `bg1`, `tx2` and `bg2`, and
|
||||
/// a direct `dk1` keeps the theme's colour.
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct SourceTheme {
|
||||
/// The theme's own colours, no colour map applied.
|
||||
pub colors: deckcraft_color::ColorScheme,
|
||||
pub color_map: Vec<(String, String)>,
|
||||
pub fonts: deckcraft_model::theme::FontScheme,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Default)]
|
||||
pub struct Clipboard {
|
||||
pub shapes: Vec<Shape>,
|
||||
@@ -465,9 +476,8 @@ pub struct Clipboard {
|
||||
pub media: Vec<deckcraft_model::MediaItem>,
|
||||
pub text: Option<deckcraft_model::TextBody>,
|
||||
pub plain: String,
|
||||
/// The copied shapes' theme colours (colour map applied) and fonts, for Paste Special ›
|
||||
/// Keep Source Formatting.
|
||||
pub source_theme: Option<(deckcraft_color::ColorScheme, deckcraft_model::theme::FontScheme)>,
|
||||
/// The theme the shapes were copied under, for Paste Special › Keep Source Formatting.
|
||||
pub source_theme: Option<SourceTheme>,
|
||||
/// How many times the shapes were pasted (each paste offsets further).
|
||||
pub pastes: u32,
|
||||
}
|
||||
|
||||
@@ -307,7 +307,6 @@ fn keep_source_formatting_uses_the_slides_colour_map() {
|
||||
let p = &sh.text.as_ref().unwrap().paragraphs[0];
|
||||
resolve::text_color(&ctx, &resolve::run(&ctx, sh, p, &p.runs[0].props))
|
||||
};
|
||||
let swapped = || Some([("bg1", "dk1"), ("tx1", "lt1"), ("bg2", "dk2"), ("tx2", "lt2")].map(|(a, b)| (a.to_string(), b.to_string())).to_vec());
|
||||
for on_layout in [false, true] {
|
||||
let mut s = session();
|
||||
s.execute("slide.new", &json!({})).unwrap();
|
||||
@@ -320,9 +319,9 @@ fn keep_source_formatting_uses_the_slides_colour_map() {
|
||||
if on_layout {
|
||||
let layout = doc.slides[at].layout;
|
||||
let m = std::sync::Arc::make_mut(&mut doc.masters[0]);
|
||||
m.layouts.iter_mut().find(|l| l.id == layout).unwrap().color_map = swapped();
|
||||
m.layouts.iter_mut().find(|l| l.id == layout).unwrap().color_map = swapped_colour_map();
|
||||
} else {
|
||||
std::sync::Arc::make_mut(&mut doc.slides[at]).color_map = swapped();
|
||||
std::sync::Arc::make_mut(&mut doc.slides[at]).color_map = swapped_colour_map();
|
||||
}
|
||||
let want = first_run_colour(&s, title);
|
||||
assert_ne!(want, plain, "the override changes how the title draws");
|
||||
@@ -336,6 +335,85 @@ fn keep_source_formatting_uses_the_slides_colour_map() {
|
||||
}
|
||||
}
|
||||
|
||||
/// A colour map that swaps text and background, as a dark slide or layout has.
|
||||
fn swapped_colour_map() -> Option<Vec<(String, String)>> {
|
||||
Some([("bg1", "dk1"), ("tx1", "lt1"), ("bg2", "dk2"), ("tx2", "lt2")].map(|(a, b)| (a.to_string(), b.to_string())).to_vec())
|
||||
}
|
||||
|
||||
/// A new rectangle in the edited tree, filled with theme colour `slot`.
|
||||
fn rect_filled_with(s: &mut Session, slot: deckcraft_color::SchemeSlot) -> ShapeId {
|
||||
use deckcraft_model::{ColorRef, Fill};
|
||||
let id = s.execute("shape.insert", &json!({"preset": "rect", "rect": [10, 10, 50, 50]})).unwrap()["id"].as_u64().unwrap();
|
||||
let id = ShapeId(u32::try_from(id).unwrap());
|
||||
s.edit(|doc, sel| {
|
||||
let list = crate::shapes_mut(doc, sel).unwrap();
|
||||
list.iter_mut().find(|x| x.id == id).unwrap().fill = Some(Fill::solid(ColorRef::scheme(slot)));
|
||||
Ok(())
|
||||
})
|
||||
.unwrap();
|
||||
id
|
||||
}
|
||||
|
||||
/// How shape `id`'s fill draws in the edited tree.
|
||||
fn drawn_fill(s: &Session, id: ShapeId) -> deckcraft_color::Rgba {
|
||||
use deckcraft_model::{Fill, resolve};
|
||||
let st = s.doc().unwrap();
|
||||
let ctx = crate::cmd::text::ctx_for(&st.doc, &st.selection).unwrap();
|
||||
match resolve::fill(&ctx, st.shape(id).unwrap()).0 {
|
||||
Some(Fill::Solid { color }) => ctx.color(&color, None),
|
||||
other => panic!("not a solid fill: {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
/// Copy the selected shape and paste it with Keep Source Formatting; the copy's id.
|
||||
fn keep_source_copy(s: &mut Session, id: ShapeId) -> ShapeId {
|
||||
s.execute("selection.select", &json!({"id": id.0})).unwrap();
|
||||
s.execute("edit.copy", &json!({})).unwrap();
|
||||
s.execute("edit.pasteSpecial", &json!({"as": "keepSource"})).unwrap();
|
||||
let copy = s.doc().unwrap().selection.shapes[0];
|
||||
assert_ne!(copy, id);
|
||||
copy
|
||||
}
|
||||
|
||||
/// A colour map renames only `tx1`, `bg1`, `tx2` and `bg2`. Keep Source Formatting fixes every slot
|
||||
/// as it draws: `tx1` follows the map, while a direct `dk1` stays the theme's dark colour.
|
||||
#[test]
|
||||
fn keep_source_formatting_maps_only_the_alias_colours() {
|
||||
use deckcraft_color::SchemeSlot::{self, *};
|
||||
let mut s = session();
|
||||
let st = s.doc_mut().unwrap();
|
||||
let at = st.selection.slide;
|
||||
std::sync::Arc::make_mut(&mut std::sync::Arc::make_mut(&mut st.doc).slides[at]).color_map = swapped_colour_map();
|
||||
let slots: [SchemeSlot; 9] = [Dk1, Lt1, Dk2, Lt2, Tx1, Bg1, Tx2, Bg2, Accent1];
|
||||
let mut drawn = vec![];
|
||||
for slot in slots {
|
||||
let id = rect_filled_with(&mut s, slot);
|
||||
let want = drawn_fill(&s, id);
|
||||
let copy = keep_source_copy(&mut s, id);
|
||||
assert_eq!(drawn_fill(&s, copy), want, "{slot:?}");
|
||||
drawn.push(want);
|
||||
}
|
||||
assert_ne!(drawn[0], drawn[4], "the map sets `tx1` apart from `dk1`");
|
||||
}
|
||||
|
||||
/// In Slide Master view a copy draws through the edited master's map, not an override on the slide
|
||||
/// that happens to be current.
|
||||
#[test]
|
||||
fn keep_source_formatting_in_slide_master_view_ignores_the_slides_colour_map() {
|
||||
use deckcraft_color::SchemeSlot::Tx1;
|
||||
for master in [false, true] {
|
||||
let mut s = session();
|
||||
let st = s.doc_mut().unwrap();
|
||||
let at = st.selection.slide;
|
||||
std::sync::Arc::make_mut(&mut std::sync::Arc::make_mut(&mut st.doc).slides[at]).color_map = swapped_colour_map();
|
||||
s.execute("view.slideMaster", &if master { json!({"master": 0}) } else { json!({}) }).unwrap();
|
||||
let id = rect_filled_with(&mut s, Tx1);
|
||||
let want = drawn_fill(&s, id);
|
||||
let copy = keep_source_copy(&mut s, id);
|
||||
assert_eq!(drawn_fill(&s, copy), want, "editing the master: {master}");
|
||||
}
|
||||
}
|
||||
|
||||
/// A slide's placeholder pasted in Slide Master view stays a placeholder that inherits from where
|
||||
/// it lands; only a paste on a slide turns it into a regular shape with the look it had.
|
||||
#[test]
|
||||
|
||||
@@ -419,6 +419,10 @@ fn ctrl_v_pastes_the_picture_a_browser_copied_not_its_link() {
|
||||
let pictures = shapes.iter().filter(|s| matches!(s.kind, deckcraft_model::ShapeKind::Picture { .. })).count();
|
||||
assert_eq!(pictures, pictures_after, "{text:?}");
|
||||
assert_eq!(shapes.len(), shapes_before + 1, "one shape, picture or text, for {text:?}");
|
||||
if pictures_after == 0 {
|
||||
let pasted = shapes.last().and_then(|s| s.text.as_ref()).map(deckcraft_model::TextBody::text);
|
||||
assert_eq!(pasted.as_deref(), Some(text.as_str()), "the text survives");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user