mirror of
https://github.com/storytold/deckcraft.git
synced 2026-10-24 10:37:03 -06:00
Fix up #169: auto RTL lives only in the engine and right-aligns only unaligned text
Drop the layout-time direction detection in crates/text (and its test); the engine's promote_rtl_if_needed sets align Right only when neither the paragraph nor what it inherits (list styles, layout, master) resolves an alignment. right_to_left_paragraphs types its original Arabic text again, after an explicit LTR so the toggle it tests starts from left-to-right. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
bd005c4626
commit
4b6f307ebb
@@ -305,6 +305,7 @@ fn typed(s: &Session, body: &TextBody, at: Pos, text: &str) -> String {
|
||||
}
|
||||
|
||||
pub(crate) fn insert_text(s: &mut Session, text: &str, props: Option<RunProps>) -> Result<Value> {
|
||||
let inherited = inherited_aligns_for_edit(s);
|
||||
let text = {
|
||||
let st = s.doc()?;
|
||||
match &st.selection.text {
|
||||
@@ -324,7 +325,7 @@ pub(crate) fn insert_text(s: &mut Session, text: &str, props: Option<RunProps>)
|
||||
t.caret = c;
|
||||
for pi in at.0..=c.0 {
|
||||
if let Some(para) = body.paragraphs.get_mut(pi) {
|
||||
promote_rtl_if_needed(para);
|
||||
promote_rtl_if_needed(para, &inherited);
|
||||
}
|
||||
}
|
||||
c
|
||||
@@ -333,10 +334,37 @@ pub(crate) fn insert_text(s: &mut Session, text: &str, props: Option<RunProps>)
|
||||
Ok(json!({"caret": [pos.0, pos.1]}))
|
||||
}
|
||||
|
||||
fn promote_rtl_if_needed(para: &mut deckcraft_model::text::Paragraph) {
|
||||
/// Per outline level (0..=8), whether shape `id` on the current slide already resolves an
|
||||
/// alignment for its paragraphs (its own list style, the layout's or the master's). All `false`
|
||||
/// when the shape can't be resolved (notes, a shape that is gone).
|
||||
fn inherited_aligns(s: &Session, id: ShapeId) -> [bool; 9] {
|
||||
let mut out = [false; 9];
|
||||
let Ok(st) = s.doc() else { return out };
|
||||
let Some(sh) = st.shape(id) else { return out };
|
||||
let Some(ctx) = ctx_for(&st.doc, &st.selection) else { return out };
|
||||
for (lvl, set) in out.iter_mut().enumerate() {
|
||||
let para = deckcraft_model::text::Paragraph { level: lvl as u8, ..Default::default() };
|
||||
*set = resolve::para(&ctx, sh, ¶).align.is_some();
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
/// The aligns [`inherited_aligns`] gives for the text being edited (none for notes).
|
||||
fn inherited_aligns_for_edit(s: &Session) -> [bool; 9] {
|
||||
match s.doc().ok().and_then(|st| st.selection.text.clone()) {
|
||||
Some(t) if !t.notes => inherited_aligns(s, t.shape),
|
||||
_ => [false; 9],
|
||||
}
|
||||
}
|
||||
|
||||
/// A paragraph with no direction of its own whose first strong character is right-to-left
|
||||
/// (Arabic, Hebrew, Kurdish…) becomes right-to-left, and right-aligned when nothing (the
|
||||
/// paragraph or what it inherits, `inherited`) sets its alignment.
|
||||
fn promote_rtl_if_needed(para: &mut deckcraft_model::text::Paragraph, inherited: &[bool; 9]) {
|
||||
if para.props.rtl.is_none() && deckcraft_model::bidi::first_strong_is_rtl(¶.text()) {
|
||||
para.props.rtl = Some(true);
|
||||
if para.props.align.is_none() {
|
||||
let inherits_align = inherited.get(usize::from(para.level)).copied().unwrap_or(false);
|
||||
if para.props.align.is_none() && !inherits_align {
|
||||
para.props.align = Some(Align::Right);
|
||||
}
|
||||
}
|
||||
@@ -612,6 +640,7 @@ fn autocorrect_options(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
}
|
||||
|
||||
pub(crate) fn paste_body(s: &mut Session, src: &TextBody) -> Result<Value> {
|
||||
let inherited = inherited_aligns_for_edit(s);
|
||||
let pos = edit_text(s, |body, t| {
|
||||
let (a, b) = t.ordered();
|
||||
let at = ed::delete(body, a, b);
|
||||
@@ -620,7 +649,7 @@ pub(crate) fn paste_body(s: &mut Session, src: &TextBody) -> Result<Value> {
|
||||
t.caret = c;
|
||||
for pi in at.0..=c.0 {
|
||||
if let Some(para) = body.paragraphs.get_mut(pi) {
|
||||
promote_rtl_if_needed(para);
|
||||
promote_rtl_if_needed(para, &inherited);
|
||||
}
|
||||
}
|
||||
c
|
||||
@@ -1056,6 +1085,7 @@ fn set(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
None => s.doc()?.selection.shapes.first().copied().ok_or_else(|| bad("text.set", "missing `id` (or select a shape)"))?,
|
||||
};
|
||||
let cell = p.get("cell").and_then(Value::as_array).and_then(|a| Some((a.first()?.as_u64()? as usize, a.get(1)?.as_u64()? as usize)));
|
||||
let inherited = inherited_aligns(s, id);
|
||||
edit_shapes(s, &json!({"id": id}), "text.set", |sh| {
|
||||
let target = match (&mut sh.kind, cell) {
|
||||
(ShapeKind::Table(t), Some((r, c))) => t.cell_mut(r, c).map(|x| &mut x.text),
|
||||
@@ -1074,7 +1104,7 @@ fn set(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
para.level = *lv;
|
||||
para.props = pp.clone();
|
||||
}
|
||||
promote_rtl_if_needed(para);
|
||||
promote_rtl_if_needed(para, &inherited);
|
||||
for r in &mut para.runs {
|
||||
r.props = keep.clone();
|
||||
}
|
||||
@@ -1284,6 +1314,30 @@ fn replace_font(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
Ok(json!({"replaced": n}))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod rtl_promote_tests {
|
||||
use deckcraft_model::text::{Align, Paragraph, Run};
|
||||
|
||||
use super::promote_rtl_if_needed;
|
||||
|
||||
fn para(text: &str) -> Paragraph {
|
||||
Paragraph { runs: vec![Run::new(text)], ..Default::default() }
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn right_aligns_only_when_no_alignment_is_set_or_inherited() {
|
||||
let mut p = para("مرحبا");
|
||||
promote_rtl_if_needed(&mut p, &[false; 9]);
|
||||
assert_eq!((p.props.rtl, p.props.align), (Some(true), Some(Align::Right)));
|
||||
let mut p = para("مرحبا");
|
||||
promote_rtl_if_needed(&mut p, &[true; 9]);
|
||||
assert_eq!((p.props.rtl, p.props.align), (Some(true), None));
|
||||
let mut p = para("Hello مرحبا");
|
||||
promote_rtl_if_needed(&mut p, &[false; 9]);
|
||||
assert_eq!((p.props.rtl, p.props.align), (None, None));
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod replace_font_tests {
|
||||
use serde_json::json;
|
||||
|
||||
@@ -488,7 +488,9 @@ fn right_to_left_paragraphs() {
|
||||
let mut s = session();
|
||||
let title = s.doc().unwrap().current_slide().unwrap().shapes[0].id.0;
|
||||
s.execute("text.edit", &json!({"id": title})).unwrap();
|
||||
s.execute("text.insert", &json!({"text": "Year 2024 (Retrait) only"})).unwrap();
|
||||
// Explicitly left-to-right first, so typing Arabic doesn't already turn it right-to-left (#169).
|
||||
s.execute("format.rtl", &json!({"on": false})).unwrap();
|
||||
s.execute("text.insert", &json!({"text": "سنة 2024 (Retrait) فقط"})).unwrap();
|
||||
s.execute("format.rtl", &json!({})).unwrap();
|
||||
let para = |s: &Session| s.doc().unwrap().current_slide().unwrap().shapes[0].text.as_ref().unwrap().paragraphs[0].props.clone();
|
||||
assert_eq!(para(&s).rtl, Some(true));
|
||||
@@ -530,16 +532,26 @@ fn typing_kurdish_or_arabic_auto_detects_rtl_and_advances_caret_smoothly() {
|
||||
}
|
||||
last_x = cx;
|
||||
|
||||
// Caret must remain near the right margin (e.g. > 400.0 pt in title placeholder)
|
||||
// and must NOT jump back to the left margin (e.g. ~7.2 pt).
|
||||
assert!(cx > 400.0, "caret should not jump to left margin: {cx}");
|
||||
// The title is centred: the caret must NOT jump back to the left margin (~7.2 pt).
|
||||
assert!(cx > 100.0, "caret should not jump to left margin: {cx}");
|
||||
}
|
||||
|
||||
// Caret advances leftwards overall as text is appended
|
||||
assert!(last_x < first_x.unwrap(), "caret should advance leftwards: last={last_x} first={:?}", first_x);
|
||||
let para = s.doc().unwrap().current_slide().unwrap().shapes[0].text.as_ref().unwrap().paragraphs[0].props.clone();
|
||||
assert_eq!(para.rtl, Some(true), "auto-detected as RTL");
|
||||
assert_eq!(para.align, Some(deckcraft_model::text::Align::Right), "aligned right");
|
||||
assert_eq!(para.align, None, "the title keeps the alignment it inherits");
|
||||
|
||||
// A text box inherits the deck's default (left) alignment, which it keeps too.
|
||||
s.execute("text.exit", &json!({})).ok();
|
||||
let id = s.execute("insert.textBox", &json!({"rect": [10, 10, 300, 100], "text": ""})).unwrap()["id"].as_u64().unwrap();
|
||||
s.execute("text.edit", &json!({"id": id})).unwrap();
|
||||
s.execute("text.insert", &json!({"text": "مرحبا"})).unwrap();
|
||||
let st = s.doc().unwrap();
|
||||
let sh = st.shape(deckcraft_model::ShapeId(id as u32)).unwrap();
|
||||
let para = sh.text.as_ref().unwrap().paragraphs[0].props.clone();
|
||||
assert_eq!(para.rtl, Some(true));
|
||||
assert_eq!(para.align, None, "the text box keeps the alignment it inherits");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -712,11 +712,8 @@ fn shape_para(ctx: &Ctx, shape: &Shape, body: &TextBody, pi: usize, scale: f64,
|
||||
rtl: false,
|
||||
};
|
||||
};
|
||||
let mut props = resolve::para(ctx, shape, para);
|
||||
let rtl = props.rtl.unwrap_or_else(|| deckcraft_model::bidi::first_strong_is_rtl(¶.text()));
|
||||
if rtl && para.props.align.is_none() {
|
||||
props.align = Some(Align::Right);
|
||||
}
|
||||
let props = resolve::para(ctx, shape, para);
|
||||
let rtl = props.rtl.unwrap_or(false);
|
||||
let mut styles: Vec<Style> = vec![];
|
||||
let mut faces: Vec<Arc<FontFace>> = vec![];
|
||||
// The logical paragraph: its text and each character's style.
|
||||
|
||||
@@ -504,19 +504,6 @@ fn rtl_left_and_center_alignment() {
|
||||
assert!((c - (l.inner.x0 + l.inner.x1) / 2.0).abs() < 0.5);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn layout_auto_detects_rtl_from_first_strong_character() {
|
||||
let (p, mut sh) = bidi_setup(vec![Run::new("من ناوم هێڤاره")], 400.0, false, Align::Left);
|
||||
// Leave props.rtl as None and props.align as None (unassigned)
|
||||
sh.text.as_mut().unwrap().paragraphs[0].props.rtl = None;
|
||||
sh.text.as_mut().unwrap().paragraphs[0].props.align = None;
|
||||
let l = lay(&p, &sh, 400.0, 300.0);
|
||||
let line = &l.lines[0];
|
||||
assert!(line.rtl, "line should auto-detect as RTL");
|
||||
// Line aligns to the right edge
|
||||
assert!(line.extent.1 > 350.0, "extent right: {}", line.extent.1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bidi_caret_stops_round_trip_through_hit_testing() {
|
||||
for rtl in [true, false] {
|
||||
|
||||
Reference in New Issue
Block a user