From cff29f6192ff5fea466b7aa6d002d4887be0417c Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Fri, 9 Jun 2023 15:49:10 +0400 Subject: [PATCH 1/3] Fix issues with mouse cursor position when editing nodes --- .../src/component/node/input/area.rs | 74 ++++++++++++++----- 1 file changed, 57 insertions(+), 17 deletions(-) diff --git a/app/gui/view/graph-editor/src/component/node/input/area.rs b/app/gui/view/graph-editor/src/component/node/input/area.rs index 7a0c73de8897..e3d55287030f 100644 --- a/app/gui/view/graph-editor/src/component/node/input/area.rs +++ b/app/gui/view/graph-editor/src/component/node/input/area.rs @@ -133,13 +133,15 @@ impl From for Expression { /// Internal model of the port area. #[derive(Debug)] pub struct Model { - app: Application, - display_object: display::object::Instance, + app: Application, + display_object: display::object::Instance, edit_mode_label: text::Text, - expression: RefCell, - styles: StyleWatch, - styles_frp: StyleWatchFrp, - widget_tree: widget::Tree, + label_layer: RefCell, + edit_mode_label_displayed: Cell, + expression: RefCell, + styles: StyleWatch, + styles_frp: StyleWatchFrp, + widget_tree: widget::Tree, } #[derive(Debug, Clone, Hash, PartialEq, Eq)] @@ -156,13 +158,26 @@ impl Model { let display_object = display::object::Instance::new_named("input"); let edit_mode_label = app.new_view::(); + let label_layer = RefCell::new(app.display.default_scene.layers.label.downgrade()); + let edit_mode_label_displayed = default(); let expression = default(); let styles = StyleWatch::new(&app.display.default_scene.style_sheet); let styles_frp = StyleWatchFrp::new(&app.display.default_scene.style_sheet); let widget_tree = widget::Tree::new(&app); + display_object.add_child(&edit_mode_label); with_context(|ctx| ctx.layers.widget.add(&widget_tree)); - Self { app, display_object, edit_mode_label, expression, styles, styles_frp, widget_tree } - .init() + Self { + app, + display_object, + edit_mode_label, + edit_mode_label_displayed, + label_layer, + expression, + styles, + styles_frp, + widget_tree, + } + .init() } /// React to edit mode change. Shows and hides appropriate child views according to current @@ -172,24 +187,42 @@ impl Model { let expression = self.expression.borrow(); self.edit_mode_label.set_content(expression.code.clone()); self.display_object.remove_child(&self.widget_tree); - self.display_object.add_child(&self.edit_mode_label); + self.show_edit_mode_label(); self.edit_mode_label.set_cursor_at_mouse_position(); } else { - self.display_object.remove_child(&self.edit_mode_label); + self.hide_edit_mode_label(); self.display_object.add_child(&self.widget_tree); self.edit_mode_label.set_content(""); } self.edit_mode_label.deprecated_set_focus(edit_mode_active); } + /// A workaround to fix the cursor position calculation when clicking into the node. + /// + /// Using standard [`ObjectOps::add_child`] and [`ObjectOps::remove_child`] for + /// [`edit_mode_label`] does not allow setting the cursor position in the same frame after + /// showing the label, as the position of the display object is not updated yet. To fix this, + /// we hide the label by using the `DETACHED` layer so that its position is always kept up to + /// date. To show the label, one can use [`Self::show_edit_mode_label`]. (which will move it + /// back to the correct scene layer) + fn hide_edit_mode_label(&self) { + self.edit_mode_label_displayed.set(false); + self.app.display.default_scene.layers.DETACHED.add(&self.edit_mode_label); + } + + /// Show the edit mode label by placing it in the correct layer. + /// See [`Self::hide_edit_mode_label`]. + fn show_edit_mode_label(&self) { + if let Some(layer) = self.label_layer.borrow().upgrade() { + self.edit_mode_label_displayed.set(true); + self.edit_mode_label.add_to_scene_layer(&layer); + } else { + error!("Cannot show edit mode label, the layer is missing."); + } + } + #[profile(Debug)] fn init(self) -> Self { - // TODO: Depth sorting of labels to in front of the mouse pointer. Temporary solution. - // It needs to be more flexible once we have proper depth management. - // See https://www.pivotaltracker.com/story/show/183567632. - let scene = &self.app.display.default_scene; - self.set_label_layer(&scene.layers.label); - let text_color = self.styles.get_color(theme::graph_editor::node::text); self.edit_mode_label.set_single_line_mode(true); @@ -205,12 +238,19 @@ impl Model { self.widget_tree.set_xy(widgets_origin); self.edit_mode_label.set_xy(label_origin); self.set_edit_mode(false); + self.hide_edit_mode_label(); self } fn set_label_layer(&self, layer: &display::scene::Layer) { - self.edit_mode_label.add_to_scene_layer(layer); + *self.label_layer.borrow_mut() = layer.downgrade(); + // We're taking extra measures to ensure that the actual layer of the label will be updated + // even if it is already displayed. It should be unnecessary with the current + // implementation. See [`Self::show_edit_mode_label`] and [`Self::hide_edit_mode_label`]. + if self.edit_mode_label_displayed.get() { + self.show_edit_mode_label(); + } } fn set_connected(&self, crumbs: &Crumbs, status: Option) { From 74d80553248f33c780bda5126b3e4fb015085ecf Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Mon, 12 Jun 2023 14:45:24 +0400 Subject: [PATCH 2/3] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5e44de3a65b..303f27ee3946 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -181,6 +181,7 @@ being loaded was potentially confusing to users. - [Performance and readability of documentation panel was improved][6893]. The documentation is now split into separate pages, which are much smaller. +- [Fixed cursor position when ctrl-clicking the node][7014]. [6279]: https://github.com/enso-org/enso/pull/6279 [6421]: https://github.com/enso-org/enso/pull/6421 @@ -194,6 +195,7 @@ [6844]: https://github.com/enso-org/enso/pull/6844 [6827]: https://github.com/enso-org/enso/pull/6827 [6893]: https://github.com/enso-org/enso/pull/6893 +[7014]: https://github.com/enso-org/enso/pull/7014 #### EnsoGL (rendering engine) From 673031ed3cc4083ccd0b558b626fefc0839701d8 Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Tue, 13 Jun 2023 16:32:44 +0400 Subject: [PATCH 3/3] Address review comments --- CHANGELOG.md | 4 +++- app/gui/view/graph-editor/src/component/node/input/area.rs | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1295ac65dba..3c84abcc179f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -183,7 +183,9 @@ execution failed][6918]. - [Performance and readability of documentation panel was improved][6893]. The documentation is now split into separate pages, which are much smaller. -- [Fixed cursor position when ctrl-clicking the node][7014]. +- [Fixed cursor position when ctrl-clicking the node][7014]. Sometimes + ctrl-clicking to edit the node placed the mouse cursor in the wrong position + in the text. This is fixed now. [6279]: https://github.com/enso-org/enso/pull/6279 [6421]: https://github.com/enso-org/enso/pull/6421 diff --git a/app/gui/view/graph-editor/src/component/node/input/area.rs b/app/gui/view/graph-editor/src/component/node/input/area.rs index e3d55287030f..22f1530f43fd 100644 --- a/app/gui/view/graph-editor/src/component/node/input/area.rs +++ b/app/gui/view/graph-editor/src/component/node/input/area.rs @@ -245,9 +245,9 @@ impl Model { fn set_label_layer(&self, layer: &display::scene::Layer) { *self.label_layer.borrow_mut() = layer.downgrade(); - // We're taking extra measures to ensure that the actual layer of the label will be updated - // even if it is already displayed. It should be unnecessary with the current - // implementation. See [`Self::show_edit_mode_label`] and [`Self::hide_edit_mode_label`]. + // Currently, we never sets label layer when it's already displayed, but - as + // `set_label_layer` is a public method of this component - we're taking extra measures. + // See [`Self::show_edit_mode_label`] and [`Self::hide_edit_mode_label`]. if self.edit_mode_label_displayed.get() { self.show_edit_mode_label(); }