diff --git a/crates/op-editor-core/src/editor_ui_state.rs b/crates/op-editor-core/src/editor_ui_state.rs index 553334d85..11772b74d 100644 --- a/crates/op-editor-core/src/editor_ui_state.rs +++ b/crates/op-editor-core/src/editor_ui_state.rs @@ -434,6 +434,10 @@ pub struct EditorUiState { /// capped, so a document with many colour variables scrolls inside /// it rather than stretching the inspector. Reset when it opens. pub property_color_variable_picker_scroll: jian_core::scroll::ScrollState, + /// Row slot hovered in that dropdown (`None` = none). It indexes the + /// popup's laid-out row list, so slot 0 is the leading unbind row + /// whenever a variable is bound. Cleared whenever the popup closes. + pub property_color_variable_picker_hover: Option, /// Whether the image-fill editor popover is open. pub image_fill_popover_open: bool, /// Image-fill node currently in Figma-style crop editing mode. diff --git a/crates/op-editor-core/src/editor_ui_state/defaults.rs b/crates/op-editor-core/src/editor_ui_state/defaults.rs index ee6b85d6e..1b106183e 100644 --- a/crates/op-editor-core/src/editor_ui_state/defaults.rs +++ b/crates/op-editor-core/src/editor_ui_state/defaults.rs @@ -130,6 +130,7 @@ impl Default for EditorUiState { font_import_supported: false, batch_frame_export_supported: false, property_color_variable_picker_scroll: Default::default(), + property_color_variable_picker_hover: None, system_fonts_loaded: false, missing_fonts_prompt: None, missing_fonts_modal_open: false, diff --git a/crates/op-editor-core/src/editor_ui_state/methods.rs b/crates/op-editor-core/src/editor_ui_state/methods.rs index e9265a662..5789a7d78 100644 --- a/crates/op-editor-core/src/editor_ui_state/methods.rs +++ b/crates/op-editor-core/src/editor_ui_state/methods.rs @@ -160,6 +160,19 @@ impl EditorUiState { changed } + /// Close the fill / stroke colour-variable popup, dropping the row + /// hover and list scroll with it so the next open starts clean. + /// Returns whether anything changed. + pub fn close_color_variable_picker(&mut self) -> bool { + let changed = self.property_color_variable_picker_open.is_some() + || self.property_color_variable_picker_hover.is_some() + || self.property_color_variable_picker_scroll.offset != 0.0; + self.property_color_variable_picker_open = None; + self.property_color_variable_picker_hover = None; + self.property_color_variable_picker_scroll.offset = 0.0; + changed + } + pub fn open_icon_picker(&mut self, replace_selection: bool) { self.close_icon_picker(); self.icon_picker.open = true; @@ -368,7 +381,7 @@ impl EditorUiState { self.stroke_edit_mode_anchor = String::new(); self.stroke_mode_popover_open = false; self.stroke_mode_popover_hover = None; - self.property_color_variable_picker_open = None; + self.close_color_variable_picker(); self.image_crop_editing = None; self.axis_dropdown_open = None; self.variables_theme_rename_axis = None; diff --git a/crates/op-editor-core/src/host_image_panel_transitions.rs b/crates/op-editor-core/src/host_image_panel_transitions.rs index 793f421a3..4971ee2d3 100644 --- a/crates/op-editor-core/src/host_image_panel_transitions.rs +++ b/crates/op-editor-core/src/host_image_panel_transitions.rs @@ -112,7 +112,7 @@ pub fn close_other_property_popovers_for_image(ui: &mut EditorUiState) { ui.font_weight_picker_open = false; ui.export_scale_picker_open = false; ui.export_format_picker_open = false; - ui.property_color_variable_picker_open = None; + ui.close_color_variable_picker(); ui.close_font_picker(); } diff --git a/crates/op-editor-core/src/host_ui_transitions.rs b/crates/op-editor-core/src/host_ui_transitions.rs index c9928ebf8..747e88335 100644 --- a/crates/op-editor-core/src/host_ui_transitions.rs +++ b/crates/op-editor-core/src/host_ui_transitions.rs @@ -40,7 +40,7 @@ impl EditorUiState { self.stroke_mode_popover_open = false; self.export_scale_picker_open = false; self.export_format_picker_open = false; - self.property_color_variable_picker_open = None; + self.close_color_variable_picker(); } } diff --git a/crates/op-editor-ui/src/widgets/cursor_hover_flow.rs b/crates/op-editor-ui/src/widgets/cursor_hover_flow.rs index b36a01e26..6f293c766 100644 --- a/crates/op-editor-ui/src/widgets/cursor_hover_flow.rs +++ b/crates/op-editor-ui/src/widgets/cursor_hover_flow.rs @@ -164,6 +164,40 @@ pub fn property_base_hover( changed } +/// Fill / stroke colour-variable popup hover. Returns +/// `(over_popup, changed)`: the popup owns every point on its chrome, so +/// a caller seeing `over_popup` must consume the move and clear the +/// hover painted beneath it instead of letting the rail light up through +/// the overlay. `panel` is `None` when the rail can't be probed for this +/// move, which clears the row hover rather than freezing it. +pub fn color_variable_picker_hover( + state: &mut EditorState, + panel: Option<&PropertyPanel>, + property_rect: Rect, + point: Point2D, +) -> (bool, bool) { + if state + .editor_ui + .property_color_variable_picker_open + .is_none() + { + return (false, false); + } + let (over_popup, new_hover) = panel + .map(|panel| { + ( + panel.color_variable_picker_contains(property_rect, point), + panel.color_variable_picker_row_at(property_rect, point), + ) + }) + .unwrap_or((false, None)); + let changed = new_hover != state.editor_ui.property_color_variable_picker_hover; + if changed { + state.editor_ui.property_color_variable_picker_hover = new_hover; + } + (over_popup, changed) +} + /// Code-panel hover wash. Reuses the Code panel's click geometry so /// framework chips, scroll chevrons, and body actions share hit-testing /// with paint. `eligible` folds in each host's own suppression gates diff --git a/crates/op-editor-ui/src/widgets/press_flow.rs b/crates/op-editor-ui/src/widgets/press_flow.rs index 2305e01de..453eaf114 100644 --- a/crates/op-editor-ui/src/widgets/press_flow.rs +++ b/crates/op-editor-ui/src/widgets/press_flow.rs @@ -130,6 +130,14 @@ pub fn press_color_variable_picker( ) -> PropertyOverlayPress { let property_rect = property_panel_rect(state, viewport_width, viewport_height); if let Some(panel) = PropertyPanel::for_selection(state) { + // The popup's own rows first: they are painted over the rail and + // carry the list scroll, which the panel's ordinary control walk + // knows nothing about. Without this the row press fell through to + // the `contains` swallow below and the click was eaten. + if let Some(action) = panel.color_variable_picker_action_at(property_rect, point) { + return PropertyOverlayPress::Action(action); + } + // The `{}` trigger itself sits outside the popup and toggles it. if let Some(action) = panel.hit_test_action(property_rect, point) { if matches!( action, @@ -146,7 +154,7 @@ pub fn press_color_variable_picker( return PropertyOverlayPress::Swallow; } } - state.editor_ui.property_color_variable_picker_open = None; + state.editor_ui.close_color_variable_picker(); PropertyOverlayPress::Dismissed } diff --git a/crates/op-editor-ui/src/widgets/property_panel.rs b/crates/op-editor-ui/src/widgets/property_panel.rs index 876470992..866680131 100644 --- a/crates/op-editor-ui/src/widgets/property_panel.rs +++ b/crates/op-editor-ui/src/widgets/property_panel.rs @@ -147,6 +147,10 @@ pub struct PropertyPanel { /// list. The popup is height-capped, so a long variable set scrolls /// inside it instead of stretching the inspector. pub color_variable_picker_scroll: f32, + /// Row slot of the open colour-variable popup under the cursor + /// (`None` = none), mirrored from + /// `editor_ui.property_color_variable_picker_hover`. + pub color_variable_picker_hover: Option, pub color_variables: Vec, pub fill_variable_ref: Option, pub stroke_variable_ref: Option, diff --git a/crates/op-editor-ui/src/widgets/property_panel/build.rs b/crates/op-editor-ui/src/widgets/property_panel/build.rs index 30af3c57d..2264e4fc6 100644 --- a/crates/op-editor-ui/src/widgets/property_panel/build.rs +++ b/crates/op-editor-ui/src/widgets/property_panel/build.rs @@ -297,6 +297,7 @@ impl PropertyPanel { Self { id: WidgetId::new(2000), color_variable_picker_scroll: ui.property_color_variable_picker_scroll.offset.max(0.0), + color_variable_picker_hover: ui.property_color_variable_picker_hover, snapshot, theme: theme_for(ui), page_only: false, diff --git a/crates/op-editor-ui/src/widgets/property_panel/paint.rs b/crates/op-editor-ui/src/widgets/property_panel/paint.rs index cac745fed..93256b7a5 100644 --- a/crates/op-editor-ui/src/widgets/property_panel/paint.rs +++ b/crates/op-editor-ui/src/widgets/property_panel/paint.rs @@ -499,6 +499,7 @@ impl Widget for PropertyPanel { &layout, &self.color_variables, self.bound_color_variable_ref(), + self.color_variable_picker_hover, self.locale, ); } diff --git a/crates/op-editor-ui/src/widgets/property_panel_color_variables.rs b/crates/op-editor-ui/src/widgets/property_panel_color_variables.rs index d3c6533af..14b27b702 100644 --- a/crates/op-editor-ui/src/widgets/property_panel_color_variables.rs +++ b/crates/op-editor-ui/src/widgets/property_panel_color_variables.rs @@ -92,13 +92,17 @@ impl ColorVariablePickerLayout { /// Row under `point`, or `None` when the point is outside the list /// or over a row scrolled out of view. pub fn row_at(&self, point: Point2D) -> Option { + self.row_slot_at(point).map(|slot| self.rows[slot].0) + } + + /// Index into [`Self::rows`] of the row under `point` — the key the + /// hosts retain as hover state, so paint and hit-test agree on which + /// row is lit without re-deriving the leading unbind offset. + pub fn row_slot_at(&self, point: Point2D) -> Option { if !self.viewport.contains(point) { return None; } - self.rows - .iter() - .find(|(_, rect)| rect.contains(point)) - .map(|(row, _)| *row) + self.rows.iter().position(|(_, rect)| rect.contains(point)) } /// Whether `point` is anywhere on the popup — the host swallows @@ -177,13 +181,15 @@ pub fn color_variable_picker_layout( } /// Paint the popup for an already-resolved layout. `current` is the -/// bound variable name for the picker's target, if any. +/// bound variable name for the picker's target, if any; `hover` is the +/// row slot under the cursor (see [`ColorVariablePickerLayout::row_slot_at`]). pub fn paint_color_variable_picker( cx: &mut PaintCx<'_>, theme: &Theme, layout: &ColorVariablePickerLayout, variables: &[ColorVariableOption], current: Option<&str>, + hover: Option, locale: op_editor_core::Locale, ) { cx.backend @@ -194,19 +200,20 @@ pub fn paint_color_variable_picker( cx.backend.save(); cx.backend.clip_rect(layout.viewport); let viewport_bottom = layout.viewport.origin.y + layout.viewport.size.y; - for (row, rect) in &layout.rows { + for (slot, (row, rect)) in layout.rows.iter().enumerate() { if rect.origin.y + rect.size.y < layout.viewport.origin.y || rect.origin.y > viewport_bottom { continue; } + let hovered = hover == Some(slot); match row { - ColorVariableRow::Unbind => paint_unbind_row(cx, theme, locale, *rect), + ColorVariableRow::Unbind => paint_unbind_row(cx, theme, locale, hovered, *rect), ColorVariableRow::Variable(index) => { let Some(variable) = variables.get(*index) else { continue; }; let active = current == Some(variable.name.as_str()); - paint_variable_row(cx, theme, variable, active, *rect); + paint_variable_row(cx, theme, variable, active, hovered, *rect); } } } @@ -256,9 +263,13 @@ fn paint_unbind_row( cx: &mut PaintCx<'_>, theme: &Theme, locale: op_editor_core::Locale, + hovered: bool, row: Rect, ) { let inner = row_inner(row); + if hovered { + cx.backend.fill_round_rect(inner, ROW_RADIUS, theme.muted); + } draw_icon( cx.backend, Icon::Close, @@ -292,12 +303,17 @@ fn paint_variable_row( theme: &Theme, variable: &ColorVariableOption, active: bool, + hovered: bool, row: Rect, ) { let inner = row_inner(row); + // The bound row's tint wins over the hover wash, the way the export + // and effect-add popups resolve the same overlap. if active { cx.backend .fill_round_rect(inner, ROW_RADIUS, theme.row_selected_primary); + } else if hovered { + cx.backend.fill_round_rect(inner, ROW_RADIUS, theme.muted); } let swatch = Rect { origin: Point2D::new( diff --git a/crates/op-editor-ui/src/widgets/property_panel_color_variables_tests.rs b/crates/op-editor-ui/src/widgets/property_panel_color_variables_tests.rs index b964ac4c8..15f350984 100644 --- a/crates/op-editor-ui/src/widgets/property_panel_color_variables_tests.rs +++ b/crates/op-editor-ui/src/widgets/property_panel_color_variables_tests.rs @@ -7,6 +7,7 @@ //! tests pin both fixes: the popup is an overlay (it must never move any //! other panel control), and its two text columns can never overlap. +use super::press_flow::PropertyOverlayPress; use super::property_panel::{PropertyPanel, PropertyPanelAction}; use super::property_panel_color_variables::{ color_variable_picker_layout, row_hex_rect, row_name_budget, row_name_rect, @@ -453,3 +454,254 @@ fn empty_variable_set_has_no_popup() { ) .is_none()); } + +// ─── Press + hover routing ───────────────────────────────────────────── +// +// The popup's rows used to be unreachable: the host routed its presses +// through the panel's ordinary control walk, which knows nothing about +// the overlay's scrolled row rects, so every row click fell into the +// `contains` swallow and was eaten. Hover had no state at all, so the +// controls painted underneath lit up through the popup. + +const VIEWPORT_W: f32 = 1280.0; +const VIEWPORT_H: f32 = 740.0; + +/// The rail rect the shared press / hover flows derive themselves. +fn press_panel_rect(state: &EditorState) -> Rect { + crate::widgets::press_flow::property_panel_rect(state, VIEWPORT_W, VIEWPORT_H) +} + +fn press(state: &mut EditorState, point: Point2D) -> PropertyOverlayPress { + crate::widgets::press_flow::press_color_variable_picker(state, VIEWPORT_W, VIEWPORT_H, point) +} + +/// Run the picked action through the shared property dispatch, the way +/// both hosts' `finish_property_overlay_press` does. +fn dispatch(state: &mut EditorState, action: &PropertyPanelAction) { + use crate::widgets::property_panel_dispatch as dispatch; + let mut image_adjustment_drag = None; + let mut effect_radius_drag = None; + let _ = dispatch::apply_property_action( + state, + action, + dispatch::PropertyActionContext { + now_ms: 0, + resolved_sizing_fallback: None, + image_adjustment_drag: &mut image_adjustment_drag, + effect_radius_drag: &mut effect_radius_drag, + }, + ); +} + +fn hover(state: &mut EditorState, rect: Rect, point: Point2D) -> (bool, bool) { + let panel = PropertyPanel::for_selection(state).expect("rectangle panel"); + crate::widgets::cursor_hover_flow::color_variable_picker_hover(state, Some(&panel), rect, point) +} + +/// A press on a variable row binds it and the popup closes behind the +/// selection. +#[test] +fn press_on_variable_row_binds_and_closes() { + let mut state = state_with_variables(6); + state.editor_ui.property_color_variable_picker_open = Some(ColorTarget::Fill); + let rect = press_panel_rect(&state); + let layout = open_layout(&state, rect); + let (row, row_rect) = layout.rows[2]; + assert_eq!(row, ColorVariableRow::Variable(2)); + + let action = match press(&mut state, row_center(row_rect)) { + PropertyOverlayPress::Action(action) => action, + other => panic!("row press must dispatch an action, got {other:?}"), + }; + assert_eq!( + action, + PropertyPanelAction::BindColorVariable { + target: ColorTarget::Fill, + index: 2, + } + ); + + dispatch(&mut state, &action); + assert_eq!(state.editor_ui.property_color_variable_picker_open, None); + assert_eq!( + state.editor_ui.property_color_variable_picker_scroll.offset, 0.0, + "closing must reset the list scroll" + ); + let panel = PropertyPanel::for_selection(&state).expect("rectangle panel"); + assert_eq!( + panel.fill_variable_ref.as_deref(), + Some("color-border-subtle-02"), + "the pressed row is the variable that got bound" + ); +} + +/// Scrolling the list moves which variable a press at a given screen +/// point binds — the row rects carry the offset and the press must read +/// the same geometry paint does. +#[test] +fn press_after_scroll_binds_the_scrolled_row() { + let mut state = state_with_variables(40); + state.editor_ui.property_color_variable_picker_open = Some(ColorTarget::Fill); + state.editor_ui.property_color_variable_picker_scroll.offset = COLOR_VARIABLE_MENU_ROW_H * 3.0; + let rect = press_panel_rect(&state); + let layout = open_layout(&state, rect); + let (row, row_rect) = *layout + .rows + .iter() + .find(|(_, r)| r.origin.y >= layout.viewport.origin.y) + .expect("some row is still visible"); + assert!( + matches!(row, ColorVariableRow::Variable(index) if index >= 3), + "three rows of scroll should reveal a later variable, got {row:?}" + ); + + match press(&mut state, row_center(row_rect)) { + PropertyOverlayPress::Action(PropertyPanelAction::BindColorVariable { target, index }) => { + assert_eq!(target, ColorTarget::Fill); + assert_eq!(ColorVariableRow::Variable(index), row); + } + other => panic!("scrolled row press must bind that row, got {other:?}"), + } +} + +/// The leading unbind row resolves the binding back to a concrete +/// colour and closes the popup. +#[test] +fn press_on_unbind_row_unbinds() { + let mut state = state_with_variables(4); + assert!(state.bind_selected_color_variable(ColorTarget::Fill, "color-border-subtle-01")); + state.editor_ui.property_color_variable_picker_open = Some(ColorTarget::Fill); + let rect = press_panel_rect(&state); + let layout = open_layout(&state, rect); + assert_eq!(layout.rows[0].0, ColorVariableRow::Unbind); + + let action = match press(&mut state, row_center(layout.rows[0].1)) { + PropertyOverlayPress::Action(action) => action, + other => panic!("unbind row press must dispatch an action, got {other:?}"), + }; + assert_eq!( + action, + PropertyPanelAction::UnbindColorVariable(ColorTarget::Fill) + ); + + dispatch(&mut state, &action); + assert_eq!(state.editor_ui.property_color_variable_picker_open, None); + let panel = PropertyPanel::for_selection(&state).expect("rectangle panel"); + assert_eq!(panel.fill_variable_ref, None); +} + +/// A press on the popup's own padding is swallowed — the popup stays +/// open and nothing is bound. +#[test] +fn press_on_popup_padding_is_swallowed() { + let mut state = state_with_variables(6); + state.editor_ui.property_color_variable_picker_open = Some(ColorTarget::Fill); + let rect = press_panel_rect(&state); + let layout = open_layout(&state, rect); + let padding = Point2D::new(row_center(layout.popup).x, layout.popup.origin.y + 1.0); + assert!( + layout.row_at(padding).is_none(), + "fixture point must be popup chrome, not a row" + ); + + assert_eq!(press(&mut state, padding), PropertyOverlayPress::Swallow); + assert_eq!( + state.editor_ui.property_color_variable_picker_open, + Some(ColorTarget::Fill), + "chrome presses keep the popup open" + ); +} + +/// A press outside the popup dismisses it. +#[test] +fn press_outside_dismisses_and_clears() { + let mut state = state_with_variables(6); + state.editor_ui.property_color_variable_picker_open = Some(ColorTarget::Fill); + state.editor_ui.property_color_variable_picker_hover = Some(1); + let rect = press_panel_rect(&state); + let layout = open_layout(&state, rect); + let outside = Point2D::new(layout.popup.origin.x - 20.0, row_center(layout.popup).y); + + assert_eq!(press(&mut state, outside), PropertyOverlayPress::Dismissed); + assert_eq!(state.editor_ui.property_color_variable_picker_open, None); + assert_eq!(state.editor_ui.property_color_variable_picker_hover, None); +} + +/// A cursor over a row lights that row and reports that the popup owns +/// the point, so the host consumes the move instead of letting the rail +/// underneath hover. +#[test] +fn hover_over_row_sets_row_hover_and_owns_point() { + let mut state = state_with_variables(6); + state.editor_ui.property_color_variable_picker_open = Some(ColorTarget::Fill); + let rect = press_panel_rect(&state); + let layout = open_layout(&state, rect); + + let (over_popup, changed) = hover(&mut state, rect, row_center(layout.rows[1].1)); + assert!(over_popup, "a row press point is on the popup"); + assert!(changed); + assert_eq!( + state.editor_ui.property_color_variable_picker_hover, + Some(1) + ); + + // Re-entering the same row is not a repaint. + let (over_popup, changed) = hover(&mut state, rect, row_center(layout.rows[1].1)); + assert!(over_popup); + assert!(!changed); +} + +/// Moving off the popup drops the row hover and hands the move back to +/// the surfaces below. +#[test] +fn hover_outside_popup_clears_row_hover() { + let mut state = state_with_variables(6); + state.editor_ui.property_color_variable_picker_open = Some(ColorTarget::Fill); + state.editor_ui.property_color_variable_picker_hover = Some(1); + let rect = press_panel_rect(&state); + let layout = open_layout(&state, rect); + let outside = Point2D::new(layout.popup.origin.x - 20.0, row_center(layout.popup).y); + + let (over_popup, changed) = hover(&mut state, rect, outside); + assert!(!over_popup); + assert!(changed); + assert_eq!(state.editor_ui.property_color_variable_picker_hover, None); +} + +/// Popup chrome owns the point without lighting any row. +#[test] +fn hover_over_popup_padding_owns_point_without_a_row() { + let mut state = state_with_variables(6); + state.editor_ui.property_color_variable_picker_open = Some(ColorTarget::Fill); + state.editor_ui.property_color_variable_picker_hover = Some(1); + let rect = press_panel_rect(&state); + let layout = open_layout(&state, rect); + let padding = Point2D::new(row_center(layout.popup).x, layout.popup.origin.y + 1.0); + + let (over_popup, changed) = hover(&mut state, rect, padding); + assert!(over_popup); + assert!(changed); + assert_eq!(state.editor_ui.property_color_variable_picker_hover, None); +} + +/// A closed popup is inert for hover, and closing drops the retained +/// row hover with the scroll. +#[test] +fn closing_the_popup_clears_hover_and_scroll() { + let mut state = state_with_variables(6); + state.editor_ui.property_color_variable_picker_open = Some(ColorTarget::Fill); + state.editor_ui.property_color_variable_picker_hover = Some(2); + state.editor_ui.property_color_variable_picker_scroll.offset = COLOR_VARIABLE_MENU_ROW_H; + + assert!(state.editor_ui.close_color_variable_picker()); + assert_eq!(state.editor_ui.property_color_variable_picker_hover, None); + assert_eq!( + state.editor_ui.property_color_variable_picker_scroll.offset, + 0.0 + ); + + let rect = press_panel_rect(&state); + let (over_popup, changed) = hover(&mut state, rect, Point2D::new(1100.0, 300.0)); + assert!(!over_popup); + assert!(!changed, "a closed popup never reports a hover change"); +} diff --git a/crates/op-editor-ui/src/widgets/property_panel_dispatch.rs b/crates/op-editor-ui/src/widgets/property_panel_dispatch.rs index 82d47aafc..b7582997c 100644 --- a/crates/op-editor-ui/src/widgets/property_panel_dispatch.rs +++ b/crates/op-editor-ui/src/widgets/property_panel_dispatch.rs @@ -306,7 +306,7 @@ pub fn apply_property_action( ui.image_fill_popover_open = false; ui.close_font_picker(); ui.font_weight_picker_open = false; - ui.property_color_variable_picker_open = None; + ui.close_color_variable_picker(); Handled } A::SetFillType { index, fill_type } => { @@ -314,7 +314,7 @@ pub fn apply_property_action( let ui = &mut state.editor_ui; ui.close_fill_type_picker(); ui.image_fill_popover_open = false; - ui.property_color_variable_picker_open = None; + ui.close_color_variable_picker(); Handled } A::AddFill => { @@ -330,7 +330,7 @@ pub fn apply_property_action( let ui = &mut state.editor_ui; ui.close_fill_type_picker(); ui.image_fill_popover_open = false; - ui.property_color_variable_picker_open = None; + ui.close_color_variable_picker(); Handled } A::AddGradientStop => { @@ -349,7 +349,7 @@ pub fn apply_property_action( ui.font_weight_picker_open = false; ui.export_scale_picker_open = false; ui.export_format_picker_open = false; - ui.property_color_variable_picker_open = None; + ui.close_color_variable_picker(); if ui.image_fill_popover_open { FollowUp(F::EnterImageCropEdit) } else { @@ -388,7 +388,7 @@ pub fn apply_property_action( ui.font_weight_picker_open = false; ui.export_scale_picker_open = false; ui.export_format_picker_open = false; - ui.property_color_variable_picker_open = None; + ui.close_color_variable_picker(); Handled } A::SetTextAlign(value) => { @@ -412,7 +412,7 @@ pub fn apply_property_action( ui.image_fill_popover_open = false; ui.export_scale_picker_open = false; ui.export_format_picker_open = false; - ui.property_color_variable_picker_open = None; + ui.close_color_variable_picker(); if opening { FollowUp(F::EnsureSystemFontsLoaded) } else { @@ -457,7 +457,7 @@ pub fn apply_property_action( ui.image_fill_popover_open = false; ui.export_scale_picker_open = false; ui.export_format_picker_open = false; - ui.property_color_variable_picker_open = None; + ui.close_color_variable_picker(); Handled } A::SetFontWeight(choice) => { @@ -478,7 +478,7 @@ pub fn apply_property_action( ui.image_fill_popover_open = false; ui.export_scale_picker_open = false; ui.export_format_picker_open = false; - ui.property_color_variable_picker_open = None; + ui.close_color_variable_picker(); Handled } A::SetPaddingMode(mode) => { @@ -505,7 +505,7 @@ pub fn apply_property_action( ui.image_fill_popover_open = false; ui.export_scale_picker_open = false; ui.export_format_picker_open = false; - ui.property_color_variable_picker_open = None; + ui.close_color_variable_picker(); Handled } A::SetStrokeMode(mode) => { @@ -520,13 +520,13 @@ pub fn apply_property_action( } A::OpenColorPicker(target) => { // Fallback anchor when called outside the press path. - state.editor_ui.property_color_variable_picker_open = None; + state.editor_ui.close_color_variable_picker(); let _ = state.open_color_picker(color_target(*target), 0.0); Handled } A::OpenFillColorPicker(index) => { // Fallback anchor when called outside the press path. - state.editor_ui.property_color_variable_picker_open = None; + state.editor_ui.close_color_variable_picker(); let _ = state.open_color_picker_for_fill( op_editor_core::ui_draft::ColorTarget::Fill, *index, @@ -537,14 +537,12 @@ pub fn apply_property_action( A::ToggleColorVariablePicker(target) => { let target = color_target(*target); let ui = &mut state.editor_ui; - ui.property_color_variable_picker_open = - if ui.property_color_variable_picker_open == Some(target) { - None - } else { - // Each open starts at the top of the list. - ui.property_color_variable_picker_scroll.offset = 0.0; - Some(target) - }; + let reopening = ui.property_color_variable_picker_open != Some(target); + // Each open starts at the top of the list, un-hovered. + ui.close_color_variable_picker(); + if reopening { + ui.property_color_variable_picker_open = Some(target); + } ui.close_fill_type_picker(); ui.image_fill_popover_open = false; ui.close_font_picker(); @@ -558,13 +556,13 @@ pub fn apply_property_action( state.commit_history(); let _ = state.bind_selected_color_variable(color_target(*target), &name); } - state.editor_ui.property_color_variable_picker_open = None; + state.editor_ui.close_color_variable_picker(); Handled } A::UnbindColorVariable(target) => { state.commit_history(); let _ = state.unbind_selected_color_variable(color_target(*target)); - state.editor_ui.property_color_variable_picker_open = None; + state.editor_ui.close_color_variable_picker(); Handled } A::ToggleExportScalePicker => { @@ -574,7 +572,7 @@ pub fn apply_property_action( ui.close_font_picker(); ui.font_weight_picker_open = false; ui.export_picker_hover = None; - ui.property_color_variable_picker_open = None; + ui.close_color_variable_picker(); Handled } A::ToggleExportFormatPicker => { @@ -584,7 +582,7 @@ pub fn apply_property_action( ui.close_font_picker(); ui.font_weight_picker_open = false; ui.export_picker_hover = None; - ui.property_color_variable_picker_open = None; + ui.close_color_variable_picker(); Handled } A::SetExportScale(scale) => { @@ -688,7 +686,7 @@ pub fn apply_property_action( ui.font_weight_picker_open = false; ui.export_scale_picker_open = false; ui.export_format_picker_open = false; - ui.property_color_variable_picker_open = None; + ui.close_color_variable_picker(); Handled } A::SetInteractionNavigate { path } => { diff --git a/crates/op-editor-ui/src/widgets/property_panel_overlay_hit.rs b/crates/op-editor-ui/src/widgets/property_panel_overlay_hit.rs index a9975780f..ceacc65ed 100644 --- a/crates/op-editor-ui/src/widgets/property_panel_overlay_hit.rs +++ b/crates/op-editor-ui/src/widgets/property_panel_overlay_hit.rs @@ -198,6 +198,14 @@ impl PropertyPanel { }) } + /// Row slot under `point` for hover tracking — the same scrolled + /// geometry [`Self::color_variable_picker_action_at`] presses + /// against, so a hovered row is always a clickable one. + pub fn color_variable_picker_row_at(&self, panel_rect: Rect, point: Point2D) -> Option { + self.color_variable_picker_layout(panel_rect)? + .row_slot_at(point) + } + /// Scroll range of the popup's list (host wheel handler clamp). pub fn color_variable_picker_max_scroll(&self, panel_rect: Rect) -> f32 { self.color_variable_picker_layout(panel_rect) diff --git a/crates/op-host-native/src/widget_host/cursor_move_overlays.rs b/crates/op-host-native/src/widget_host/cursor_move_overlays.rs index 2ef618944..f1cb6ab7c 100644 --- a/crates/op-host-native/src/widget_host/cursor_move_overlays.rs +++ b/crates/op-host-native/src/widget_host/cursor_move_overlays.rs @@ -135,6 +135,11 @@ impl WidgetHostNative { || self.editor_state.editor_ui.stroke_mode_popover_open || self.editor_state.editor_ui.font_weight_picker_open || self.editor_state.editor_ui.font_picker.open + || self + .editor_state + .editor_ui + .property_color_variable_picker_open + .is_some() || self.editor_state.editor_ui.image_fill_popover_open || self.editor_state.editor_ui.image_panel.search_open || self.editor_state.editor_ui.image_panel.generate_open; @@ -262,6 +267,30 @@ impl WidgetHostNative { return Some(true); } } + // Fill / stroke colour-variable popup row hover (no-op when + // closed). Same press order as `press_property_overlay_tiers`: + // this popup sits above the padding / font popovers below it. + let (over_color_variable_popup, color_variable_hover_changed) = + hover_flow::color_variable_picker_hover( + &mut self.editor_state, + property_panel, + property_rect, + point, + ); + if color_variable_hover_changed { + self.mark_dirty(); + } + if over_color_variable_popup { + let below_changed = self.clear_chat_and_lower_hover(); + return Some(color_variable_hover_changed || below_changed); + } + if color_variable_hover_changed { + if chat_or_picker_owns_point { + ctx.upper_hover_changed = true; + } else { + return Some(true); + } + } // Padding-mode gear popover row hover (no-op when closed). let over_padding_or_stroke_popup = property_panel.is_some_and(|panel| { (self.editor_state.editor_ui.padding_mode_popover_open diff --git a/crates/op-host-native/src/widget_host/keyboard_escape.rs b/crates/op-host-native/src/widget_host/keyboard_escape.rs index b02fc76a4..02b56a016 100644 --- a/crates/op-host-native/src/widget_host/keyboard_escape.rs +++ b/crates/op-host-native/src/widget_host/keyboard_escape.rs @@ -264,13 +264,9 @@ impl WidgetHostNative { .editor_state .editor_ui .property_color_variable_picker_open - .take() .is_some() { - self.editor_state - .editor_ui - .property_color_variable_picker_scroll - .offset = 0.0; + self.editor_state.editor_ui.close_color_variable_picker(); self.mark_dirty(); return true; } diff --git a/crates/op-host-native/src/widget_host/overlay_cursor_tests.rs b/crates/op-host-native/src/widget_host/overlay_cursor_tests.rs index 4e74c8f74..e45356a86 100644 --- a/crates/op-host-native/src/widget_host/overlay_cursor_tests.rs +++ b/crates/op-host-native/src/widget_host/overlay_cursor_tests.rs @@ -103,3 +103,57 @@ fn topmost_design_panel_cursor_move_clears_stale_path_anchor_menu_hover() { "stale lower-menu hover should clear under the topmost panel" ); } + +/// The colour-variable popup used to have no hover state at all, so a +/// cursor over its rows highlighted whatever inspector control sat +/// underneath. The popup now owns every point on its chrome: its own row +/// lights up and the rail's stale wash is dropped. +#[test] +fn color_variable_popup_owns_hover_instead_of_the_rail_underneath() { + use jian_ops_schema::variable::{VariableKind, VariableScalar}; + use op_editor_core::{ColorTarget, EditorState}; + use op_editor_ui::widgets::{press_flow, PropertyPanel}; + + let mut host = WidgetHostNative::new(); + host.last_viewport_w = VIEWPORT_W; + host.last_viewport_h = VIEWPORT_H; + *host.editor_state_mut() = EditorState::sample(); + host.editor_state_mut() + .set_single_selection(NodeId::new("n13")); + for i in 0..6 { + assert!(host.editor_state_mut().create_variable( + &format!("color-surface-{i:02}"), + VariableKind::Color, + VariableScalar::Str("#DBD8CB".into()), + )); + } + let ui = &mut host.editor_state_mut().editor_ui; + ui.property_color_variable_picker_open = Some(ColorTarget::Fill); + // A wash left over from the inspector row the popup now covers. + ui.property_action_hover = Some(0); + + let rect = press_flow::property_panel_rect(host.editor_state(), VIEWPORT_W, VIEWPORT_H); + let layout = PropertyPanel::for_selection(host.editor_state()) + .expect("rectangle panel") + .color_variable_picker_layout(rect) + .expect("open picker lays out"); + let row = layout.rows[1].1; + let point = Point2D::new( + row.origin.x + row.size.x / 2.0, + row.origin.y + row.size.y / 2.0, + ); + + assert!(host.apply_cursor_move(point.x, point.y)); + assert_eq!( + host.editor_state() + .editor_ui + .property_color_variable_picker_hover, + Some(1), + "the row under the cursor must light up" + ); + assert_eq!( + host.editor_state().editor_ui.property_action_hover, + None, + "hover must not pass through the popup to the rail underneath" + ); +} diff --git a/crates/op-host-native/src/widget_host/press_property_tiers.rs b/crates/op-host-native/src/widget_host/press_property_tiers.rs index 13f12c72f..7fc7372d2 100644 --- a/crates/op-host-native/src/widget_host/press_property_tiers.rs +++ b/crates/op-host-native/src/widget_host/press_property_tiers.rs @@ -253,9 +253,7 @@ impl WidgetHostNative { { // Non-primary fill swatch — bind the picker to this // fill so HSV writes back to `fills[index]`. - self.editor_state - .editor_ui - .property_color_variable_picker_open = None; + self.editor_state.editor_ui.close_color_variable_picker(); let _ = self.editor_state.open_color_picker_for_fill( op_editor_core::ui_draft::ColorTarget::Fill, index, diff --git a/crates/op-host-web/src/widget_host/cursor_input.rs b/crates/op-host-web/src/widget_host/cursor_input.rs index b3dfdd2ba..376f7cba1 100644 --- a/crates/op-host-web/src/widget_host/cursor_input.rs +++ b/crates/op-host-web/src/widget_host/cursor_input.rs @@ -108,6 +108,11 @@ impl WidgetHost { || self.editor_state.editor_ui.stroke_mode_popover_open || self.editor_state.editor_ui.font_weight_picker_open || self.editor_state.editor_ui.font_picker.open + || self + .editor_state + .editor_ui + .property_color_variable_picker_open + .is_some() || self.editor_state.editor_ui.image_fill_popover_open || self.editor_state.editor_ui.image_panel.search_open || self.editor_state.editor_ui.image_panel.generate_open; @@ -249,6 +254,15 @@ impl WidgetHost { } } } + if let Some(consumed) = self.color_variable_picker_hover_tier( + panel, + property_rect, + point, + chat_or_picker_surface_owns_point, + &mut upper_hover_changed, + ) { + return consumed; + } if self.editor_state.editor_ui.padding_mode_popover_open || self.editor_state.editor_ui.stroke_mode_popover_open { diff --git a/crates/op-host-web/src/widget_host/hover_property_panel.rs b/crates/op-host-web/src/widget_host/hover_property_panel.rs index dc7429624..03979d1b4 100644 --- a/crates/op-host-web/src/widget_host/hover_property_panel.rs +++ b/crates/op-host-web/src/widget_host/hover_property_panel.rs @@ -67,3 +67,44 @@ impl WidgetHost { changed } } + +impl WidgetHost { + /// Fill / stroke colour-variable popup hover tier of + /// `apply_cursor_move`. Same press order as + /// `press_property_overlay_tiers`: this popup sits above the padding + /// and font popovers below it, so it gets first refusal on the point. + /// + /// `Some(consumed)` ends the move; `None` falls through to the next + /// popover. + pub(in crate::widget_host) fn color_variable_picker_hover_tier( + &mut self, + panel: &op_editor_ui::widgets::PropertyPanel, + property_rect: op_editor_ui::Rect, + point: op_editor_ui::Point2D, + chat_or_picker_surface_owns_point: bool, + upper_hover_changed: &mut bool, + ) -> Option { + let (over_popup, hover_changed) = + op_editor_ui::widgets::cursor_hover_flow::color_variable_picker_hover( + &mut self.editor_state, + Some(panel), + property_rect, + point, + ); + if hover_changed { + self.mark_dirty(); + } + if over_popup { + self.clear_chat_and_lower_hover(); + return Some(true); + } + if hover_changed { + if chat_or_picker_surface_owns_point { + *upper_hover_changed = true; + } else { + return Some(true); + } + } + None + } +} diff --git a/crates/op-host-web/src/widget_host/keyboard_escape.rs b/crates/op-host-web/src/widget_host/keyboard_escape.rs index fc795e9d8..6b33df8e6 100644 --- a/crates/op-host-web/src/widget_host/keyboard_escape.rs +++ b/crates/op-host-web/src/widget_host/keyboard_escape.rs @@ -160,13 +160,9 @@ impl WidgetHost { .editor_state .editor_ui .property_color_variable_picker_open - .take() .is_some() { - self.editor_state - .editor_ui - .property_color_variable_picker_scroll - .offset = 0.0; + self.editor_state.editor_ui.close_color_variable_picker(); self.mark_dirty(); return true; } diff --git a/crates/op-host-web/src/widget_host/press_property_tiers.rs b/crates/op-host-web/src/widget_host/press_property_tiers.rs index 7c92a542d..2f5f71978 100644 --- a/crates/op-host-web/src/widget_host/press_property_tiers.rs +++ b/crates/op-host-web/src/widget_host/press_property_tiers.rs @@ -306,9 +306,7 @@ impl WidgetHost { { // Non-primary fill swatch — bind the picker to this // fill so HSV writes back to `fills[index]`. - self.editor_state - .editor_ui - .property_color_variable_picker_open = None; + self.editor_state.editor_ui.close_color_variable_picker(); let _ = self.editor_state.open_color_picker_for_fill( op_editor_core::ui_draft::ColorTarget::Fill, index, diff --git a/crates/op-host-web/src/widget_host/property_hover_tests.rs b/crates/op-host-web/src/widget_host/property_hover_tests.rs index b587ef4a7..075fd9ba0 100644 --- a/crates/op-host-web/src/widget_host/property_hover_tests.rs +++ b/crates/op-host-web/src/widget_host/property_hover_tests.rs @@ -412,3 +412,55 @@ fn codegen_preview_wheel_scrolls_code_not_property_panel() { 0.0 ); } + +/// Web twin of the native `color_variable_popup_owns_hover_…` test: the +/// popup owns every point on its chrome, so its own row lights up and +/// the rail's stale wash underneath is dropped instead of showing +/// through. +#[test] +fn color_variable_popup_owns_hover_instead_of_the_rail_underneath() { + use jian_ops_schema::variable::{VariableKind, VariableScalar}; + use op_editor_core::{ColorTarget, NodeId}; + + let mut host = WidgetHost::new(); + host.editor_state = EditorState::sample(); + host.mark_dirty(); + host.last_viewport_w = 1280.0; + host.last_viewport_h = 740.0; + host.editor_state.set_single_selection(NodeId::new("n13")); + for i in 0..6 { + assert!(host.editor_state.create_variable( + &format!("color-surface-{i:02}"), + VariableKind::Color, + VariableScalar::Str("#DBD8CB".into()), + )); + } + host.editor_state + .editor_ui + .property_color_variable_picker_open = Some(ColorTarget::Fill); + // A wash left over from the inspector row the popup now covers. + host.editor_state.editor_ui.property_action_hover = Some(0); + + let rect = property_rect(&host); + let layout = PropertyPanel::for_selection(&host.editor_state) + .expect("rectangle panel") + .color_variable_picker_layout(rect) + .expect("open picker lays out"); + let row = layout.rows[1].1; + + assert!(host.apply_cursor_move( + row.origin.x + row.size.x / 2.0, + row.origin.y + row.size.y / 2.0, + )); + assert_eq!( + host.editor_state + .editor_ui + .property_color_variable_picker_hover, + Some(1), + "the row under the cursor must light up" + ); + assert_eq!( + host.editor_state.editor_ui.property_action_hover, None, + "hover must not pass through the popup to the rail underneath" + ); +}