Placement of newly opened Component Browser when nodes are selected

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 #### Visual Environment
 
+- [Nodes created via the <kbd>TAB</kbd> key or by clicking the (+) button on the
+  screen are now placed below all the selected nodes when more than one node is
+  selected.][3361] (Previously, they were placed below the first node that was
+  selected.) This makes it easier to achieve a compact, vertical layout of the
+  graph.
 - [Nodes can be added to the graph by double-clicking the output ports of
   existing nodes (or by clicking them with the right mouse button).][3346]
 - [Node Searcher preserves its zoom factor.][3327] The visible size of the node
@@ -19,6 +24,10 @@
 - [Node connections can be dropped by pressing the Esc key while dragging
   them.][3231]
 - [Added support of source maps for JS-based visualizations.][3208]
+- [Fixed the alignment of newly created nodes to existing nodes with
+  visualizations enabled.][3361] When applicable, new nodes are now placed below
+  visualizations. (Previously, they were placed to the left of the
+  visualizations.)
 - [Fixed histograms coloring and added a color legend.][3153]
 - [Fixed broken node whose expression contains non-ASCII characters.][3166]
 - [Fixed developer console warnings about views being created but not
@@ -121,6 +130,7 @@
 [3344]: https://github.com/enso-org/enso/pull/3344
 [3346]: https://github.com/enso-org/enso/pull/3346
 [3349]: https://github.com/enso-org/enso/pull/3349
+[3361]: https://github.com/enso-org/enso/pull/3361
 
 #### Enso Compiler
 

app/gui/view/graph-editor/src/component/node.rs

@@ -610,21 +610,22 @@ impl NodeModel {
         self.drag_area.size.set(padded_size);
         self.error_indicator.size.set(padded_size);
         self.vcs_indicator.set_size(padded_size);
-        self.backdrop.mod_position(|t| t.x = width / 2.0);
-        self.background.mod_position(|t| t.x = width / 2.0);
-        self.drag_area.mod_position(|t| t.x = width / 2.0);
-        self.error_indicator.set_position_x(width / 2.0);
-        self.vcs_indicator.set_position_x(width / 2.0);
+        let x_offset_to_node_center = x_offset_to_node_center(width);
+        self.backdrop.set_position_x(x_offset_to_node_center);
+        self.background.set_position_x(x_offset_to_node_center);
+        self.drag_area.set_position_x(x_offset_to_node_center);
+        self.error_indicator.set_position_x(x_offset_to_node_center);
+        self.vcs_indicator.set_position_x(x_offset_to_node_center);
 
         let action_bar_width = ACTION_BAR_WIDTH;
         self.action_bar.mod_position(|t| {
-            t.x = width + CORNER_RADIUS + action_bar_width / 2.0;
+            t.x = x_offset_to_node_center + width / 2.0 + CORNER_RADIUS + action_bar_width / 2.0;
         });
         self.action_bar.frp.set_size(Vector2::new(action_bar_width, ACTION_BAR_HEIGHT));
 
-        let visualization_pos = Vector2(width / 2.0, VISUALIZATION_OFFSET_Y);
-        self.error_visualization.set_position_xy(visualization_pos);
-        self.visualization.set_position_xy(visualization_pos);
+        let visualization_offset = visualization_offset(width);
+        self.error_visualization.set_position_xy(visualization_offset);
+        self.visualization.set_position_xy(visualization_offset);
 
         size
     }
@@ -752,14 +753,6 @@ impl Node {
             eval new_size ((t) model.output.frp.set_size.emit(t));
 
 
-            // === Bounding Box ===
-            bounding_box_input <- all2(&new_size,&position);
-            out.bounding_box <+ bounding_box_input.map(|(size,position)| {
-                let position = position - Vector2::new(0.0,size.y / 2.0);
-                BoundingBox::from_position_and_size(position,*size)
-            });
-
-
             // === Action Bar ===
 
             let visualization_enabled = action_bar.action_visibility.clone_ref();
@@ -934,6 +927,16 @@ impl Node {
                 <+ visualization_visible.not().and(&no_error_set);
 
 
+            // === Bounding Box ===
+
+            let visualization_size = &model.visualization.frp.size;
+            // Visualization can be enabled and not visible when the node has an error.
+            visualization_enabled_and_visible <- visualization_enabled && visualization_visible;
+            bbox_input <- all4(
+                &position,&new_size,&visualization_enabled_and_visible,visualization_size);
+            out.bounding_box <+ bbox_input.map(|(a,b,c,d)| bounding_box(*a,*b,*c,*d));
+
+
             // === VCS Handling ===
 
             model.vcs_indicator.frp.set_status <+ input.set_vcs_status;
@@ -969,3 +972,37 @@ impl display::Object for Node {
         &self.model.display_object
     }
 }
+
+
+// === Positioning ===
+
+fn x_offset_to_node_center(node_width: f32) -> f32 {
+    node_width / 2.0
+}
+
+/// Calculate a position where to render the [`visualization::Container`] of a node, relative to
+/// the node's origin.
+fn visualization_offset(node_width: f32) -> Vector2 {
+    Vector2(x_offset_to_node_center(node_width), VISUALIZATION_OFFSET_Y)
+}
+
+fn bounding_box(
+    node_position: Vector2,
+    node_size: Vector2,
+    visualization_enabled_and_visible: bool,
+    visualization_size: Vector2,
+) -> BoundingBox {
+    let x_offset_to_node_center = x_offset_to_node_center(node_size.x);
+    let node_bbox_pos = node_position + Vector2(x_offset_to_node_center, 0.0) - node_size / 2.0;
+    let node_bbox = BoundingBox::from_position_and_size(node_bbox_pos, node_size);
+    if visualization_enabled_and_visible {
+        let visualization_offset = visualization_offset(node_size.x);
+        let visualization_pos = node_position + visualization_offset;
+        let visualization_bbox_pos = visualization_pos - visualization_size / 2.0;
+        let visualization_bbox =
+            BoundingBox::from_position_and_size(visualization_bbox_pos, visualization_size);
+        node_bbox.concat_ref(visualization_bbox)
+    } else {
+        node_bbox
+    }
+}

app/gui/view/graph-editor/src/lib.rs

@@ -31,8 +31,7 @@ pub mod component;
 
 pub mod builtin;
 pub mod data;
-#[warn(missing_docs)]
-pub mod free_place_finder;
+pub mod new_node_position;
 #[warn(missing_docs)]
 pub mod profiling;
 #[warn(missing_docs)]
@@ -49,8 +48,6 @@ use crate::component::visualization;
 use crate::component::visualization::instance::PreprocessorConfiguration;
 use crate::component::visualization::MockDataGenerator3D;
 use crate::data::enso;
-use crate::free_place_finder::find_free_place;
-use crate::free_place_finder::OccupiedArea;
 pub use crate::node::profiling::Status as NodeProfilingStatus;
 
 use enso_config::ARGS;
@@ -1388,17 +1385,20 @@ impl GraphEditorModelWithNetwork {
 
 // === Node Creation ===
 
+/// Describes the way used to request creation of a new node.
 #[derive(Clone, Debug)]
-enum WayOfCreatingNode {
+pub enum WayOfCreatingNode {
     /// "add_node" FRP event was emitted.
     AddNodeEvent,
     /// "start_node_creation" FRP event was emitted.
     StartCreationEvent,
     /// "start_node_creation_from_port" FRP event was emitted.
+    #[allow(missing_docs)]
     StartCreationFromPortEvent { endpoint: EdgeEndpoint },
     /// add_node_button was clicked.
     ClickingButton,
     /// The edge was dropped on the stage.
+    #[allow(missing_docs)]
     DroppingEdge { edge_id: EdgeId },
 }
 
@@ -1425,36 +1425,28 @@ impl GraphEditorModelWithNetwork {
         way: &WayOfCreatingNode,
         mouse_position: Vector2,
     ) -> (NodeId, Option<NodeSource>, bool) {
-        use WayOfCreatingNode::*;
-        let should_edit = !matches!(way, AddNodeEvent);
-        let selection = self.nodes.selected.first_cloned();
-        let source_node = match way {
-            AddNodeEvent => None,
-            StartCreationEvent | ClickingButton => selection,
-            DroppingEdge { edge_id } => self.edge_source_node_id(*edge_id),
-            StartCreationFromPortEvent { endpoint } => Some(endpoint.node_id),
-        };
-        let source = source_node.map(|node| NodeSource { node });
-        let screen_center =
-            self.scene().screen_to_object_space(&self.display_object, Vector2(0.0, 0.0));
-        let position: Vector2 = match way {
-            AddNodeEvent => default(),
-            StartCreationEvent | ClickingButton if selection.is_some() =>
-                self.find_free_place_under(selection.unwrap()),
-            StartCreationEvent => mouse_position,
-            ClickingButton =>
-                self.find_free_place_for_node(screen_center, Vector2(0.0, -1.0)).unwrap(),
-            DroppingEdge { .. } => mouse_position,
-            StartCreationFromPortEvent { endpoint } => self.find_free_place_under(endpoint.node_id),
-        };
+        let position = new_node_position::new_node_position(self, way, mouse_position);
         let node = self.new_node(ctx);
         node.set_position_xy(position);
+        let should_edit = !matches!(way, WayOfCreatingNode::AddNodeEvent);
         if should_edit {
             node.view.set_expression(node::Expression::default());
         }
+        let source = self.data_source_for_new_node(way);
         (node.id(), source, should_edit)
     }
 
+    fn data_source_for_new_node(&self, way: &WayOfCreatingNode) -> Option<NodeSource> {
+        use WayOfCreatingNode::*;
+        let source_node = match way {
+            AddNodeEvent => None,
+            StartCreationEvent | ClickingButton => self.nodes.selected.first_cloned(),
+            DroppingEdge { edge_id } => self.edge_source_node_id(*edge_id),
+            StartCreationFromPortEvent { endpoint } => Some(endpoint.node_id),
+        };
+        source_node.map(|node| NodeSource { node })
+    }
+
     fn new_node(&self, ctx: &NodeCreationContext) -> Node {
         use ensogl::application::command::FrpNetworkProvider;
         let view = component::Node::new(&self.app, self.vis_registry.clone_ref());
@@ -1722,7 +1714,7 @@ impl GraphEditorModel {
 
     /// Create a new node and place it at a free place below `above` node.
     pub fn add_node_below(&self, above: NodeId) -> NodeId {
-        let pos = self.find_free_place_under(above);
+        let pos = new_node_position::under(self, above);
         self.add_node_at(pos)
     }
 
@@ -1732,46 +1724,6 @@ impl GraphEditorModel {
         self.frp.set_node_position((node_id, pos));
         node_id
     }
-
-    /// Return the first available position for a new node below `node_above` node.
-    pub fn find_free_place_under(&self, node_above: NodeId) -> Vector2 {
-        let above_pos = self.node_position(node_above);
-        let y_gap = self.frp.default_y_gap_between_nodes.value();
-        let y_offset = y_gap + node::HEIGHT;
-        let starting_point = above_pos - Vector2(0.0, y_offset);
-        let direction = Vector2(-1.0, 0.0);
-        self.find_free_place_for_node(starting_point, direction).unwrap()
-    }
-
-    /// Return the first unoccupied point when going along the ray starting from `starting_point`
-    /// and parallel to `direction` vector.
-    pub fn find_free_place_for_node(
-        &self,
-        starting_from: Vector2,
-        direction: Vector2,
-    ) -> Option<Vector2> {
-        let x_gap = self.frp.default_x_gap_between_nodes.value();
-        let y_gap = self.frp.default_y_gap_between_nodes.value();
-        // This is how much horizontal space we are looking for.
-        let min_spacing = self.frp.min_x_spacing_for_new_nodes.value();
-        let nodes = self.nodes.all.raw.borrow();
-        // The "occupied area" for given node consists of:
-        // - area taken by node view (obviously);
-        // - the minimum gap between nodes in all directions, so the new node won't be "glued" to
-        //   another;
-        // - the new node size measured from origin point at each direction accordingly: because
-        //   `find_free_place` looks for free place for the origin point, and we want to fit not
-        //   only the point, but the whole node.
-        let node_areas = nodes.values().map(|node| {
-            let position = node.position();
-            let left = position.x - x_gap - min_spacing;
-            let right = position.x + node.view.model.width() + x_gap;
-            let top = position.y + node::HEIGHT + y_gap;
-            let bottom = position.y - node::HEIGHT - y_gap;
-            OccupiedArea { x1: left, x2: right, y1: top, y2: bottom }
-        });
-        find_free_place(starting_from, direction, node_areas)
-    }
 }
 
 
@@ -2101,6 +2053,14 @@ impl GraphEditorModel {
         self.nodes.get_cloned_ref(&node_id).map(|node| node.position().xy()).unwrap_or_default()
     }
 
+    /// Return the bounding box of the node identified by `node_id`, or a default bounding box if
+    /// the node was not found.
+    pub fn node_bounding_box(&self, node_id: impl Into<NodeId>) -> selection::BoundingBox {
+        let node_id = node_id.into();
+        let node = self.nodes.get_cloned_ref(&node_id);
+        node.map(|node| node.bounding_box.value()).unwrap_or_default()
+    }
+
     #[allow(missing_docs)] // FIXME[everyone] All pub functions should have docs.
     pub fn node_pos_mod(&self, node_id: impl Into<NodeId>, pos_diff: Vector2) -> (NodeId, Vector2) {
         let node_id = node_id.into();