SemianiakaVY/openpencil
1
0
Fork 0
Code Issues Pull requests Actions 1 Packages Projects Releases Wiki Activity

fix(panels): fix color variable picker interaction routing

Browse source
This commit is contained in:
Fini 2026-07-30 08:57:13 +08:00
parent 76af87930f
commit bcce4e7071
23 changed files with 570 additions and 52 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
crates/op-editor-core/src/editor_ui_state.rs
View file

@ -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<usize>,
/// Whether the image-fill editor popover is open.
pub image_fill_popover_open: bool,
/// Image-fill node currently in Figma-style crop editing mode.

1
crates/op-editor-core/src/editor_ui_state/defaults.rs
View file

@ -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,

15
crates/op-editor-core/src/editor_ui_state/methods.rs
View file

@ -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;

2
crates/op-editor-core/src/host_image_panel_transitions.rs
View file

@ -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();
}

2
crates/op-editor-core/src/host_ui_transitions.rs
View file

@ -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();
}
}

34
crates/op-editor-ui/src/widgets/cursor_hover_flow.rs
View file

@ -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

10
crates/op-editor-ui/src/widgets/press_flow.rs
View file

@ -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
}

4
crates/op-editor-ui/src/widgets/property_panel.rs
View file

@ -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<usize>,
pub color_variables: Vec<ColorVariableOption>,
pub fill_variable_ref: Option<String>,
pub stroke_variable_ref: Option<String>,

1
crates/op-editor-ui/src/widgets/property_panel/build.rs
View file

@ -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,

1
crates/op-editor-ui/src/widgets/property_panel/paint.rs
View file

@ -499,6 +499,7 @@ impl Widget for PropertyPanel {
&layout,
&self.color_variables,
self.bound_color_variable_ref(),
self.color_variable_picker_hover,
self.locale,
);
}

32
crates/op-editor-ui/src/widgets/property_panel_color_variables.rs
View file

@ -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<ColorVariableRow> {
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<usize> {
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<usize>,
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(

252
crates/op-editor-ui/src/widgets/property_panel_color_variables_tests.rs
View file

@ -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");
}

46
crates/op-editor-ui/src/widgets/property_panel_dispatch.rs
View file

@ -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 } => {

8
crates/op-editor-ui/src/widgets/property_panel_overlay_hit.rs
View file

@ -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<usize> {
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)

29
crates/op-host-native/src/widget_host/cursor_move_overlays.rs
View file

@ -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

6
crates/op-host-native/src/widget_host/keyboard_escape.rs
View file

@ -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;
}

54
crates/op-host-native/src/widget_host/overlay_cursor_tests.rs
View file

@ -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"
);
}

4
crates/op-host-native/src/widget_host/press_property_tiers.rs
View file

@ -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,

14
crates/op-host-web/src/widget_host/cursor_input.rs
View file

@ -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
{

41
crates/op-host-web/src/widget_host/hover_property_panel.rs
View file

@ -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<bool> {
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
}
}

6
crates/op-host-web/src/widget_host/keyboard_escape.rs
View file

@ -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;
}

4
crates/op-host-web/src/widget_host/press_property_tiers.rs
View file

@ -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,

52
crates/op-host-web/src/widget_host/property_hover_tests.rs
View file

@ -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"
);
}