diff --git a/crates/op-orchestrator/src/loop_finalize_tests.rs b/crates/op-orchestrator/src/loop_finalize_tests.rs index c2df109f3..c35e1b960 100644 --- a/crates/op-orchestrator/src/loop_finalize_tests.rs +++ b/crates/op-orchestrator/src/loop_finalize_tests.rs @@ -288,6 +288,86 @@ fn loop_finalize_preserves_page_root_background() { ); } +#[test] +fn loop_finalize_restores_nested_front_card_surface_without_reordering_stack() { + let mut state = state_with_forest(json!([{ + "type":"frame","id":"page","name":"Page","width":375,"height":812, + "layout":"vertical","children":[{ + "type":"frame","id":"preview","name":"Word Card Stack Preview", + "layout":"vertical","children":[{ + "type":"frame","id":"stack","name":"Card Stack Container", + "width":"fill_container","height":168,"layout":"none","children":[ + { + "type":"frame","id":"front","name":"Front Card", + "x":0,"y":18,"width":345,"height":148, + "layout":"vertical","cornerRadius":20,"fill":[], + "children":[ + {"type":"frame","id":"tag","name":"Tag Pill", + "fill":[{"type":"solid","color":"#F3F4F6"}], + "children":[{"type":"text","id":"tag-text","content":"Today"}]}, + {"type":"frame","id":"body","name":"Front Center Block", + "children":[{"type":"text","id":"word","content":"Resilient"}]}, + {"type":"frame","id":"action","name":"Audio Button","role":"button", + "fill":[{"type":"solid","color":"#FF6B6B"}], + "children":[{"type":"icon_font","id":"audio","iconFontName":"volume-2"}]} + ] + }, + { + "type":"frame","id":"back","name":"Peeked Back Card","role":"card", + "x":14,"y":0,"width":317,"height":148,"cornerRadius":18, + "fill":[{"type":"solid","color":"$color-surface-3"}], + "children":[{"type":"text","id":"back-text","content":"Example"}] + } + ] + }] + }] + }])); + + apply_loop_finalize(&mut state); + + assert_eq!( + role_of(&state, "Front Card").as_deref(), + Some("card"), + "a nested real card must not be stripped as a forest-root wrapper" + ); + assert_eq!( + fill_of(&state, "Front Card"), + Some(json!([{"type":"solid","color":"$color-surface"}])), + "the front card needs a theme-aware opaque surface, not page-bg white that later gets stripped" + ); + + let stack = + find_by_name(state.active_children(), "Card Stack Container").expect("card stack survives"); + let children = stack.children().expect("stack children"); + let names: Vec<&str> = children + .iter() + .map(|child| child.base().name.as_deref().unwrap_or("")) + .collect(); + assert_eq!( + names, + ["Front Card", "Peeked Back Card"], + "topmost-first authored order is already correct and must not change" + ); + assert_eq!( + [ + children[0].base().x, + children[0].base().y, + children[0].width_px(), + children[0].height_px(), + ], + [Some(0.0), Some(18.0), Some(345.0), Some(148.0)] + ); + assert_eq!( + [ + children[1].base().x, + children[1].base().y, + children[1].width_px(), + children[1].height_px(), + ], + [Some(14.0), Some(0.0), Some(317.0), Some(148.0)] + ); +} + /// Empty document → no panic, no-op. #[test] fn loop_finalize_on_empty_doc_is_noop() { diff --git a/crates/op-orchestrator/src/mobile_content_rail.rs b/crates/op-orchestrator/src/mobile_content_rail.rs index 48bb274e5..f0d71e08c 100644 --- a/crates/op-orchestrator/src/mobile_content_rail.rs +++ b/crates/op-orchestrator/src/mobile_content_rail.rs @@ -20,6 +20,8 @@ use std::collections::{BTreeMap, HashSet}; const DEFAULT_MOBILE_RAIL: f64 = 24.0; const MIN_MOBILE_WIDTH: f64 = 320.0; const MAX_MOBILE_WIDTH: f64 = 480.0; +const MIN_CONTENT_RAIL: f64 = 16.0; +const MAX_CONTENT_RAIL: f64 = 28.0; pub(crate) fn repair_mobile_content_rails_for_all_roots(sink: &mut dyn DocSink) { let root_ids: Vec = sink @@ -34,21 +36,43 @@ pub(crate) fn repair_mobile_content_rails_for_all_roots(sink: &mut dyn DocSink) } pub(crate) fn repair_mobile_content_rails(sink: &mut dyn DocSink, root_id: &str) { - let repairs = { - let Some(root) = sink - .state() - .active_children() - .iter() - .find(|node| node.id_str() == root_id) - else { + // `root_id` is a top-level active root in the common case (fresh-document + // generation), but the classic selected-frame append path nests the + // inserted subtree root under an existing top-level mobile screen instead + // of placing it at the top level (the same "Component 11c" shape + // `cleanup.rs::find_root` already works around). The appended fragment + // itself is usually just a section — it will not pass + // `looks_like_mobile_screen` on its own — so on a miss we walk up to the + // enclosing top-level screen to use as detection CONTEXT, but keep + // mutations scoped to `root_id`'s own subtree so pre-existing content + // elsewhere in that screen is never touched. + let (repairs, apply_root_id) = { + let roots = sink.state().active_children(); + if let Some(root) = roots.iter().find(|node| node.id_str() == root_id) { + (collect_repairs(root), root_id.to_string()) + } else if let Some((screen, scope)) = find_enclosing_top_level(roots, root_id) { + let repairs = collect_repairs(screen) + .into_iter() + .filter(|repair| repair_touches_scope(repair, &scope)) + .collect(); + (repairs, screen.id_str().to_string()) + } else { return; - }; - collect_repairs(root) + } }; + // Scope filtering above is per-repair, but some repairs are only correct + // as a PAIR (a scroller's own leading-rail add + its inner lane's + // matching leading-rail clear): if the append's scope contains only the + // inner lane, filtering can keep the "clear" half and drop the "add" + // half, net-erasing the rail entirely. Drop any repair whose declared + // partner did not survive scoping. + let repairs = drop_unpaired_repairs(repairs); for repair in repairs { match repair { - RailRepair::SetPadding { node_id, padding } => { + RailRepair::SetPadding { + node_id, padding, .. + } => { sink.apply(EditorCommand::SetNodeLayoutProp { node_id: NodeId::new(node_id), property: "padding".to_string(), @@ -63,7 +87,7 @@ pub(crate) fn repair_mobile_content_rails(sink: &mut dyn DocSink, root_id: &str) let wrapper_id = NodeId::new(wrapper.id_str().to_string()); if !sink.apply(EditorCommand::InsertAuthoredSubtree { nodes: vec![*wrapper], - parent_id: NodeId::new(root_id.to_string()), + parent_id: NodeId::new(apply_root_id.clone()), page_id: None, }) { continue; @@ -82,7 +106,7 @@ pub(crate) fn repair_mobile_content_rails(sink: &mut dyn DocSink, root_id: &str) } sink.apply(EditorCommand::MoveNode { node_id: wrapper_id, - target_parent: NodeId::new(root_id.to_string()), + target_parent: NodeId::new(apply_root_id.clone()), page_id: None, index: Some(original_index), }); @@ -91,11 +115,75 @@ pub(crate) fn repair_mobile_content_rails(sink: &mut dyn DocSink, root_id: &str) } } +/// Finds the top-level active root whose subtree contains `target_id`, +/// returning that root together with the full id set of `target_id`'s own +/// subtree (the mutation-scope allowlist). +fn find_enclosing_top_level<'a>( + roots: &'a [PenNode], + target_id: &str, +) -> Option<(&'a PenNode, HashSet)> { + roots + .iter() + .find_map(|root| subtree_ids_rooted_at(root, target_id).map(|scope| (root, scope))) +} + +/// Returns the id set of `target_id`'s own subtree (including itself) if +/// `target_id` appears anywhere under `node`, else `None`. +fn subtree_ids_rooted_at(node: &PenNode, target_id: &str) -> Option> { + if node.id_str() == target_id { + let mut ids = HashSet::new(); + collect_node_ids(node, &mut ids); + return Some(ids); + } + node.children() + .into_iter() + .flatten() + .find_map(|child| subtree_ids_rooted_at(child, target_id)) +} + +/// True when a repair's mutation target lies inside the scoped subtree. +fn repair_touches_scope(repair: &RailRepair, scope: &HashSet) -> bool { + match repair { + RailRepair::SetPadding { node_id, .. } => scope.contains(node_id), + RailRepair::WrapSurface { surface_id, .. } => scope.contains(surface_id), + } +} + +/// Drops any `SetPadding` repair whose `requires` partner (the other half of +/// an atomic outer-add/inner-clear pair, see `collect_scroller_padding_repairs`) +/// is not itself present in `repairs`. A no-op when `repairs` is unfiltered +/// (both halves of every pair are always present together), so this is safe +/// to run unconditionally. +fn drop_unpaired_repairs(repairs: Vec) -> Vec { + let present: HashSet = repairs + .iter() + .filter_map(|repair| match repair { + RailRepair::SetPadding { node_id, .. } => Some(node_id.clone()), + RailRepair::WrapSurface { .. } => None, + }) + .collect(); + repairs + .into_iter() + .filter(|repair| match repair { + RailRepair::SetPadding { + requires: Some(req), + .. + } => present.contains(req), + _ => true, + }) + .collect() +} + #[derive(Debug)] enum RailRepair { SetPadding { node_id: String, padding: Vec, + /// When `Some(id)`, this repair is only meaningful if the repair + /// targeting `id` is ALSO present in the final (post-scope-filter) + /// list — see `collect_scroller_padding_repairs`'s outer-add / + /// inner-clear pair. + requires: Option, }, WrapSurface { surface_id: String, @@ -132,12 +220,7 @@ fn collect_repairs(root: &PenNode) -> Vec { // A root-direct horizontal viewport owns its own asymmetric rail, // including image-only carousels that have no text/icon descendants. if is_clipped_horizontal_scroller(section) { - if horizontal_padding(section).is_none_or(|pair| !nonzero_pair(pair)) { - repairs.push(RailRepair::SetPadding { - node_id: section.id_str().to_string(), - padding: padding_with_leading_rail(section, rail), - }); - } + collect_scroller_padding_repairs(section, rail, &mut repairs); continue; } @@ -173,14 +256,7 @@ fn collect_repairs(root: &PenNode) -> Vec { // viewport. for child in section.children().into_iter().flatten() { if is_clipped_horizontal_scroller(child) { - if !has_expression_padding(child) - && horizontal_padding(child).is_none_or(|pair| !nonzero_pair(pair)) - { - repairs.push(RailRepair::SetPadding { - node_id: child.id_str().to_string(), - padding: padding_with_leading_rail(child, rail), - }); - } + collect_scroller_padding_repairs(child, rail, &mut repairs); } else if is_scroller_header(child) && !has_expression_padding(child) && horizontal_padding(child).is_none_or(|pair| !nonzero_pair(pair)) @@ -188,6 +264,7 @@ fn collect_repairs(root: &PenNode) -> Vec { repairs.push(RailRepair::SetPadding { node_id: child.id_str().to_string(), padding: padding_with_horizontal_rail(child, rail), + requires: None, }); } } @@ -213,6 +290,7 @@ fn collect_repairs(root: &PenNode) -> Vec { repairs.push(RailRepair::SetPadding { node_id: section.id_str().to_string(), padding: padding_with_horizontal_rail(section, rail), + requires: None, }); } } @@ -254,7 +332,7 @@ fn infer_content_rail(sections: &[PenNode]) -> f64 { let Some((left, right)) = horizontal_padding(section) else { continue; }; - if (left - right).abs() > 0.5 || !(16.0..=28.0).contains(&left) { + if (left - right).abs() > 0.5 || !(MIN_CONTENT_RAIL..=MAX_CONTENT_RAIL).contains(&left) { continue; } *counts.entry(left.round() as i64).or_default() += 1; @@ -437,9 +515,87 @@ fn is_clipped_horizontal_scroller(node: &PenNode) -> bool { }) } +fn collect_scroller_padding_repairs(scroller: &PenNode, rail: f64, repairs: &mut Vec) { + if has_expression_padding(scroller) { + return; + } + + let current_padding = horizontal_padding(scroller); + let needs_leading_rail = current_padding.is_none_or(|(left, _)| left <= 0.0); + + if needs_leading_rail { + repairs.push(RailRepair::SetPadding { + node_id: scroller.id_str().to_string(), + padding: padding_with_leading_rail(scroller, rail), + requires: None, + }); + } + + if let Some((node_id, padding, inner_left)) = redundant_inner_scroller_rail_repair(scroller) { + let duplicates_existing_rail = + current_padding.is_some_and(|(outer_left, _)| (outer_left - inner_left).abs() <= 0.5); + if needs_leading_rail || duplicates_existing_rail { + // The inner lane's rail is only redundant once the OUTER + // scroller actually owns a leading rail. When this push is + // triggered by `needs_leading_rail`, the outer's own add above + // is what establishes that ownership — if scope filtering drops + // the outer add (e.g. an append whose inserted root is this + // inner lane itself, not the outer viewport), clearing the inner + // lane alone would erase the rail entirely. `duplicates_existing_rail` + // needs no such pairing: the outer already owns a rail we are not + // touching, so the inner clear stands on its own. + let requires = needs_leading_rail.then(|| scroller.id_str().to_string()); + repairs.push(RailRepair::SetPadding { + node_id, + padding, + requires, + }); + } + } +} + +fn redundant_inner_scroller_rail_repair(scroller: &PenNode) -> Option<(String, Vec, f64)> { + let children = scroller.children()?; + let [inner] = children.as_slice() else { + return None; + }; + let props = container_props(inner)?; + if props.layout != Some(LayoutMode::Horizontal) + || props.clip_content == Some(true) + || !matches!( + props.width.as_ref(), + Some(SizingBehavior::Keyword(SizingKeyword::FitContent)) + ) + || !is_transparent_surface(inner) + || has_expression_padding(inner) + || inner.children().map_or(0, |children| children.len()) < 2 + { + return None; + } + + let (left, right) = horizontal_padding(inner)?; + if !(MIN_CONTENT_RAIL..=MAX_CONTENT_RAIL).contains(&left) { + return None; + } + let (top, bottom) = vertical_padding(inner); + Some(( + inner.id_str().to_string(), + vec![top, right, bottom, 0.0], + left, + )) +} + fn contains_clipped_horizontal_scroller(node: &PenNode) -> bool { - is_clipped_horizontal_scroller(node) - || node + if is_clipped_horizontal_scroller(node) { + return true; + } + + // Only transparent structural wrappers can pass scroller ownership up to + // an ancestor section. Surfaced cards often contain clipped horizontal + // progress meters; treating those meters as page rails suppresses the + // card group's own mobile content inset. + is_transparent_surface(node) + && node .children() .is_some_and(|children| children.iter().any(contains_clipped_horizontal_scroller)) } diff --git a/crates/op-orchestrator/src/mobile_content_rail_tests.rs b/crates/op-orchestrator/src/mobile_content_rail_tests.rs index a18cad791..849fe3731 100644 --- a/crates/op-orchestrator/src/mobile_content_rail_tests.rs +++ b/crates/op-orchestrator/src/mobile_content_rail_tests.rs @@ -1,4 +1,6 @@ -use crate::mobile_content_rail::repair_mobile_content_rails_for_all_roots; +use crate::mobile_content_rail::{ + repair_mobile_content_rails, repair_mobile_content_rails_for_all_roots, +}; use crate::test_support::VecDocSink; use crate::types::DocSink; use jian_ops_schema::node::PenNode; @@ -104,6 +106,144 @@ fn clipped_scroller_repairs_header_and_leading_edge_only() { ); } +#[test] +fn clipped_scroller_moves_duplicate_inner_leading_rail_to_viewport() { + let mut sink = insert(json!({ + "type":"frame","id":"root","width":375,"height":812,"layout":"vertical", + "children":[ + {"type":"frame","id":"status","role":"status-bar","height":44}, + {"type":"frame","id":"hourly","width":"fill_container","layout":"vertical", + "children":[ + {"type":"frame","id":"header","name":"Header Row","layout":"horizontal", + "children":[text("title","Hourly Forecast")]}, + {"type":"frame","id":"viewport","width":"fill_container","layout":"horizontal", + "clipContent":true,"children":[ + {"type":"frame","id":"rail","width":"fit_content","layout":"horizontal", + "gap":8,"padding":[0,24],"children":[ + {"type":"frame","id":"now","width":58,"cornerRadius":12, + "children":[text("now-t","NOW")]}, + {"type":"frame","id":"later","width":58,"cornerRadius":12, + "children":[text("later-t","13:00")]} + ]} + ]} + ]}, + {"type":"frame","id":"body","padding":[0,24],"children":[text("body-t","Body")]}, + {"type":"frame","id":"nav","role":"bottom-tab-bar","height":72} + ] + })); + + repair_mobile_content_rails_for_all_roots(&mut sink); + + assert_eq!( + node_json(&sink, "viewport")["padding"], + json!([0.0, 0.0, 0.0, 24.0]) + ); + assert_eq!( + node_json(&sink, "rail")["padding"], + json!([0.0, 24.0, 0.0, 0.0]), + "the viewport owns the leading rail while the lane keeps its end spacer" + ); + + sink.applied.clear(); + repair_mobile_content_rails_for_all_roots(&mut sink); + assert!( + sink.applied.is_empty(), + "the single-owner scroller rail must be idempotent" + ); +} + +#[test] +fn existing_double_scroller_rail_collapses_to_one_leading_owner() { + let mut sink = insert(json!({ + "type":"frame","id":"root","width":375,"height":812,"layout":"vertical", + "children":[ + {"type":"frame","id":"status","role":"status-bar","height":44}, + {"type":"frame","id":"viewport","width":"fill_container","layout":"horizontal", + "padding":[0,0,0,24],"clipContent":true,"children":[ + {"type":"frame","id":"rail","width":"fit_content","layout":"horizontal", + "padding":[0,24],"children":[ + {"type":"frame","id":"a","width":58,"children":[text("a-t","A")]}, + {"type":"frame","id":"b","width":58,"children":[text("b-t","B")]} + ]} + ]}, + {"type":"frame","id":"body","padding":[0,24],"children":[text("body-t","Body")]}, + {"type":"frame","id":"nav","role":"bottom-tab-bar","height":72} + ] + })); + + repair_mobile_content_rails_for_all_roots(&mut sink); + + assert_eq!( + node_json(&sink, "viewport")["padding"], + json!([0.0, 0.0, 0.0, 24.0]) + ); + assert_eq!( + node_json(&sink, "rail")["padding"], + json!([0.0, 24.0, 0.0, 0.0]) + ); +} + +#[test] +fn authored_small_viewport_inset_does_not_consume_the_inner_content_rail() { + let mut sink = insert(json!({ + "type":"frame","id":"root","width":375,"height":812,"layout":"vertical", + "children":[ + {"type":"frame","id":"status","role":"status-bar","height":44}, + {"type":"frame","id":"viewport","width":"fill_container","layout":"horizontal", + "padding":[0,0,0,8],"clipContent":true,"children":[ + {"type":"frame","id":"rail","width":"fit_content","layout":"horizontal", + "padding":[0,24],"children":[ + {"type":"frame","id":"a","width":58,"children":[text("a-t","A")]}, + {"type":"frame","id":"b","width":58,"children":[text("b-t","B")]} + ]} + ]}, + {"type":"frame","id":"body","padding":[0,24],"children":[text("body-t","Body")]}, + {"type":"frame","id":"nav","role":"bottom-tab-bar","height":72} + ] + })); + + repair_mobile_content_rails_for_all_roots(&mut sink); + + assert_eq!( + node_json(&sink, "viewport")["padding"], + json!([0.0, 0.0, 0.0, 8.0]) + ); + assert_eq!( + node_json(&sink, "rail")["padding"], + json!([0.0, 24.0]), + "a non-canonical authored viewport inset is not proof of duplicate rail ownership" + ); +} + +#[test] +fn clipped_scroller_does_not_rewrite_a_surfaced_child_card() { + let mut sink = insert(json!({ + "type":"frame","id":"root","width":375,"height":812,"layout":"vertical", + "children":[ + {"type":"frame","id":"status","role":"status-bar","height":44}, + {"type":"frame","id":"viewport","width":"fill_container","layout":"horizontal", + "clipContent":true,"children":[ + {"type":"frame","id":"card","width":"fit_content","layout":"horizontal", + "padding":[0,24],"cornerRadius":16, + "fill":[{"type":"solid","color":"#181818"}],"children":[ + {"type":"text","id":"card-title","content":"Title"}, + {"type":"text","id":"card-meta","content":"Meta"} + ]} + ]}, + {"type":"frame","id":"body","padding":[0,24],"children":[text("body-t","Body")]}, + {"type":"frame","id":"nav","role":"bottom-tab-bar","height":72} + ] + })); + + repair_mobile_content_rails_for_all_roots(&mut sink); + + assert_eq!( + node_json(&sink, "viewport")["padding"], + json!([0.0, 0.0, 0.0, 24.0]) + ); + assert_eq!(node_json(&sink, "card")["padding"], json!([0.0, 24.0])); +} + #[test] fn root_direct_scroller_gets_leading_rail_without_trailing_inset() { let mut sink = insert(json!({ @@ -131,6 +271,33 @@ fn root_direct_scroller_gets_leading_rail_without_trailing_inset() { ); } +#[test] +fn trailing_only_scroller_padding_still_gets_a_leading_rail() { + let mut sink = insert(json!({ + "type":"frame","id":"root","width":375,"height":812,"layout":"vertical", + "children":[ + {"type":"frame","id":"status","role":"status-bar","height":44}, + {"type":"frame","id":"viewport","width":"fill_container","layout":"horizontal", + "padding":[0,12,0,0],"clipContent":true,"children":[ + {"type":"frame","id":"row","width":"fit_content","layout":"horizontal", + "children":[ + {"type":"image","id":"image-a","width":120,"height":90,"src":"a.png"}, + {"type":"image","id":"image-b","width":120,"height":90,"src":"b.png"} + ]} + ]}, + {"type":"frame","id":"body","padding":[0,24],"children":[text("body-t","Body")]}, + {"type":"frame","id":"nav","role":"bottom-tab-bar","height":72} + ] + })); + + repair_mobile_content_rails_for_all_roots(&mut sink); + + assert_eq!( + node_json(&sink, "viewport")["padding"], + json!([0.0, 12.0, 0.0, 24.0]) + ); +} + #[test] fn deeper_scroller_is_not_closed_by_symmetric_ancestor_padding() { let mut sink = insert(json!({ @@ -156,6 +323,62 @@ fn deeper_scroller_is_not_closed_by_symmetric_ancestor_padding() { assert!(node_json(&sink, "viewport").get("padding").is_none()); } +#[test] +fn progress_meter_inside_surfaced_card_does_not_suppress_outer_section_rail() { + let mut sink = insert(json!({ + "type":"frame","id":"root","width":375,"height":812,"layout":"vertical", + "children":[ + {"type":"frame","id":"status","role":"status-bar","height":44}, + {"type":"frame","id":"forecast","width":"fill_container","layout":"vertical", + "padding":[0,24],"children":[text("forecast-t","7-Day Forecast")]}, + {"type":"frame","id":"metrics","width":"fill_container","layout":"vertical", + "children":[ + {"type":"frame","id":"metrics-row","width":"fill_container","layout":"horizontal", + "gap":12,"children":[ + {"type":"frame","id":"sun-card","width":"fill_container","layout":"vertical", + "padding":14,"cornerRadius":16, + "fill":[{"type":"solid","color":"#181818"}], + "stroke":{"thickness":1,"fill":[{"type":"solid","color":"#333333"}]}, + "children":[text("sunset","Sunset 8:32 PM")]}, + {"type":"frame","id":"environment-card","width":"fill_container", + "layout":"vertical","padding":14,"cornerRadius":16, + "fill":[{"type":"solid","color":"#181818"}], + "stroke":{"thickness":1,"fill":[{"type":"solid","color":"#333333"}]}, + "children":[ + text("humidity","Humidity 78%"), + {"type":"frame","id":"progress-meter","width":"fill_container","height":6, + "layout":"horizontal","clipContent":true, + "fill":[{"type":"solid","color":"#222222"}],"children":[ + {"type":"frame","id":"progress-fill","width":110, + "height":"fill_container", + "fill":[{"type":"solid","color":"#C4F82A"}],"children":[]} + ]} + ]} + ]} + ]}, + {"type":"frame","id":"nav","role":"bottom-tab-bar","height":72} + ] + })); + + repair_mobile_content_rails_for_all_roots(&mut sink); + + assert_eq!( + node_json(&sink, "metrics")["padding"], + json!([0.0, 24.0, 0.0, 24.0]) + ); + assert!( + node_json(&sink, "progress-meter").get("padding").is_none(), + "a clipped meter inside a surfaced card is not a page-level scroller" + ); + + sink.applied.clear(); + repair_mobile_content_rails_for_all_roots(&mut sink); + assert!( + sink.applied.is_empty(), + "the repaired outer section rail must be idempotent" + ); +} + #[test] fn full_bleed_surface_existing_root_rail_and_desktop_are_untouched() { let mut sink = insert(json!({ @@ -390,3 +613,184 @@ fn mixed_bottom_nav_shell_is_demoted_before_content_rail_repair() { "real nav keeps its own normalized internal chrome padding" ); } + +// -- Selected-frame append path: `root_id` nests under an existing top-level +// mobile screen rather than being a top-level active root itself. -- + +#[test] +fn appended_nested_section_gets_its_content_rail_repaired() { + let mut sink = insert(json!({ + "type":"frame","id":"root","width":375,"height":812,"layout":"vertical", + "children":[ + {"type":"frame","id":"status","role":"status-bar","height":44}, + {"type":"frame","id":"existing","padding":[0,24],"children":[text("existing-t","Existing")]}, + {"type":"frame","id":"appended","width":"fill_container","layout":"horizontal", + "clipContent":true,"children":[ + {"type":"frame","id":"appended-rail","width":"fit_content","layout":"horizontal", + "padding":[0,24],"children":[ + {"type":"frame","id":"a","width":58,"children":[text("a-t","A")]}, + {"type":"frame","id":"b","width":58,"children":[text("b-t","B")]} + ]} + ]}, + {"type":"frame","id":"nav","role":"bottom-tab-bar","height":72} + ] + })); + + // `appended` is not a top-level active root — it is a direct child of + // `root` — mirroring `inserted_root_ids` from a selected-frame append. + repair_mobile_content_rails(&mut sink, "appended"); + + assert_eq!( + node_json(&sink, "appended")["padding"], + json!([0.0, 0.0, 0.0, 24.0]), + "the nested viewport must gain the same leading rail a top-level one would" + ); + assert_eq!( + node_json(&sink, "appended-rail")["padding"], + json!([0.0, 24.0, 0.0, 0.0]), + "the redundant inner rail collapses just like the top-level case" + ); +} + +#[test] +fn appended_nested_section_inside_desktop_frame_is_untouched() { + let mut sink = insert(json!({ + "type":"frame","id":"desktop","width":1440,"height":900,"layout":"vertical", + "children":[ + {"type":"frame","id":"existing","children":[text("existing-t","Existing")]}, + {"type":"frame","id":"appended","width":"fill_container","layout":"horizontal", + "clipContent":true,"children":[ + {"type":"frame","id":"appended-rail","width":"fit_content","layout":"horizontal", + "padding":[0,24],"children":[ + {"type":"frame","id":"a","width":58,"children":[text("a-t","A")]}, + {"type":"frame","id":"b","width":58,"children":[text("b-t","B")]} + ]} + ]} + ] + })); + + repair_mobile_content_rails(&mut sink, "appended"); + + assert!( + sink.applied.is_empty(), + "the enclosing top-level root is not a mobile screen, so the append is out of scope" + ); +} + +#[test] +fn appended_section_repair_never_touches_a_sibling_double_rail_scroller() { + let mut sink = insert(json!({ + "type":"frame","id":"root","width":375,"height":812,"layout":"vertical", + "children":[ + {"type":"frame","id":"status","role":"status-bar","height":44}, + {"type":"frame","id":"existing-viewport","width":"fill_container","layout":"horizontal", + "padding":[0,0,0,24],"clipContent":true,"children":[ + {"type":"frame","id":"existing-rail","width":"fit_content","layout":"horizontal", + "padding":[0,24],"children":[ + {"type":"frame","id":"x","width":58,"children":[text("x-t","X")]}, + {"type":"frame","id":"y","width":58,"children":[text("y-t","Y")]} + ]} + ]}, + {"type":"frame","id":"appended","width":"fill_container","layout":"horizontal", + "clipContent":true,"children":[ + {"type":"frame","id":"appended-rail","width":"fit_content","layout":"horizontal", + "padding":[0,24],"children":[ + {"type":"frame","id":"a","width":58,"children":[text("a-t","A")]}, + {"type":"frame","id":"b","width":58,"children":[text("b-t","B")]} + ]} + ]}, + {"type":"frame","id":"nav","role":"bottom-tab-bar","height":72} + ] + })); + + // `existing-viewport` already carries the exact same double-rail + // duplicate the appended section has, but it was not part of this + // append — the fix must be scoped to `appended`'s own subtree only. + repair_mobile_content_rails(&mut sink, "appended"); + + assert_eq!( + node_json(&sink, "appended")["padding"], + json!([0.0, 0.0, 0.0, 24.0]) + ); + assert_eq!( + node_json(&sink, "appended-rail")["padding"], + json!([0.0, 24.0, 0.0, 0.0]) + ); + assert_eq!( + node_json(&sink, "existing-viewport")["padding"], + json!([0.0, 0.0, 0.0, 24.0]), + "sibling scroller outside the inserted subtree keeps its original padding" + ); + assert_eq!( + node_json(&sink, "existing-rail")["padding"], + json!([0.0, 24.0]), + "sibling scroller's inner rail is out of mutation scope and must be left alone" + ); +} + +#[test] +fn appended_section_repair_is_idempotent() { + let mut sink = insert(json!({ + "type":"frame","id":"root","width":375,"height":812,"layout":"vertical", + "children":[ + {"type":"frame","id":"status","role":"status-bar","height":44}, + {"type":"frame","id":"existing","padding":[0,24],"children":[text("existing-t","Existing")]}, + {"type":"frame","id":"appended","width":"fill_container","layout":"horizontal", + "clipContent":true,"children":[ + {"type":"frame","id":"appended-rail","width":"fit_content","layout":"horizontal", + "padding":[0,24],"children":[ + {"type":"frame","id":"a","width":58,"children":[text("a-t","A")]}, + {"type":"frame","id":"b","width":58,"children":[text("b-t","B")]} + ]} + ]}, + {"type":"frame","id":"nav","role":"bottom-tab-bar","height":72} + ] + })); + + repair_mobile_content_rails(&mut sink, "appended"); + sink.applied.clear(); + repair_mobile_content_rails(&mut sink, "appended"); + + assert!( + sink.applied.is_empty(), + "a second repair pass over the same nested append must be a no-op" + ); +} + +#[test] +fn appended_inner_lane_alone_does_not_get_its_rail_stripped() { + let mut sink = insert(json!({ + "type":"frame","id":"root","width":375,"height":812,"layout":"vertical", + "children":[ + {"type":"frame","id":"status","role":"status-bar","height":44}, + {"type":"frame","id":"viewport","width":"fill_container","layout":"horizontal", + "clipContent":true,"children":[ + {"type":"frame","id":"lane","width":"fit_content","layout":"horizontal", + "padding":[0,24],"children":[ + {"type":"frame","id":"a","width":58,"children":[text("a-t","A")]}, + {"type":"frame","id":"b","width":58,"children":[text("b-t","B")]} + ]} + ]}, + {"type":"frame","id":"body","padding":[0,24],"children":[text("body-t","Body")]}, + {"type":"frame","id":"nav","role":"bottom-tab-bar","height":72} + ] + })); + + // `viewport` already exists (no leading padding of its own) and `lane` + // is the append's inserted root — i.e. root_id lands on the INNER lane, + // not the outer scroller. The outer add and inner clear are an atomic + // pair; since the outer viewport is out of scope, neither half may + // apply, or `lane`'s own canonical rail gets wiped with nothing put in + // its place. + repair_mobile_content_rails(&mut sink, "lane"); + + assert_eq!( + node_json(&sink, "lane")["padding"], + json!([0.0, 24.0]), + "the inner lane's own rail must survive when its outer viewport pairing is out of scope" + ); + assert!( + node_json(&sink, "viewport").get("padding").is_none(), + "the outer viewport is out of scope for this append and must not be touched either" + ); +}