From ea9dca5d10a5fadb0b5ee899394932a9d242f72a Mon Sep 17 00:00:00 2001 From: Fini Date: Fri, 24 Jul 2026 21:11:27 +0800 Subject: [PATCH] fix(panels): hide right rail when nothing is selected --- crates/op-editor-core/src/mutators.rs | 24 ++++-- crates/op-editor-core/src/tests_mutators.rs | 27 ++++--- .../src/widgets/property_panel.rs | 24 +----- .../src/widgets/property_panel_page.rs | 25 +----- .../src/widgets/property_panel_tests.rs | 24 +++--- crates/op-host-desktop/src/main_tests.rs | 10 +-- crates/op-host-desktop/src/persistence.rs | 4 +- .../src/widget_host/input_tests.rs | 80 +++++++++++++++++-- crates/op-host-services/src/design_session.rs | 40 ++++++++++ .../src/widget_host/overlay_press_tests.rs | 44 ++++++++++ 10 files changed, 214 insertions(+), 88 deletions(-) diff --git a/crates/op-editor-core/src/mutators.rs b/crates/op-editor-core/src/mutators.rs index a634104bf..addc517c2 100644 --- a/crates/op-editor-core/src/mutators.rs +++ b/crates/op-editor-core/src/mutators.rs @@ -149,17 +149,25 @@ impl EditorState { out } - /// Right-rail PropertyPanel visibility gate. Both tabs always have a - /// meaningful fallback: Design exposes active-page properties whenever - /// the selection is empty or stale, and Code uses the active page's - /// children. Keep this consistent with `PropertyPanel::for_selection_at` - /// so canvas geometry never disagrees with the panel that gets painted. + /// Right-rail PropertyPanel visibility gate. Design and Interact require + /// at least one live selected node. Code remains selection-independent: + /// it falls back to the active page's children. pub fn property_panel_visible(&self) -> bool { - true + if self.editor_ui.property_tab == crate::PropertyTab::Code { + return true; + } + if self.selection.set.is_empty() { + return false; + } + let children = self.active_children(); + self.selection.set.iter().any(|id| { + find_node(children, id).is_some() + || crate::instance_override::split_instance_child_anchor(id, &self.doc).is_some() + }) } - /// True when any widget occupies the right rail: currently the - /// selection inspector or active-page inspector. + /// True when a selection inspector or selection-independent Code panel + /// occupies the right rail. pub fn right_rail_visible(&self) -> bool { self.property_panel_visible() } diff --git a/crates/op-editor-core/src/tests_mutators.rs b/crates/op-editor-core/src/tests_mutators.rs index 56592317f..74ea9165c 100644 --- a/crates/op-editor-core/src/tests_mutators.rs +++ b/crates/op-editor-core/src/tests_mutators.rs @@ -840,26 +840,29 @@ fn toggle_node_collapsed_none_id_is_noop() { // --- Panel visibility predicates (Gap 4) ---------------------------- #[test] -fn property_panel_visible_falls_back_to_page_for_empty_or_stale_selection() { +fn property_panel_visible_tracks_selection() { let mut s = three_rects(); s.clear_selection(); - assert!(s.property_panel_visible()); + assert!(!s.property_panel_visible()); + s.editor_ui.property_tab = crate::PropertyTab::Interact; + assert!(!s.property_panel_visible()); + s.editor_ui.property_tab = crate::PropertyTab::Design; s.set_single_selection(NodeId::new("n1")); assert!(s.property_panel_visible()); - // A stale selection falls back to the active-page inspector. + // A selection of an id that does not resolve is not visible. s.set_single_selection(NodeId::new("nope")); - assert!(s.property_panel_visible()); + assert!(!s.property_panel_visible()); } #[test] -fn right_rail_visible_for_page_or_selection_inspector() { +fn right_rail_visible_true_on_selection_only() { let mut s = three_rects(); s.clear_selection(); - // No selection → active-page inspector. - assert!(s.right_rail_visible()); + // No selection → hidden. + assert!(!s.right_rail_visible()); // The VariablesPanel is a floating canvas overlay, not a right-rail panel. s.editor_ui.variables_panel_open = true; - assert!(s.right_rail_visible()); + assert!(!s.right_rail_visible()); s.editor_ui.variables_panel_open = false; // Selection makes it visible. s.set_single_selection(NodeId::new("n1")); @@ -870,16 +873,16 @@ fn right_rail_visible_for_page_or_selection_inspector() { fn right_rail_stays_visible_on_code_tab_without_selection() { let mut s = three_rects(); s.clear_selection(); - // Design tab + no selection → active-page properties. - assert!(s.right_rail_visible()); + // Design tab + no selection → hidden (baseline). + assert!(!s.right_rail_visible()); // The Code tab is selection-independent (TS falls back to the active // page's children), so the rail must stay open with no selection. s.editor_ui.property_tab = crate::PropertyTab::Code; assert!(s.property_panel_visible()); assert!(s.right_rail_visible()); - // Back to Design without a selection → page inspector remains. + // Back to Design without a selection → hidden again. s.editor_ui.property_tab = crate::PropertyTab::Design; - assert!(s.right_rail_visible()); + assert!(!s.right_rail_visible()); } #[test] diff --git a/crates/op-editor-ui/src/widgets/property_panel.rs b/crates/op-editor-ui/src/widgets/property_panel.rs index 0cc9bf39d..7ac2b99af 100644 --- a/crates/op-editor-ui/src/widgets/property_panel.rs +++ b/crates/op-editor-ui/src/widgets/property_panel.rs @@ -363,29 +363,7 @@ impl PropertyPanel { None, )); } - let (page_name, page_background) = match state.doc.pages.as_ref() { - Some(pages) if !pages.is_empty() => { - let index = state.ui.active_page_index.min(pages.len() - 1); - ( - pages[index].name.clone(), - pages[index].background_color.clone(), - ) - } - _ => ("Page 1".to_string(), None), - }; - let mut panel = Self::build_from_snapshot( - state, - NodeSnapshot::empty_for_code_tab(), - op_editor_core::FillType::Solid, - now_ms, - false, - None, - None, - ); - panel.page_only = true; - panel.page_name = page_name; - panel.page_background = page_background; - Some(panel) + None } /// Selection-driven panel builder — `None` when no selected id diff --git a/crates/op-editor-ui/src/widgets/property_panel_page.rs b/crates/op-editor-ui/src/widgets/property_panel_page.rs index b4fcf4f02..fec272ab9 100644 --- a/crates/op-editor-ui/src/widgets/property_panel_page.rs +++ b/crates/op-editor-ui/src/widgets/property_panel_page.rs @@ -232,7 +232,7 @@ mod tests { } #[test] - fn no_selection_page_inspector_preserves_rgba_and_hits_input_and_clear() { + fn no_selection_does_not_implicitly_open_page_inspector() { let doc = jian_ops_schema::load_str( r##"{ "version":"1.0.0", @@ -247,27 +247,6 @@ mod tests { .expect("page fixture") .value; let state = op_editor_core::EditorState::from_document(doc); - let panel = PropertyPanel::for_selection(&state).expect("page inspector"); - assert!(panel.page_only); - assert_eq!(panel.page_name, "Canvas A"); - assert_eq!(panel.page_background.as_deref(), Some("#d7e4f380")); - - let panel_rect = Rect::xywh(0.0, 0.0, 280.0, 500.0); - let input = background_input_rect(panel_rect, true); - assert_eq!( - panel.hit_test( - panel_rect, - Point2D::new(input.origin.x + 8.0, input.origin.y + 8.0) - ), - Some(PropertyFocus::PageBackgroundHex) - ); - let clear = clear_rect(panel_rect, true).unwrap(); - assert_eq!( - panel.hit_test_action( - panel_rect, - Point2D::new(clear.origin.x + 8.0, clear.origin.y + 8.0) - ), - Some(PropertyPanelAction::ClearPageBackground) - ); + assert!(PropertyPanel::for_selection(&state).is_none()); } } diff --git a/crates/op-editor-ui/src/widgets/property_panel_tests.rs b/crates/op-editor-ui/src/widgets/property_panel_tests.rs index 230ceeb7b..8dc6ac1fd 100644 --- a/crates/op-editor-ui/src/widgets/property_panel_tests.rs +++ b/crates/op-editor-ui/src/widgets/property_panel_tests.rs @@ -131,12 +131,9 @@ fn scene_aware_panel_keeps_unbounded_group_aggregate_dimensions() { } #[test] -fn for_selection_without_selection_builds_page_inspector() { +fn for_selection_without_selection_returns_none() { let state = EditorState::new(); - let panel = PropertyPanel::for_selection(&state).expect("implicit page inspector"); - assert!(panel.page_only); - assert_eq!(panel.page_name, "Page 1"); - assert_eq!(panel.page_background, None); + assert!(PropertyPanel::for_selection(&state).is_none()); } #[test] @@ -164,12 +161,19 @@ fn for_selection_code_tab_builds_panel_without_selection() { } #[test] -fn for_selection_design_tab_shows_page_inspector_without_selection() { +fn for_selection_design_tab_still_hides_panel_without_selection() { let mut state = EditorState::sample(); state.clear_selection(); state.editor_ui.property_tab = PropertyTab::Design; - let panel = PropertyPanel::for_selection(&state).expect("page inspector"); - assert!(panel.page_only); + assert!(PropertyPanel::for_selection(&state).is_none()); +} + +#[test] +fn for_selection_interact_tab_still_hides_panel_without_selection() { + let mut state = EditorState::sample(); + state.clear_selection(); + state.editor_ui.property_tab = PropertyTab::Interact; + assert!(PropertyPanel::for_selection(&state).is_none()); } #[test] @@ -202,10 +206,10 @@ fn inactive_property_tab_hover_paints_pill_background() { } #[test] -fn for_selection_with_stale_selection_falls_back_to_page_inspector() { +fn for_selection_with_stale_selection_returns_none() { let mut state = EditorState::sample(); state.set_single_selection(NodeId::new("n9999")); - assert!(PropertyPanel::for_selection(&state).is_some_and(|panel| panel.page_only)); + assert!(PropertyPanel::for_selection(&state).is_none()); } #[test] diff --git a/crates/op-host-desktop/src/main_tests.rs b/crates/op-host-desktop/src/main_tests.rs index f91d13e31..cee4f4996 100644 --- a/crates/op-host-desktop/src/main_tests.rs +++ b/crates/op-host-desktop/src/main_tests.rs @@ -168,10 +168,10 @@ fn fresh_app_fits_blank_frame_like_ts_canvas_init() { assert!(app.host.editor_state().selection.is_empty()); let v = app.host.editor_state().viewport; - // The page-properties rail stays visible for background editing; fit uses its narrower canvas. - assert!((v.zoom - 0.68).abs() < 1e-3, "zoom {}", v.zoom); + // No implicit page inspector without a selection: fit uses the full canvas width. + assert!((v.zoom - 0.8933333).abs() < 1e-3, "zoom {}", v.zoom); assert!((v.pan_x - 64.0).abs() < 1e-2, "pan_x {}", v.pan_x); - assert!((v.pan_y - 158.0).abs() < 1e-2, "pan_y {}", v.pan_y); + assert!((v.pan_y - 72.66669).abs() < 1e-2, "pan_y {}", v.pan_y); } #[test] @@ -182,9 +182,9 @@ fn fresh_app_refits_blank_frame_to_actual_window_size_once() { assert!(app.fit_initial_blank_frame_to_actual_viewport()); let v = app.host.editor_state().viewport; - assert!((v.zoom - 0.31333333).abs() < 1e-3, "zoom {}", v.zoom); + assert!((v.zoom - 0.52666664).abs() < 1e-3, "zoom {}", v.zoom); assert!((v.pan_x - 64.0).abs() < 1e-2, "pan_x {}", v.pan_x); - assert!((v.pan_y - 204.66667).abs() < 1e-2, "pan_y {}", v.pan_y); + assert!((v.pan_y - 119.33334).abs() < 1e-2, "pan_y {}", v.pan_y); app.viewport_width = 1200.0; app.viewport_height = 800.0; diff --git a/crates/op-host-desktop/src/persistence.rs b/crates/op-host-desktop/src/persistence.rs index 07f80441b..de6103dd6 100644 --- a/crates/op-host-desktop/src/persistence.rs +++ b/crates/op-host-desktop/src/persistence.rs @@ -498,9 +498,9 @@ mod tests { Some(jian_ops_schema::sizing::SizingBehavior::Number(800.0)) )); let v = host.editor_state().viewport; - assert!((v.zoom - 0.68).abs() < 1e-3, "zoom {}", v.zoom); + assert!((v.zoom - 0.8933333).abs() < 1e-3, "zoom {}", v.zoom); assert!((v.pan_x - 64.0).abs() < 1e-2, "pan_x {}", v.pan_x); - assert!((v.pan_y - 158.0).abs() < 1e-2, "pan_y {}", v.pan_y); + assert!((v.pan_y - 72.66669).abs() < 1e-2, "pan_y {}", v.pan_y); } #[test] diff --git a/crates/op-host-native/src/widget_host/input_tests.rs b/crates/op-host-native/src/widget_host/input_tests.rs index e2b9c9330..e4ef4262d 100644 --- a/crates/op-host-native/src/widget_host/input_tests.rs +++ b/crates/op-host-native/src/widget_host/input_tests.rs @@ -732,19 +732,25 @@ fn toolbar_panel_actions_open_variables_and_design_panels() { )) ); // The VariablesPanel is a floating canvas overlay, not a right-rail - // tab. With no node selected the rail remains the active-page - // inspector, so toggling Variables must not affect its visibility. - assert!(host.editor_state().right_rail_visible()); + // tab. With no node selected the rail stays hidden, so toggling + // Variables must not affect its visibility. + assert!(!host.editor_state().right_rail_visible()); + let (canvas_left, _, canvas_width, _) = host.canvas_region(viewport_w, viewport_h); + assert_eq!( + canvas_width, + viewport_w - canvas_left, + "an empty Design selection must not reserve a blank right rail" + ); assert!(host.apply_press(variables_x, variables_y, viewport_w, viewport_h)); assert!(host.editor_state().editor_ui.variables_panel_open); - assert!(host.editor_state().right_rail_visible()); + assert!(!host.editor_state().right_rail_visible()); assert_eq!( host.editor_state().editor_ui.property_tab, op_editor_core::PropertyTab::Design ); assert!(host.apply_press(variables_x, variables_y, viewport_w, viewport_h)); assert!(!host.editor_state().editor_ui.variables_panel_open); - assert!(host.editor_state().right_rail_visible()); + assert!(!host.editor_state().right_rail_visible()); let (design_x, design_y) = toolbar_action_point_for_test( &host, @@ -2578,3 +2584,67 @@ fn import_menu_routes_its_two_rows_to_figma_and_html() { assert!(host.apply_press(button_center.x, button_center.y, vw, vh)); assert!(!host.editor_state().editor_ui.import_menu_open); } + +#[test] +fn right_rail_host_routing_tracks_design_selection_and_code_fallback() { + let mut host = WidgetHostNative::new(); + seed( + &mut host, + r#"{"version":"1.0.0","children":[{"type":"rectangle","id":"n-rail","name":"Rail probe","x":0,"y":0,"width":100,"height":50}]}"#, + ); + host.editor_state_mut().chat.collapsed = true; + host.editor_state_mut().editor_ui.property_tab = op_editor_core::PropertyTab::Design; + host.editor_state_mut().clear_selection(); + + let viewport_w = 1200.0; + let viewport_h = 800.0; + let property_width = host.editor_state().editor_ui.property_panel_width; + let old_property_gutter_x = viewport_w - property_width; + let press_y = TOP_BAR_HEIGHT + 180.0; + + let (canvas_left, _, canvas_width, _) = host.canvas_region(viewport_w, viewport_h); + assert_eq!( + canvas_left + canvas_width, + viewport_w, + "empty Design selection must release the entire right rail to the canvas" + ); + assert_eq!( + host.panel_resize_hover(old_property_gutter_x, press_y, viewport_w), + None, + "the former property-panel gutter must not remain interactive" + ); + + let right_edge_x = viewport_w - 2.0; + assert!(host.over_canvas(right_edge_x, press_y, viewport_w, viewport_h)); + host.apply_press(right_edge_x, press_y, viewport_w, viewport_h); + assert!( + host.marquee_drag.is_some(), + "a blank press at the viewport's right edge must route to the canvas" + ); + assert!(!host.is_resizing_panel()); + assert!(host.apply_release_with_viewport(viewport_w, viewport_h)); + + host.editor_state_mut() + .set_single_selection(NodeId::new("n-rail")); + assert!(host.editor_state().right_rail_visible()); + let (selected_left, _, selected_width, _) = host.canvas_region(viewport_w, viewport_h); + assert_eq!(selected_left + selected_width, old_property_gutter_x); + assert!(matches!( + host.panel_resize_hover(old_property_gutter_x, press_y, viewport_w), + Some(super::PanelResizeKind::PropertyLeft) + )); + assert!(!host.over_canvas(right_edge_x, press_y, viewport_w, viewport_h)); + + host.editor_state_mut().clear_selection(); + host.editor_state_mut().editor_ui.property_tab = op_editor_core::PropertyTab::Code; + assert!( + host.editor_state().right_rail_visible(), + "Code remains selection-independent" + ); + let (code_left, _, code_width, _) = host.canvas_region(viewport_w, viewport_h); + assert_eq!(code_left + code_width, old_property_gutter_x); + assert!(matches!( + host.panel_resize_hover(old_property_gutter_x, press_y, viewport_w), + Some(super::PanelResizeKind::PropertyLeft) + )); +} diff --git a/crates/op-host-services/src/design_session.rs b/crates/op-host-services/src/design_session.rs index 44683f3e6..cd27f13b8 100644 --- a/crates/op-host-services/src/design_session.rs +++ b/crates/op-host-services/src/design_session.rs @@ -750,6 +750,46 @@ mod viewport_fit_tests { EditorState::from_document(doc) } + #[test] + fn design_canvas_size_reserves_the_right_rail_only_when_the_panel_is_visible() { + const VIEWPORT_WIDTH: f32 = 1200.0; + const VIEWPORT_HEIGHT: f32 = 800.0; + const PROPERTY_PANEL_WIDTH: f32 = 280.0; + + let mut state = state_with_root(844.0); + state.editor_ui.sidebar_open = false; + state.editor_ui.property_panel_width = PROPERTY_PANEL_WIDTH; + state.editor_ui.property_tab = op_editor_core::PropertyTab::Design; + state.clear_selection(); + + assert_eq!( + design_canvas_size(&state, VIEWPORT_WIDTH, VIEWPORT_HEIGHT), + (VIEWPORT_WIDTH, VIEWPORT_HEIGHT - TOP_BAR_HEIGHT), + "Design with no selection must use the full canvas width" + ); + + state.set_single_selection(op_editor_core::NodeId::new("root")); + assert_eq!( + design_canvas_size(&state, VIEWPORT_WIDTH, VIEWPORT_HEIGHT), + ( + VIEWPORT_WIDTH - PROPERTY_PANEL_WIDTH, + VIEWPORT_HEIGHT - TOP_BAR_HEIGHT + ), + "Design with a live selection must reserve the property rail" + ); + + state.clear_selection(); + state.editor_ui.property_tab = op_editor_core::PropertyTab::Code; + assert_eq!( + design_canvas_size(&state, VIEWPORT_WIDTH, VIEWPORT_HEIGHT), + ( + VIEWPORT_WIDTH - PROPERTY_PANEL_WIDTH, + VIEWPORT_HEIGHT - TOP_BAR_HEIGHT + ), + "Code remains selection-independent and must reserve the property rail" + ); + } + #[test] fn growth_past_the_viewport_triggers_refit_and_refit_restores_visibility() { let mut state = state_with_root(844.0); diff --git a/crates/op-host-web/src/widget_host/overlay_press_tests.rs b/crates/op-host-web/src/widget_host/overlay_press_tests.rs index 9ae9dda92..b3fd4b9e3 100644 --- a/crates/op-host-web/src/widget_host/overlay_press_tests.rs +++ b/crates/op-host-web/src/widget_host/overlay_press_tests.rs @@ -66,6 +66,50 @@ fn painted_inside(backend: &CaptureBackend, target: Rect) -> bool { }) } +#[test] +fn empty_design_selection_reclaims_property_rail_until_a_node_is_selected() { + let mut host = WidgetHost::new(); + assert!(host.editor_state.selection.is_empty()); + assert_eq!( + host.editor_state.editor_ui.property_tab, + op_editor_core::PropertyTab::Design + ); + + let property_width = host.editor_state.editor_ui.property_panel_width; + let property_rect = Rect::xywh( + W - property_width, + TOP_BAR_HEIGHT, + property_width, + H - TOP_BAR_HEIGHT, + ); + let (canvas_left, _, canvas_width, _) = host.canvas_region(W, H); + assert!( + (canvas_left + canvas_width - W).abs() < f32::EPSILON, + "an empty Design selection must let the canvas reach the viewport edge" + ); + + let mut empty_backend = CaptureBackend::default(); + host.paint_editor(&mut empty_backend, W, H); + assert!( + !painted_inside(&empty_backend, property_rect), + "an empty Design selection must not paint the PropertyPanel rail" + ); + + host.editor_state.set_single_selection(NodeId::new("n10")); + let (selected_left, _, selected_width, _) = host.canvas_region(W, H); + assert!( + (selected_left + selected_width - property_rect.origin.x).abs() < f32::EPSILON, + "a live selection must reserve exactly the PropertyPanel rail" + ); + + let mut selected_backend = CaptureBackend::default(); + host.paint_editor(&mut selected_backend, W, H); + assert!( + painted_inside(&selected_backend, property_rect), + "a live selection must paint the PropertyPanel rail" + ); +} + fn seed_layer_doc(host: &mut WidgetHost) { let doc = jian_ops_schema::load_str( r#"{"version":"1.0.0","children":[