fix(agent): unify complete shared navigation chrome
This commit is contained in:
Fini
parent
c9aae4dd11
commit
76b8ce43b8
|
|
@ -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<PenNode>),
|
||||
/// the target's already-matching outer chrome and actual-row path.
|
||||
RetargetActiveOnly {
|
||||
surface_id: String,
|
||||
tab_row_path: Vec<usize>,
|
||||
own_surface: Box<PenNode>,
|
||||
},
|
||||
}
|
||||
|
||||
/// 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<usize>,
|
||||
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<SyncTarget> {
|
||||
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<ContentSnapshot> {
|
||||
nav.children()
|
||||
.into_iter()
|
||||
.flatten()
|
||||
.map(snapshot_content)
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
struct CanonicalNavIdentity {
|
||||
tabs: Vec<ContentSnapshot>,
|
||||
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<String> {
|
||||
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<usize> {
|
|||
}
|
||||
|
||||
/// 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<String>,
|
||||
icon_names: Vec<String>,
|
||||
events: Vec<serde_json::Value>,
|
||||
}
|
||||
|
||||
/// 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<NodePositionSnapshot>,
|
||||
content: ContentSnapshot,
|
||||
}
|
||||
|
||||
struct NodePositionSnapshot {
|
||||
path: Vec<usize>,
|
||||
name: Option<String>,
|
||||
events: Option<jian_ops_schema::events::EventHandlers>,
|
||||
}
|
||||
|
||||
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<usize>,
|
||||
out: &mut Vec<NodePositionSnapshot>,
|
||||
) {
|
||||
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<jian_ops_schema::events::EventHandlers>) {
|
||||
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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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::<Vec<_>>();
|
||||
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::<Vec<_>>();
|
||||
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::<Vec<_>>();
|
||||
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::<Vec<_>>();
|
||||
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::<Vec<_>>();
|
||||
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());
|
||||
|
|
|
|||
Loading…
Reference in a new issue