diff --git a/crates/op-orchestrator/src/unify_shared_nav.rs b/crates/op-orchestrator/src/unify_shared_nav.rs index 5a8eda8b7..4bb5067fc 100644 --- a/crates/op-orchestrator/src/unify_shared_nav.rs +++ b/crates/op-orchestrator/src/unify_shared_nav.rs @@ -33,17 +33,13 @@ //! //! ## Active-tab idempotency gap (0718-1-k3-1 postmortem, D) //! -//! [`labels_already_unified`] only compares the tab LABEL set — it says -//! nothing about which tab is active. A model that draws every screen's nav -//! byte-for-byte identical (measured: three screens, one nav, one active -//! tab baked into all three) short-circuits `resolve_target` into `None` -//! before `retarget_active_tab` ever runs, so the wrong tab reads active on -//! every non-reference screen. [`active_tab_is_correct`] is the missing -//! second half of the idempotency check: labels matching is necessary but -//! not sufficient. When it disagrees, [`SyncTarget::RetargetActiveOnly`] -//! fixes just the active styling in place — no full nav replacement, so any -//! other authored drift on that screen's own nav (icons, ids) survives -//! untouched. +//! An earlier idempotency gate compared only tab LABELS, so equal labels hid +//! independently redrawn glyphs and wrappers. [`navs_already_unified`] now +//! compares ordered text/icon/event identity plus the complete outer chrome after +//! normalizing both navs to one active tab. [`active_tab_is_correct`] remains +//! the second half of the gate: when complete shared identity matches but +//! the active destination does not, [`SyncTarget::RetargetActiveOnly`] moves +//! only active styling on the target's otherwise-identical chrome. //! //! ## Detail-page Inject exemption (0718-1-k3-1 postmortem, product decision) //! @@ -76,9 +72,11 @@ use jian_ops_schema::node::{PenNode, TextContent}; use op_editor_core::{EditorCommand, NodeId, PenNodeExt}; use crate::types::DocSink; +#[cfg(test)] +use crate::wire_screen_navigation::collect_nav_containers; use crate::wire_screen_navigation::{ - collect_nav_containers, collect_screen_candidates, first_text_content, labels_match, - normalize_label, screen_has_back_control_in_header, ScreenCandidate, + collect_nav_parts, collect_screen_candidates, first_text_content, labels_match, + screen_has_back_control_in_header, NavParts, ScreenCandidate, }; /// What a non-reference screen needs, resolved read-only before any @@ -90,12 +88,27 @@ enum SyncTarget { /// This screen has NO nav at all, but the reference nav declares a tab /// for it — append a fresh clone as its last child (`InsertSubtree`). Inject, - /// Labels already match the reference, but the ACTIVE tab doesn't sit - /// on this screen's own tab (the D bug — see the module doc). Carries - /// the TARGET's own live nav id + an owned clone of it (not the + /// Full shared identity already matches the reference, but the ACTIVE + /// tab doesn't sit on this screen's own tab (the D bug — see the module + /// doc). Carries the TARGET's own live nav id + an owned clone of it (not the /// reference's), so the in-place fix only ever moves active styling — - /// any other authored drift on this screen's own nav survives. - RetargetActiveOnly(String, Box), + /// the target's already-matching outer chrome and actual-row path. + RetargetActiveOnly { + surface_id: String, + tab_row_path: Vec, + own_surface: Box, + }, +} + +/// Authoritative shared chrome captured from the document-order first +/// screen that owns a real tab row. `surface` is the complete outer chrome +/// copied on drift; `tab_row_path` locates the interactive row inside that +/// owned clone; `tab_row` is kept separately for read-only identity checks. +struct ReferenceNav { + surface: PenNode, + tab_row_path: Vec, + tab_row: PenNode, + canonical_label: String, } /// Entry point. No-ops when fewer than 2 screen-shaped top-level frames @@ -129,8 +142,8 @@ pub fn unify_shared_nav(sink: &mut dyn DocSink) { match target { SyncTarget::Replace(target_nav_id) => { - let mut clone = reference_nav.clone(); - retarget_active_tab(&mut clone, &screen.name); + let mut clone = reference_nav.surface.clone(); + retarget_active_tab_at_path(&mut clone, &reference_nav.tab_row_path, &screen.name); stamp_chrome_role(&mut clone); // `ReplaceSubtree` remaps every id in `clone` (root AND // descendants) to fresh, non-colliding ids on apply @@ -145,8 +158,8 @@ pub fn unify_shared_nav(sink: &mut dyn DocSink) { }); } SyncTarget::Inject => { - let mut clone = reference_nav.clone(); - retarget_active_tab(&mut clone, &screen.name); + let mut clone = reference_nav.surface.clone(); + retarget_active_tab_at_path(&mut clone, &reference_nav.tab_row_path, &screen.name); stamp_chrome_role(&mut clone); // `InsertSubtree` APPENDS to the target parent's children // (`cmd_insert_subtree`'s `slot.extend(nodes)`), so the @@ -164,16 +177,18 @@ pub fn unify_shared_nav(sink: &mut dyn DocSink) { page_id: None, }); } - SyncTarget::RetargetActiveOnly(target_nav_id, mut own_nav) => { + SyncTarget::RetargetActiveOnly { + surface_id, + tab_row_path, + mut own_surface, + } => { // Mutate the TARGET's own live nav (captured read-only in - // `resolve_target`), not a reference clone — labels already - // matched, so a full replace would silently overwrite any - // authored drift beyond the active indicator this screen's - // own nav carries (icon glyphs, ids, extra chrome). - retarget_active_tab(&mut own_nav, &screen.name); + // `resolve_target`), not a reference clone — complete shared + // identity already matched, so only active placement differs. + retarget_active_tab_at_path(&mut own_surface, &tab_row_path, &screen.name); sink.apply(EditorCommand::ReplaceSubtree { - node_id: NodeId::new(target_nav_id), - node: own_nav, + node_id: NodeId::new(surface_id), + node: own_surface, drop_children: true, page_id: None, }); @@ -190,27 +205,28 @@ pub fn unify_shared_nav(sink: &mut dyn DocSink) { fn resolve_target( sink: &dyn DocSink, screen: &ScreenCandidate, - reference_nav: &PenNode, + reference_nav: &ReferenceNav, ) -> Option { let root = op_editor_core::walkers::find_node( sink.state().active_children(), &NodeId::new(screen.id.clone()), )?; let mut navs = Vec::new(); - collect_nav_containers(root, &mut navs); + collect_nav_parts(root, &mut navs); match navs.into_iter().next() { Some(target_nav) => { - if !labels_already_unified(target_nav, reference_nav) { - Some(SyncTarget::Replace(target_nav.id_str().to_string())) - } else if active_tab_is_correct(target_nav, &screen.name) { - None // truly idempotent: labels match AND active tab is right. + if !navs_already_unified(&target_nav, reference_nav) { + Some(SyncTarget::Replace(target_nav.surface.id_str().to_string())) + } else if active_tab_is_correct(target_nav.tab_row, &screen.name) { + None // truly idempotent: shared identity + active tab are right. } else { - // Labels match but the active tab is wrong (the D bug) — - // fix in place rather than a full replace. - Some(SyncTarget::RetargetActiveOnly( - target_nav.id_str().to_string(), - Box::new(target_nav.clone()), - )) + // Shared identity matches but the active tab is wrong (the + // D bug) — fix in place rather than a full replace. + Some(SyncTarget::RetargetActiveOnly { + surface_id: target_nav.surface.id_str().to_string(), + tab_row_path: target_nav.tab_row_path, + own_surface: Box::new(target_nav.surface.clone()), + }) } } None => { @@ -220,6 +236,7 @@ fn resolve_target( // matching tab (a standalone detail page, say) is never forced // to grow chrome it never asked for. let eligible = reference_nav + .tab_row .children() .is_some_and(|tabs| find_tab_index_for_screen(tabs, &screen.name).is_some()); if !eligible { @@ -245,33 +262,105 @@ fn resolve_target( fn find_reference_nav( sink: &dyn DocSink, screens: &[ScreenCandidate], -) -> Option<(String, PenNode)> { +) -> Option<(String, ReferenceNav)> { for screen in screens { let root = op_editor_core::walkers::find_node( sink.state().active_children(), &NodeId::new(screen.id.clone()), )?; let mut navs = Vec::new(); - collect_nav_containers(root, &mut navs); + collect_nav_parts(root, &mut navs); if let Some(nav) = navs.into_iter().next() { - return Some((screen.id.clone(), nav.clone())); + let canonical_label = nav + .tab_row + .children() + .and_then(|tabs| tabs.first()) + .and_then(first_text_content) + .unwrap_or_default() + .to_string(); + return Some(( + screen.id.clone(), + ReferenceNav { + surface: nav.surface.clone(), + tab_row_path: nav.tab_row_path, + tab_row: nav.tab_row.clone(), + canonical_label, + }, + )); } } None } -/// Whether `target`'s nav already carries the SAME tab-label set (in order) -/// as `reference` — the idempotency gate. Active-tab correctness is -/// deliberately NOT part of this check (see the module doc): once the label -/// set matches, the screen is considered "already unified" even if a prior -/// run's active-tab detection degraded (see [`retarget_active_tab`]). -fn labels_already_unified(target: &PenNode, reference: &PenNode) -> bool { - nav_label_set(target) == nav_label_set(reference) +/// Whether a target already carries the reference nav's complete shared +/// identity. Ordered label/icon/event content must match, and the full outer +/// surface must have the same style/structure after both navs are +/// normalized to the same active tab. That normalization makes each +/// screen's legitimate active destination irrelevant while still catching +/// glyph, icon-size, padding, divider, wrapper, or inactive-style drift. +fn navs_already_unified(target: &NavParts<'_>, reference: &ReferenceNav) -> bool { + canonical_nav_identity( + target.surface, + &target.tab_row_path, + &reference.canonical_label, + ) == canonical_nav_identity( + &reference.surface, + &reference.tab_row_path, + &reference.canonical_label, + ) +} + +fn tab_content_identity(nav: &PenNode) -> Vec { + nav.children() + .into_iter() + .flatten() + .map(snapshot_content) + .collect() +} + +#[derive(Debug, PartialEq, Eq)] +struct CanonicalNavIdentity { + tabs: Vec, + style: String, +} + +fn canonical_nav_identity( + surface: &PenNode, + tab_row_path: &[usize], + canonical_label: &str, +) -> CanonicalNavIdentity { + let mut normalized = surface.clone(); + retarget_active_tab_at_path(&mut normalized, tab_row_path, canonical_label); + // Replace/Inject clones are stamped even when a name-matched authored + // reference has no role. Canonicalize the read-only fingerprints the + // same way, otherwise that legitimate clone would compare unequal to + // its roleless reference forever and churn ids on every cleanup run. + stamp_chrome_role(&mut normalized); + let tabs = node_at_path_mut(&mut normalized, tab_row_path) + .map(|tab_row| tab_content_identity(tab_row)) + .unwrap_or_default(); + CanonicalNavIdentity { + tabs, + style: style_fingerprint(&normalized), + } +} + +fn retarget_active_tab_at_path(nav: &mut PenNode, tab_row_path: &[usize], screen_name: &str) { + if let Some(tab_row) = node_at_path_mut(nav, tab_row_path) { + retarget_active_tab(tab_row, screen_name); + } +} + +fn node_at_path_mut<'a>(mut node: &'a mut PenNode, path: &[usize]) -> Option<&'a mut PenNode> { + for &index in path { + node = node.children_mut()?.get_mut(index)?; + } + Some(node) } /// Whether `nav`'s currently-active tab already sits on the tab matching /// `screen_name` — the other half of the idempotency check -/// [`labels_already_unified`] deliberately leaves out (see the module doc's +/// [`navs_already_unified`] deliberately leaves out (see the module doc's /// "Active-tab idempotency gap" section). Mirrors [`retarget_active_tab`]'s /// own degrade conditions exactly (same helpers, same order) so a `false` /// here reliably means `retarget_active_tab` will find a confident swap to @@ -293,15 +382,6 @@ fn active_tab_is_correct(nav: &PenNode, screen_name: &str) -> bool { active_idx == target_idx } -fn nav_label_set(nav: &PenNode) -> Vec { - nav.children() - .into_iter() - .flatten() - .filter_map(first_text_content) - .map(normalize_label) - .collect() -} - /// Move the "active" tab styling from wherever the reference nav had it onto /// the tab whose label matches `screen_name` — so a clone of the Home /// screen's nav, dropped onto the Library screen, shows Library (not Home) @@ -403,6 +483,11 @@ fn blank_content_fields(value: &mut serde_json::Value) { }; map.remove("id"); map.remove("name"); + // Events belong to each semantic tab position (Trips, Explore, ...), + // not to its active/inactive visual treatment. Keeping them here would + // make four correctly wired tabs look like four different styles and + // break majority-based active-state detection. + map.remove("events"); if map.get("type").and_then(serde_json::Value::as_str) == Some("text") { map.insert( "content".to_string(), @@ -445,35 +530,83 @@ fn find_active_by_fingerprint(fingerprints: &[String]) -> Option { } /// Swap the WHOLE nodes at `a`/`b` (carrying styling — fills, extra -/// indicator children, everything), then restore each POSITION's own -/// original content (text / icon glyph, in document order) so the tab at -/// position `a` still reads as whatever it always represented, just with -/// the OTHER position's styling now attached. +/// indicator children, everything), then restore each POSITION's semantic +/// identity (name, events, text, icon glyph) so the tab at position `a` +/// still represents the same destination, just with the OTHER position's +/// styling now attached. fn swap_tab_style(children: &mut [PenNode], a: usize, b: usize) { - let content_a = snapshot_content(&children[a]); - let content_b = snapshot_content(&children[b]); + let position_a = snapshot_tab_position(&children[a]); + let position_b = snapshot_tab_position(&children[b]); children.swap(a, b); - restore_content(&mut children[a], &content_a); - restore_content(&mut children[b], &content_b); + restore_tab_position(&mut children[a], &position_a); + restore_tab_position(&mut children[b], &position_b); } /// A tab's content identity: every `Text` node's plain string and every /// `IconFont` node's glyph name, in document (depth-first) order. +#[derive(Debug, PartialEq, Eq)] struct ContentSnapshot { texts: Vec, icon_names: Vec, + events: Vec, +} + +/// Semantic identity anchored to one tab position. Active styling is moved +/// by swapping whole nodes, but these fields must stay with the destination +/// represented by that position. +struct TabPositionSnapshot { + nodes: Vec, + content: ContentSnapshot, +} + +struct NodePositionSnapshot { + path: Vec, + name: Option, + events: Option, +} + +fn snapshot_tab_position(node: &PenNode) -> TabPositionSnapshot { + let mut nodes = Vec::new(); + collect_position_metadata(node, &mut Vec::new(), &mut nodes); + TabPositionSnapshot { + nodes, + content: snapshot_content(node), + } +} + +fn collect_position_metadata( + node: &PenNode, + path: &mut Vec, + out: &mut Vec, +) { + out.push(NodePositionSnapshot { + path: path.clone(), + name: node.base().name.clone(), + events: node.events().cloned(), + }); + for (index, child) in node.children().into_iter().flatten().enumerate() { + path.push(index); + collect_position_metadata(child, path, out); + path.pop(); + } } fn snapshot_content(node: &PenNode) -> ContentSnapshot { let mut snapshot = ContentSnapshot { texts: Vec::new(), icon_names: Vec::new(), + events: Vec::new(), }; collect_content(node, &mut snapshot); snapshot } fn collect_content(node: &PenNode, out: &mut ContentSnapshot) { + if let Some(events) = node.events() { + if let Ok(value) = serde_json::to_value(events) { + out.events.push(value); + } + } match node { PenNode::Text(t) => { if let TextContent::Plain(s) = &t.content { @@ -499,6 +632,53 @@ fn restore_content(node: &mut PenNode, snapshot: &ContentSnapshot) { restore_content_walk(node, &mut texts, &mut icons); } +fn restore_tab_position(node: &mut PenNode, snapshot: &TabPositionSnapshot) { + clear_position_metadata(node); + for original in &snapshot.nodes { + if let Some(target) = node_at_path_mut(node, &original.path) { + target.base_mut().name.clone_from(&original.name); + set_events(target, original.events.clone()); + } + } + restore_content(node, &snapshot.content); +} + +fn clear_position_metadata(node: &mut PenNode) { + node.base_mut().name = None; + set_events(node, None); + if node.children().is_some() { + for child in node.children_mut().into_iter().flatten() { + clear_position_metadata(child); + } + } +} + +fn set_events(node: &mut PenNode, events: Option) { + match node { + PenNode::Frame(n) => n.events = events, + PenNode::Group(n) => n.events = events, + PenNode::Rectangle(n) => n.events = events, + PenNode::Ellipse(n) => n.events = events, + PenNode::Line(n) => n.events = events, + PenNode::Polygon(n) => n.events = events, + PenNode::Path(n) => n.events = events, + PenNode::Text(n) => n.events = events, + PenNode::TextInput(n) => n.events = events, + PenNode::Image(n) => n.events = events, + PenNode::IconFont(n) => n.events = events, + PenNode::TextArea(n) => n.events = events, + PenNode::Select(n) => n.events = events, + PenNode::Switch(n) => n.events = events, + PenNode::Checkbox(n) => n.events = events, + PenNode::Slider(n) => n.events = events, + PenNode::RadioGroup(n) => n.events = events, + PenNode::NumberInput(n) => n.events = events, + PenNode::Progress(n) => n.events = events, + PenNode::Tabs(n) => n.events = events, + PenNode::Ref(n) => n.events = events, + } +} + fn restore_content_walk<'a>( node: &mut PenNode, texts: &mut std::slice::Iter<'a, String>, @@ -519,8 +699,14 @@ fn restore_content_walk<'a>( } _ => {} } - for child in node.children_mut().into_iter().flatten() { - restore_content_walk(child, texts, icons); + // `children_mut()` materializes an empty `children: []` on container + // variants. Only borrow it when children already exist; active-style + // normalization must not change optional-child structure merely by + // traversing a leaf rectangle such as an active indicator. + if node.children().is_some() { + for child in node.children_mut().into_iter().flatten() { + restore_content_walk(child, texts, icons); + } } } diff --git a/crates/op-orchestrator/src/unify_shared_nav_active_tab_tests.rs b/crates/op-orchestrator/src/unify_shared_nav_active_tab_tests.rs index c96d20659..ff25c16cd 100644 --- a/crates/op-orchestrator/src/unify_shared_nav_active_tab_tests.rs +++ b/crates/op-orchestrator/src/unify_shared_nav_active_tab_tests.rs @@ -11,7 +11,7 @@ use super::*; // --------------------------------------------------------------------- // D: active-tab idempotency gap (0718-1-k3-1 postmortem). -// `labels_already_unified` only compares the tab LABEL set — a model that +// The old label-only idempotency gate let a model that // draws every screen's nav byte-for-byte identical (same active tab baked // into all of them) used to short-circuit `resolve_target` into `None` // before `retarget_active_tab` ever ran. These tests exercise the new @@ -39,7 +39,7 @@ fn two_screen_doc_identical_nav(active_index_on_library: usize) -> serde_json::V fn labels_match_but_active_wrong_only_retargets_active_styling_in_place() { // Library's own nav bakes "Home" (index 0) active — wrong for the // Library screen, but same label set as the reference, so the OLD - // `labels_already_unified`-only gate would have skipped it entirely. + // label-only gate would have skipped it entirely. let mut state = state_from(two_screen_doc_identical_nav(0)); run_pass(&mut state); @@ -89,7 +89,7 @@ fn drifted_labels_still_take_the_replace_path_with_correct_active_tab() { // Regression against the D fix: a genuinely drifted label set (the // pre-existing `two_screen_drifted_doc` fixture) can ONLY reach // `resolve_target`'s `Replace` arm by construction — `RetargetActiveOnly` - // is reachable only when `labels_already_unified` is already true — so + // is reachable only when complete shared identity already matches — so // this fixture alone proves the Replace path, no id inspection needed. let mut state = state_from(two_screen_drifted_doc()); run_pass(&mut state); @@ -134,7 +134,7 @@ fn inject_path_still_activates_the_correct_tab_after_the_retarget_active_only_ad /// shape): three screens, every one authored the SAME nav byte-for-byte /// (the first tab baked active on ALL three — the real file's "Trips" tab /// lighting up on Trips/Destination/Saved alike). Before this fix, -/// `labels_already_unified` alone read every screen as already-unified, so +/// the old label-only identity gate read every screen as already-unified, so /// `retarget_active_tab` never ran on any of them. #[test] fn three_screen_byte_identical_nav_regression_each_screen_ends_up_with_its_own_active_tab() { @@ -182,6 +182,368 @@ fn three_screen_byte_identical_nav_regression_each_screen_ends_up_with_its_own_a } } +// --------------------------------------------------------------------- +// Nested outer -> inner bottom-nav regression (0722-1-gem postmortem). +// The real generated shape wraps the tab row in a full-width outer chrome +// surface (divider/background/padding). Both layers are nav-shaped, so the +// old pre-order `.next()` lookup selected the OUTER node for comparison. +// That node's direct children are divider + inner row, not tabs: labels +// therefore collapsed to the first nested label and glyph/metric drift was +// mistaken for an already-unified nav. +// --------------------------------------------------------------------- + +fn nested_nav_tab_json( + id_prefix: &str, + label: &str, + icon: &str, + icon_size: f64, + route: &str, + active: bool, +) -> serde_json::Value { + let color = if active { ACTIVE } else { INACTIVE }; + let mut children = vec![ + serde_json::json!({ + "type": "icon_font", "id": format!("{id_prefix}-icon"), + "iconFontName": icon, "width": icon_size, "height": icon_size, + "fill": [{"type":"solid", "color": color}] + }), + serde_json::json!({ + "type": "text", "id": format!("{id_prefix}-label"), "content": label, + "fontSize": 12, "fill": [{"type":"solid", "color": color}] + }), + ]; + if active { + children.push(serde_json::json!({ + "type": "ellipse", "id": format!("{id_prefix}-indicator"), + "width": 4, "height": 4, "fill": [{"type":"solid", "color": color}] + })); + } + serde_json::json!({ + "type": "frame", "id": format!("{id_prefix}-tab"), + "width": "fill_container", "height": 56, "layout": "vertical", + "padding": [4, 0], + "events": {"onTap": [{"replace": format!("\"{route}\"")}]}, + "children": children + }) +} + +struct NestedNavStyle<'a> { + icon_size: f64, + outer_height: f64, + outer_padding: [f64; 4], + outer_fill: &'a str, + divider_fill: &'a str, + inner_padding: [f64; 4], + inner_gap: f64, +} + +fn nested_nav_json( + id_prefix: &str, + active_label: &str, + icons: [&str; 4], + routes: [&str; 4], + style: NestedNavStyle<'_>, +) -> serde_json::Value { + let labels = ["Trips", "Explore", "Saved", "Profile"]; + let tabs = labels + .iter() + .zip(icons) + .zip(routes) + .enumerate() + .map(|(index, ((label, icon), route))| { + nested_nav_tab_json( + &format!("{id_prefix}-{index}"), + label, + icon, + style.icon_size, + route, + *label == active_label, + ) + }) + .collect::>(); + serde_json::json!({ + "type": "frame", "id": format!("{id_prefix}-outer"), + "name": "Bottom Navigation Bar", "role": "bottom-tab-bar", + "width": "fill_container", "height": style.outer_height, "layout": "vertical", + "padding": style.outer_padding, + "fill": [{"type":"solid", "color": style.outer_fill}], + "children": [ + { + "type": "rectangle", "id": format!("{id_prefix}-divider"), + "name": "Nav Divider", "width": "fill_container", "height": 1, + "fill": [{"type":"solid", "color": style.divider_fill}] + }, + { + "type": "frame", "id": format!("{id_prefix}-inner"), + "name": "Tab Bar", "role": "tab-row", "width": "fill_container", + "height": 64, "layout": "horizontal", "gap": style.inner_gap, + "padding": style.inner_padding, "children": tabs + } + ] + }) +} + +fn two_screen_nested_nav_chrome_drift_doc() -> serde_json::Value { + serde_json::json!({ + "version": "1.0", + "children": [ + screen_json( + "trips", + "Trips", + nested_nav_json( + "reference", + "Trips", + ["luggage", "compass", "bookmark", "user"], + ["/", "/explore", "/saved", "/profile"], + NestedNavStyle { + icon_size: 22.0, + outer_height: 88.0, + outer_padding: [8.0, 16.0, 8.0, 16.0], + outer_fill: "#FFFDFC", + divider_fill: "#E7E2DE", + inner_padding: [0.0, 4.0, 0.0, 4.0], + inner_gap: 8.0, + }, + ), + ), + // Labels and active destination are already correct, but this + // independently redrawn nav drifted in glyphs AND full-surface + // geometry. A label-only idempotency gate must not preserve it. + screen_json( + "saved", + "Saved", + nested_nav_json( + "drifted", + "Saved", + ["compass", "search", "heart", "user"], + [ + "/target-trips", + "/target-explore", + "/target-saved", + "/target-profile", + ], + NestedNavStyle { + icon_size: 20.0, + outer_height: 76.0, + outer_padding: [0.0, 24.0, 0.0, 24.0], + outer_fill: "#F4F4F5", + divider_fill: "#FF00AA", + inner_padding: [0.0, 12.0, 0.0, 12.0], + inner_gap: 20.0, + }, + ), + ), + ] + }) +} + +fn first_icon_name(node: &PenNode) -> Option<&str> { + if let PenNode::IconFont(icon) = node { + return Some(icon.icon_font_name.as_str()); + } + node.children()?.iter().find_map(first_icon_name) +} + +fn assert_saved_nested_nav_matches_reference(state: &op_editor_core::EditorState) { + let saved_root = find_by_id(state.active_children(), "saved").expect("Saved screen"); + let outer = saved_root + .children() + .and_then(|children| children.first()) + .expect("Saved keeps one complete outer nav surface"); + let outer_json = serde_json::to_value(outer).expect("outer nav serializes"); + assert_eq!(outer_json["height"], serde_json::json!(88.0)); + assert_eq!(outer_json["fill"][0]["color"], serde_json::json!("#FFFDFC")); + assert_eq!( + outer_json["padding"], + serde_json::json!([8.0, 16.0, 8.0, 16.0]), + "outer surface padding must come from the reference nav, not just the tab row" + ); + + let outer_children = outer.children().expect("divider + inner tab row"); + assert_eq!(outer_children.len(), 2, "copy the complete outer chrome"); + let divider_json = serde_json::to_value(&outer_children[0]).expect("divider serializes"); + assert_eq!(divider_json["height"], serde_json::json!(1.0)); + assert_eq!( + divider_json["fill"][0]["color"], + serde_json::json!("#E7E2DE") + ); + + let inner = &outer_children[1]; + assert_eq!( + labels_of(inner), + vec!["Trips", "Explore", "Saved", "Profile"] + ); + let inner_json = serde_json::to_value(inner).expect("inner tab row serializes"); + assert_eq!( + inner_json["padding"], + serde_json::json!([0.0, 4.0, 0.0, 4.0]) + ); + assert_eq!(inner_json["gap"], serde_json::json!(8.0)); + + let tabs = inner.children().expect("four tabs"); + let icons = tabs + .iter() + .map(|tab| first_icon_name(tab).expect("tab icon")) + .collect::>(); + assert_eq!(icons, vec!["luggage", "compass", "bookmark", "user"]); + for tab in tabs { + let icon = tab + .children() + .and_then(|children| children.first()) + .expect("icon is first tab child"); + let icon_json = serde_json::to_value(icon).expect("icon serializes"); + assert_eq!(icon_json["width"], serde_json::json!(22.0)); + assert_eq!(icon_json["height"], serde_json::json!(22.0)); + } + assert_eq!(first_fill(&tabs[0]).as_deref(), Some(INACTIVE)); + assert_eq!( + first_fill(&tabs[2]).as_deref(), + Some(ACTIVE), + "the cloned reference chrome must retarget active styling to Saved" + ); +} + +#[test] +fn nested_nav_same_labels_but_glyph_size_and_padding_drift_replaces_complete_outer_chrome() { + let mut state = state_from(two_screen_nested_nav_chrome_drift_doc()); + run_pass(&mut state); + assert_saved_nested_nav_matches_reference(&state); +} + +#[test] +fn nested_nav_complete_outer_sync_is_idempotent_on_second_run() { + let mut state = state_from(two_screen_nested_nav_chrome_drift_doc()); + run_pass(&mut state); + assert_saved_nested_nav_matches_reference(&state); + let once = serde_json::to_string(state.active_children()).expect("first pass snapshot"); + run_pass(&mut state); + let twice = serde_json::to_string(state.active_children()).expect("second pass snapshot"); + assert_eq!(once, twice, "nested nav sync must be a second-run no-op"); +} + +#[test] +fn nested_nav_retarget_preserves_reference_route_at_each_label_position() { + // Every reference tab is already wired to a distinct destination. The + // target's independently-authored routes deliberately disagree, so a + // passing result proves both halves of the contract: the complete nav + // came from the reference, and moving active styling from Trips to + // Saved did NOT move each tab's event along with that styling. + let mut state = state_from(two_screen_nested_nav_chrome_drift_doc()); + run_pass(&mut state); + + let saved_root = find_by_id(state.active_children(), "saved").expect("Saved screen"); + let outer = saved_root + .children() + .and_then(|children| children.first()) + .expect("complete outer nav"); + let inner = outer + .children() + .and_then(|children| children.get(1)) + .expect("inner tab row after divider"); + let tabs = inner.children().expect("four tabs"); + assert_eq!( + labels_of(inner), + vec!["Trips", "Explore", "Saved", "Profile"] + ); + + let routes = tabs + .iter() + .map(|tab| { + serde_json::to_value(tab).expect("tab serializes")["events"]["onTap"][0]["replace"] + .as_str() + .expect("replace route is a string literal") + .to_string() + }) + .collect::>(); + assert_eq!( + routes, + vec!["\"/\"", "\"/explore\"", "\"/saved\"", "\"/profile\""], + "routes belong to label positions; active-style retargeting must not swap them" + ); + assert_eq!(first_fill(&tabs[0]).as_deref(), Some(INACTIVE)); + assert_eq!(first_fill(&tabs[2]).as_deref(), Some(ACTIVE)); +} + +#[test] +fn route_only_drift_still_adopts_reference_routes() { + let style = || NestedNavStyle { + icon_size: 22.0, + outer_height: 88.0, + outer_padding: [8.0, 16.0, 8.0, 16.0], + outer_fill: "#FFFDFC", + divider_fill: "#E7E2DE", + inner_padding: [0.0, 4.0, 0.0, 4.0], + inner_gap: 8.0, + }; + let doc = serde_json::json!({ + "version": "1.0", + "children": [ + screen_json("trips", "Trips", nested_nav_json( + "reference", "Trips", ["luggage", "compass", "bookmark", "user"], + ["/", "/explore", "/saved", "/profile"], style(), + )), + screen_json("saved", "Saved", nested_nav_json( + "target", "Saved", ["luggage", "compass", "bookmark", "user"], + ["/wrong-1", "/wrong-2", "/wrong-3", "/wrong-4"], style(), + )), + ] + }); + let mut state = state_from(doc); + run_pass(&mut state); + + let saved = find_by_id(state.active_children(), "saved").unwrap(); + let tabs = saved.children().unwrap()[0].children().unwrap()[1] + .children() + .unwrap(); + let routes = tabs + .iter() + .map(|tab| serde_json::to_value(tab).unwrap()["events"]["onTap"][0]["replace"].clone()) + .collect::>(); + assert_eq!( + serde_json::Value::Array(routes), + serde_json::json!(["\"/\"", "\"/explore\"", "\"/saved\"", "\"/profile\""]) + ); +} + +#[test] +fn active_retarget_preserves_descendant_routes_by_tab_position() { + let mut nav_json = nested_nav_json( + "reference", + "Trips", + ["luggage", "compass", "bookmark", "user"], + ["/", "/explore", "/saved", "/profile"], + NestedNavStyle { + icon_size: 22.0, + outer_height: 88.0, + outer_padding: [8.0, 16.0, 8.0, 16.0], + outer_fill: "#FFFDFC", + divider_fill: "#E7E2DE", + inner_padding: [0.0, 4.0, 0.0, 4.0], + inner_gap: 8.0, + }, + ); + for tab in nav_json["children"][1]["children"].as_array_mut().unwrap() { + let events = tab.as_object_mut().unwrap().remove("events").unwrap(); + tab["children"][0]["events"] = events; + } + let mut nav: PenNode = serde_json::from_value(nav_json).unwrap(); + retarget_active_tab_at_path(&mut nav, &[1], "Saved"); + + let tabs = nav.children().unwrap()[1].children().unwrap(); + let routes = tabs + .iter() + .map(|tab| { + serde_json::to_value(&tab.children().unwrap()[0]).unwrap()["events"]["onTap"][0] + ["replace"] + .clone() + }) + .collect::>(); + assert_eq!( + serde_json::Value::Array(routes), + serde_json::json!(["\"/\"", "\"/explore\"", "\"/saved\"", "\"/profile\""]) + ); +} + // --------------------------------------------------------------------- // Detail-page Inject exemption (0718-1-k3-1 postmortem, product decision). // A push-in detail screen (back header, no corresponding bottom-nav tab) @@ -354,6 +716,20 @@ fn injected_nav_clone_gets_the_chrome_role_stamped() { ); } +#[test] +fn roleless_reference_with_stamped_clone_is_idempotent_on_second_run() { + let mut state = state_from(home_plus_empty_library_doc()); + run_pass(&mut state); + let once = serde_json::to_string(state.active_children()).expect("first pass snapshot"); + + run_pass(&mut state); + let twice = serde_json::to_string(state.active_children()).expect("second pass snapshot"); + assert_eq!( + once, twice, + "the stamped clone must compare equal to its roleless reference after canonicalization" + ); +} + #[test] fn authored_reference_nav_role_is_never_touched() { let mut state = state_from(home_plus_empty_library_doc());