M4: a link on text is underlined by default, and Underline takes it off (#1841)

* M4: a link on text is underlined by default, and Underline takes it off

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* text.link: only a new link takes the default underline; Underline reads the texts inside a target

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

crates/engine/src/cmd/text.rs

@@ -101,7 +101,9 @@ fn all_have(s: &Session, p: &Value, pred: impl Fn(&TextStyle) -> bool) -> bool {
         }
         return false;
     }
-    targets(s, p).iter().all(|i| matches!(doc.node(*i).map(|n| &n.kind), Some(NodeKind::Text(t)) if t.spans().iter().all(|(_, _, st)| pred(st))))
+    // The texts `style` restyles: the targets and the texts inside them.
+    let texts: Vec<&TextData> = targets(s, p).iter().flat_map(|i| doc.subtree(*i)).filter_map(|d| doc.node(d).and_then(|n| n.kind.text())).collect();
+    !texts.is_empty() && texts.iter().all(|t| t.spans().iter().all(|(_, _, st)| pred(st)))
 }
 
 fn toggle_bold(s: &mut Session, p: &Value) -> Result<Value> {
@@ -217,10 +219,24 @@ pub fn specs() -> Vec<CommandSpec> {
             let on = all_have(s, p, |st| st.decoration == TextDecoration::Strikethrough);
             style(s, p, |st| st.decoration = if on { TextDecoration::None } else { TextDecoration::Strikethrough })
         }),
-        spec!("text.link", "Create link", "{url, ids?}", true, |s, p| {
-            let url = req_string("text.link", p, "url")?;
-            style(s, p, |st| st.link = Some(url.clone()))
-        }),
+        spec!(
+            "text.link",
+            "Create link",
+            "{url, ids?}  linked text is underlined unless it has a decoration of its own (Underline takes it off again)",
+            true,
+            |s, p| {
+                let url = req_string("text.link", p, "url")?;
+                style(s, p, |st| {
+                    // "Links are styled with an underline by default" (Add links to text); Underline
+                    // (Cmd/Ctrl+U) removes it, as documented there, and changing the URL of a link
+                    // leaves its decoration as it is.
+                    if st.link.is_none() && st.decoration == TextDecoration::None {
+                        st.decoration = TextDecoration::Underline;
+                    }
+                    st.link = Some(url.clone());
+                })
+            }
+        ),
         spec!("text.bulletList", "Bulleted list", "{ids?}  while editing: the selected paragraphs", true, |s, p| set_list(
             s,
             p,

crates/engine/src/text_behaviour_tests.rs

@@ -550,3 +550,37 @@ fn css_font_family_escapes_backslash_and_newline() {
     let css = crate::codegen::css_decls(s.doc().unwrap(), s.doc().unwrap().node(t).unwrap(), &mut Default::default(), (true, true));
     assert!(css.contains(r#"font-family: "Foo\"\3C /style>";"#), "{css}");
 }
+
+/// "Links are styled with an underline by default. Use the keyboard shortcut to remove the
+/// underline: Mac: Command U, Windows: Control U" (Add links to text).
+#[test]
+fn a_link_is_underlined_by_default_and_underline_takes_it_off() {
+    use figcraft_doc::TextDecoration;
+    let mut s = Session::new();
+    let t = text(&mut s, "docs", json!({}));
+    let style = |s: &Session| s.doc().unwrap().node(t).unwrap().kind.text().unwrap().style.clone();
+    s.execute("text.link", &json!({"ids": [t], "url": "https://example.com"})).unwrap();
+    assert_eq!((style(&s).link.as_deref(), style(&s).decoration), (Some("https://example.com"), TextDecoration::Underline));
+    s.execute("text.underline", &json!({"ids": [t]})).unwrap();
+    assert_eq!(
+        (style(&s).link.as_deref(), style(&s).decoration),
+        (Some("https://example.com"), TextDecoration::None),
+        "the link stays, the underline goes"
+    );
+    // Changing the URL of a link whose underline was taken off keeps it off.
+    s.execute("text.link", &json!({"ids": [t], "url": "https://example.org"})).unwrap();
+    assert_eq!((style(&s).link.as_deref(), style(&s).decoration), (Some("https://example.org"), TextDecoration::None));
+    // A decoration of the text's own is kept.
+    let u = text(&mut s, "struck", json!({"textDecoration": "strikethrough"}));
+    s.execute("text.link", &json!({"ids": [u], "url": "https://example.com"})).unwrap();
+    assert_eq!(s.doc().unwrap().node(u).unwrap().kind.text().unwrap().style.decoration, TextDecoration::Strikethrough);
+    // On a frame, the link reaches the text inside, and Underline reads that text: it takes the
+    // underline off rather than putting it on again.
+    let f = id(s.execute("frame.create", &json!({"x": 0, "y": 100, "width": 200, "height": 100})).unwrap());
+    let inner = id(s.execute("text.create", &json!({"text": "inside", "x": 0, "y": 100, "parent": f})).unwrap());
+    let inner_style = |s: &Session| s.doc().unwrap().node(inner).unwrap().kind.text().unwrap().style.clone();
+    s.execute("text.link", &json!({"ids": [f], "url": "https://example.com"})).unwrap();
+    assert_eq!((inner_style(&s).link.as_deref(), inner_style(&s).decoration), (Some("https://example.com"), TextDecoration::Underline));
+    s.execute("text.underline", &json!({"ids": [f]})).unwrap();
+    assert_eq!(inner_style(&s).decoration, TextDecoration::None, "Underline on the frame reads the text inside it");
+}