diff --git a/crates/op-design-lint/src/detectors/typography.rs b/crates/op-design-lint/src/detectors/typography.rs index aec6921fb..e07d00c95 100644 --- a/crates/op-design-lint/src/detectors/typography.rs +++ b/crates/op-design-lint/src/detectors/typography.rs @@ -210,9 +210,6 @@ fn first_solid_color(fills: Option<&Vec>) -> Option { } } PenFill::Image(_) => continue, - // Mesh gradients / SkSL shaders have no single representative - // color for contrast checks — treated like image fills. - PenFill::MeshGradient(_) | PenFill::Shader(_) => continue, } } None diff --git a/crates/op-orchestrator/src/geometry_validation.rs b/crates/op-orchestrator/src/geometry_validation.rs index a5f8946d8..92fd51e0a 100644 --- a/crates/op-orchestrator/src/geometry_validation.rs +++ b/crates/op-orchestrator/src/geometry_validation.rs @@ -260,11 +260,11 @@ fn collect_text_overflow_fixes( // absolutely positioned, so "wider than the parent" is not an overflow to // repair (and `width: fill_container` means nothing there). let flex_parent = matches!(layout_str(v), Some("vertical" | "horizontal")); - if let Some(parent_w) = v + if let Some((parent_x, parent_w)) = v .get("id") .and_then(Value::as_str) .and_then(|id| rects.get(id)) - .map(|r| r.w) + .map(|r| (r.x, r.w)) .filter(|_| flex_parent) { for c in children(v) { @@ -286,7 +286,11 @@ fn collect_text_overflow_fixes( if fill && wrap { continue; } - if cr.w > parent_w + TEXT_OVERFLOW_EPS { + // Wider than the block, OR its right edge past the block's right + // edge (a sibling pushed it out — combined overflow the width-only + // check misses: a 36px avatar + a fit name inside a 116px row). + let past_right = cr.x + cr.w > parent_x + parent_w + TEXT_OVERFLOW_EPS; + if cr.w > parent_w + TEXT_OVERFLOW_EPS || past_right { cmds.push(EditorCommand::SetNodeLayoutProp { node_id: NodeId::new(cid.to_string()), property: "width".to_string(), @@ -385,7 +389,16 @@ fn collect_row_gap_fixes(v: &Value, rects: &HashMap, cmds: &mut Ve ) }) .collect(); - if frame_kids.len() >= 3 { + let distributes = matches!( + v.get("justifyContent").and_then(Value::as_str), + Some("space_between" | "space_around" | "space_evenly") + ); + // TWO text-bearing frame columns jammed at 0px (a date column against + // a details stack) are the two-column form of the same defect — but + // only when the row top-packs (a space_between pair separates itself, + // so a jammed pair there is the ancestor chain's problem, not gap's). + let enough_cells = frame_kids.len() >= 3 || (frame_kids.len() == 2 && !distributes); + if enough_cells { let rects_of: Vec> = frame_kids .iter() .map(|c| { @@ -563,6 +576,17 @@ fn bears_text(v: &Value) -> bool { children(v).iter().any(bears_text) } +/// A JAM participant must be a text-bearing CONTAINER cell. Two bare `text` +/// siblings set tight on purpose ("$29"+"/mo", value+unit pairs) are +/// typography, not a data-column jam — measured false positive on a pricing +/// card's price row. +fn is_cell_like(v: &Value) -> bool { + matches!( + v.get("type").and_then(Value::as_str), + Some("frame" | "group") + ) && bears_text(v) +} + /// Report adjacent siblings of a horizontal FLEX row that resolved jammed /// (text columns touching) or overlapping. Report-only: flush layouts are /// sometimes intentional (joined button groups), so the model — not a fixer — @@ -595,7 +619,16 @@ fn collect_sibling_jam_diagnostics( continue; } let breathing = rb.x - (ra.x + ra.w); - if breathing < -SIBLING_OVERLAP_EPS { + // Intentional OVERLAY, not an accident: one sibling's center inside + // the other's box — a number set on a ring (ellipse + short text), a + // corner badge on an avatar. Layout can't express children for an + // ellipse, so models stack a sibling on purpose; don't report it. + let center_inside = |inner: &Rect, outer: &Rect| { + let (cx, cy) = (inner.x + inner.w / 2.0, inner.y + inner.h / 2.0); + cx > outer.x && cx < outer.x + outer.w && cy > outer.y && cy < outer.y + outer.h + }; + let overlay = center_inside(ra, rb) || center_inside(rb, ra); + if breathing < -SIBLING_OVERLAP_EPS && !overlay { out.push(format!( "{} and {}: siblings OVERLAP by {}px — their combined width exceeds the row; shrink widths or wrap text so they fit side by side", diag_label(a), @@ -604,8 +637,8 @@ fn collect_sibling_jam_diagnostics( )); } else if breathing < SIBLING_JAM_GAP && ra.h.min(rb.h) <= ROW_CELL_MAX_H - && bears_text(a) - && bears_text(b) + && is_cell_like(a) + && is_cell_like(b) { out.push(format!( "{} and {}: text columns touch (only {}px apart) — their contents read as one word; add a gap on the row (e.g. gap: 16)", diff --git a/crates/op-orchestrator/src/geometry_validation_tests.rs b/crates/op-orchestrator/src/geometry_validation_tests.rs index 1b30302f5..c77ad17ec 100644 --- a/crates/op-orchestrator/src/geometry_validation_tests.rs +++ b/crates/op-orchestrator/src/geometry_validation_tests.rs @@ -796,7 +796,10 @@ fn resolved_rect_probe() { let v = serde_json::to_value(root).unwrap(); fn walk(v: &serde_json::Value, rects: &HashMap, pat: &str, depth: usize) { let name = v.get("name").and_then(|x| x.as_str()).unwrap_or(""); - if !pat.is_empty() && name.to_lowercase().contains(pat) { + let nid = v.get("id").and_then(|x| x.as_str()).unwrap_or(""); + if !pat.is_empty() + && (name.to_lowercase().contains(pat) || nid.eq_ignore_ascii_case(pat)) + { if let Some(r) = v .get("id") .and_then(|x| x.as_str()) @@ -988,3 +991,94 @@ fn real_layout_shrinks_rigid_fit_child_overflowing_a_narrow_card() { pair.get("width") ); } + +#[test] +fn real_layout_wraps_text_pushed_past_the_row_edge_by_a_sibling() { + use crate::test_support::VecDocSink; + use crate::types::DocSink; + use jian_ops_schema::node::PenNode; + use op_editor_core::PenNodeExt; + + // p44's verbatim shape: a 116px centered row holding [36px ellipse, fit + // text] — the text alone fits the row, but the PAIR overflows and the + // text's right edge lands past the row edge. The width-only check missed + // this; the right-edge check must wrap the text. + let root: PenNode = serde_json::from_value(json!({ + "type":"frame","id":"root","name":"Step Card","width":400,"height":"fit_content","layout":"vertical","children":[ + {"type":"frame","id":"row","name":"Avatar Row","layout":"horizontal","width":116,"height":"fit_content","justifyContent":"center","alignItems":"center","children":[ + {"type":"ellipse","id":"av","width":36,"height":36}, + {"type":"text","id":"nm","name":"Name","content":"Personalize your workspace","fontSize":14} + ]} + ] + })) + .expect("valid root"); + let mut sink = VecDocSink::new(); + sink.apply(EditorCommand::InsertSubtree { + nodes: vec![root], + parent_id: NodeId::NONE, + page_id: None, + }); + let root_id = sink.state().active_children()[0].id_str().to_string(); + geometry_validate_and_fix(&mut sink, &root_id); + let v = serde_json::to_value(sink.state().active_children()[0].clone()).unwrap(); + fn find<'a>(v: &'a serde_json::Value, name: &str) -> Option<&'a serde_json::Value> { + if v.get("name").and_then(|x| x.as_str()) == Some(name) { + return Some(v); + } + v.get("children") + .and_then(|c| c.as_array()) + .into_iter() + .flatten() + .find_map(|c| find(c, name)) + } + let nm = find(&v, "Name").expect("text survives"); + assert_eq!( + nm.get("width").and_then(|w| w.as_str()), + Some("fill_container"), + "pair-overflowed text wrapped, got {:?}", + nm.get("width") + ); +} + +#[test] +fn ring_badge_overlay_is_not_reported_as_an_overlap() { + // A step-ring: ellipse + a short number stacked ON it (center inside) — + // an intentional overlay, not an overflow accident. + let row = json!({ + "type":"frame","id":"row","name":"Ring","layout":"horizontal","children":[ + {"type":"ellipse","id":"e","width":36,"height":36}, + {"type":"text","id":"t","content":"2","fontSize":15} + ] + }); + let mut rects = std::collections::HashMap::new(); + rects.insert( + "row".into(), + Rect { + x: 0.0, + y: 0.0, + w: 116.0, + h: 36.0, + }, + ); + rects.insert( + "e".into(), + Rect { + x: 40.0, + y: 0.0, + w: 36.0, + h: 36.0, + }, + ); + rects.insert( + "t".into(), + Rect { + x: 53.0, + y: 9.0, + w: 10.0, + h: 18.0, + }, + ); // centered on the ring + let mut out = Vec::new(); + collect_sibling_jam_diagnostics(&row, &rects, &mut out); + assert!(out.is_empty(), "overlay must not be reported: {out:?}"); +}