Placement of newly opened Component Browser when nodes are selected (#…

…3361)

When a new node is created with the <kbd>TAB</kbd> key or by clicking the `(+)` on-screen button while multiple nodes are selected, place the new node below all the selected nodes. (Previously, the new node was placed below the node that was selected earliest.)

Additionally, when placing a new node below an existing non-error node with a visualization enabled, place the new node below the visualization. (Previously, the new node was placed to the left of the visualization.)

https://www.pivotaltracker.com/story/show/180887079

#### Visuals

The following screencast demonstrates the feature on various arrangements of selected nodes, with visualization enabled and disabled.

https://user-images.githubusercontent.com/273837/159971452-148aa4d7-c0f3-4b48-871a-a2783989f403.mov

The following screencast demonstrates that new nodes created by double-clicking an output port of a node with visualization enabled are now placed below the visualization:

https://user-images.githubusercontent.com/273837/160107733-e3f7d0f9-0161-49d1-8cbd-06e18c843a20.mov

# Important Notes
- Some refactorings that were needed for this PR were ported from the #3301 PR:
- the code responsible for calculating the positions of new nodes was moved to a separate module (`new_node_position`);
- the `free_place_finder` module was made a submodule of the `new_node_position` module, due to the latter being its only user.

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 created near existing nodes via the <kbd>TAB</kbd> key or by dropping a
   connection are now repositioned and aligned to existing nodes.][3301] This is
   to make the resulting graph prettier and avoid overlapping. In such cases,
@@ -24,6 +29,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
@@ -127,6 +136,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/lib.rs

@@ -392,6 +392,14 @@ impl<K, V, S> SharedHashMap<K, V, S> {
         self.raw.borrow().keys().cloned().collect_vec()
     }
 
+    /// Get the vector of map's keys and values.
+    pub fn entries(&self) -> Vec<(K, V)>
+    where
+        K: Clone,
+        V: CloneRef, {
+        self.raw.borrow().iter().map(|(k, v)| (k.clone(), v.clone_ref())).collect_vec()
+    }
+
     /// Get the vector of map's values.
     pub fn values(&self) -> Vec<V>
     where V: Clone {
@@ -1393,18 +1401,17 @@ impl GraphEditorModelWithNetwork {
 
 /// Describes the way used to request creation of a new node.
 #[derive(Clone, Debug)]
+#[allow(missing_docs)]
 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 },
 }
 
@@ -1431,27 +1438,22 @@ impl GraphEditorModelWithNetwork {
         way: &WayOfCreatingNode,
         mouse_position: Vector2,
     ) -> (NodeId, Option<NodeSource>, bool) {
-        let selection = self.nodes.selected.first_cloned();
-        let position = new_node_position::new_node_position(self, way, selection, mouse_position);
+        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, selection);
+        let source = self.data_source_for_new_node(way);
         (node.id(), source, should_edit)
     }
 
-    fn data_source_for_new_node(
-        &self,
-        way: &WayOfCreatingNode,
-        selection: Option<NodeId>,
-    ) -> Option<NodeSource> {
+    fn data_source_for_new_node(&self, way: &WayOfCreatingNode) -> Option<NodeSource> {
         use WayOfCreatingNode::*;
         let source_node = match way {
             AddNodeEvent => None,
-            StartCreationEvent | ClickingButton => selection,
+            StartCreationEvent | ClickingButton => self.nodes.selected.first_cloned(),
             DroppingEdge { edge_id } => self.edge_source_node_id(*edge_id),
             StartCreationFromPortEvent { endpoint } => Some(endpoint.node_id),
         };
@@ -2064,6 +2066,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();

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

@@ -2,8 +2,6 @@
 //!
 //! The returned positions are such that the new nodes will not overlap with existing ones.
 
-pub mod free_place_finder;
-
 use crate::prelude::*;
 
 use crate::component::node;
@@ -19,6 +17,13 @@ use crate::WayOfCreatingNode;
 use ensogl_hardcoded_theme as theme;
 
 
+// ==============
+// === Export ===
+// ==============
+
+pub mod free_place_finder;
+
+
 
 /// ============================
 /// === New Node Positioning ===
@@ -29,11 +34,12 @@ use ensogl_hardcoded_theme as theme;
 ///
 /// The reference position is chosen from among:
 ///  - the position of a source node of the dropped edge (if available),
+///  - the bottom-most selected node (if available),
 ///  - the mouse position,
 ///  - the screen center.
 /// The position is then aligned to either:
 ///  - the source node of the dropped edge (if available),
-///  - the `selection` (if available),
+///  - the selected nodes (if available),
 ///  - the node closest to the reference position (if available),
 ///  - not aligned.
 /// The choice among the options described above is governed by the `way`.
@@ -42,19 +48,17 @@ use ensogl_hardcoded_theme as theme;
 pub fn new_node_position(
     graph_editor: &GraphEditorModel,
     way: &WayOfCreatingNode,
-    selection: Option<NodeId>,
     mouse_position: Vector2,
 ) -> Vector2 {
     use WayOfCreatingNode::*;
     let scene = graph_editor.scene();
     let origin = Vector2(0.0, 0.0);
     let screen_center = scene.screen_to_object_space(&graph_editor.display_object, origin);
-    assert!(!screen_center.x.is_nan());
-    assert!(!screen_center.y.is_nan());
+    let some_nodes_are_selected = !graph_editor.nodes.selected.is_empty();
     match way {
         AddNodeEvent => default(),
-        StartCreationEvent | ClickingButton if selection.is_some() =>
-            under(graph_editor, selection.unwrap()),
+        StartCreationEvent | ClickingButton if some_nodes_are_selected =>
+            under_selected_nodes(graph_editor),
         StartCreationEvent => at_mouse_aligned_to_close_nodes(graph_editor, mouse_position),
         ClickingButton => on_ray(graph_editor, screen_center, Vector2(0.0, -1.0)).unwrap(),
         DroppingEdge { edge_id } =>
@@ -63,6 +67,55 @@ pub fn new_node_position(
     }
 }
 
+/// Return a position for a newly created node closely below all selected nodes, or a zero vector
+/// if no nodes are selected. The position is left-aligned to the first selected node, then moved
+/// to the left to the first available position if the initial position is not available.
+///
+/// Availability of a position is defined in the docs of [`on_ray`].
+pub fn under_selected_nodes(graph_editor: &GraphEditorModel) -> Vector2 {
+    let first_selected_node = graph_editor.nodes.selected.first_cloned();
+    let first_selected_node_x = match first_selected_node {
+        None => return Vector2::zeros(),
+        Some(node_id) => graph_editor.node_position(node_id).x,
+    };
+    let node_bbox_bottom = |node_id| graph_editor.node_bounding_box(node_id).bottom();
+    let selected_nodes = graph_editor.nodes.selected.raw.borrow();
+    let selection_bottom = selected_nodes.iter().map(node_bbox_bottom).reduce(min);
+    let selection_bottom_or_zero = selection_bottom.unwrap_or_default();
+    below_line_and_left_aligned(graph_editor, selection_bottom_or_zero, first_selected_node_x)
+}
+
+/// Return a position for a newly created node. Returns a position closely below the `node_id` node
+/// if the position is available, or a first available point on a ray extending to the left of that
+/// position.
+///
+/// Availability of a position is defined in the docs of [`on_ray`].
+pub fn under(graph_editor: &GraphEditorModel, node_id: NodeId) -> Vector2 {
+    let above_node_pos = graph_editor.node_position(node_id);
+    let above_node_bbox = graph_editor.node_bounding_box(node_id);
+    below_line_and_left_aligned(graph_editor, above_node_bbox.bottom(), above_node_pos.x)
+}
+
+/// Return a position for a newly created node. Returns a position closely below a horizontal line
+/// at `line_y` and left-aligned to `align_x`, or a first available position to the left of it if
+/// the initial position is not available.
+///
+/// "Closely below" means that a vertical gap is maintained between the line and the top border of
+/// a node placed at the returned position. The vertical gap is equal to
+/// [`theme::graph_editor::default_y_gap_between_nodes`].
+/// Availability of a position is defined in the docs of [`on_ray`].
+pub fn below_line_and_left_aligned(
+    graph_editor: &GraphEditorModel,
+    line_y: f32,
+    align_x: f32,
+) -> Vector2 {
+    let y_gap = graph_editor.frp.default_y_gap_between_nodes.value();
+    let y_offset = y_gap + node::HEIGHT / 2.0;
+    let starting_point = Vector2(align_x, line_y - y_offset);
+    let direction = Vector2(-1.0, 0.0);
+    on_ray(graph_editor, starting_point, direction).unwrap()
+}
+
 /// Return a position for a newly created node. The position is calculated by taking the mouse
 /// position and aligning it to the closest existing node if the mouse position is close enough to
 /// the node.
@@ -141,20 +194,6 @@ pub fn aligned_if_close_to_node(
     }
 }
 
-/// Return a position for a newly created node. Returns a position closely below the `node_id` node
-/// if the position is available, or a first available point on a ray extending to the left of that
-/// position.
-///
-/// Availability of a position is defined in the docs of [`on_ray`].
-pub fn under(graph_editor: &GraphEditorModel, node_above: NodeId) -> Vector2 {
-    let above_pos = graph_editor.node_position(node_above);
-    let y_gap = graph_editor.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);
-    on_ray(graph_editor, starting_point, direction).unwrap()
-}
-
 /// Return a position for a newly created node. Return the first available position on a ray
 /// extending from `starting_point` in the `direction`, or [`None`] if the magnitude of each
 /// coordinate of `direction` is smaller than [`f32::EPSILON`].

integration-test/src/lib.rs

@@ -25,8 +25,10 @@ use enso_frp::future::EventOutputExt;
 use enso_gui::executor::web::EventLoopExecutor;
 use enso_gui::initializer::setup_global_executor;
 use enso_gui::Ide;
+use enso_web::Closure;
 use enso_web::HtmlDivElement;
 use ensogl::application::test_utils::ApplicationExt;
+use std::pin::Pin;
 
 
 
@@ -35,6 +37,7 @@ pub mod prelude {
     pub use crate::IntegrationTest;
     pub use crate::IntegrationTestOnNewProject;
 
+    pub use crate::wait_a_frame;
     pub use enso_frp::future::EventOutputExt;
     pub use enso_gui::prelude::*;
     pub use wasm_bindgen_test::wasm_bindgen_test;
@@ -127,3 +130,54 @@ impl IntegrationTestOnNewProject {
         self.project_view().graph().clone_ref()
     }
 }
+
+
+
+// ==================
+// === WaitAFrame ===
+// ==================
+
+/// A future that resolves after one frame.
+#[derive(Default, Debug)]
+#[cfg_attr(not(target_arch = "wasm32"), allow(dead_code))]
+pub struct WaitAFrame {
+    frame_passed: Rc<Cell<bool>>,
+    closure:      Option<Closure<dyn FnMut(f64)>>,
+}
+
+impl Future for WaitAFrame {
+    type Output = ();
+
+    #[cfg(not(target_arch = "wasm32"))]
+    fn poll(
+        self: Pin<&mut Self>,
+        _cx: &mut std::task::Context<'_>,
+    ) -> std::task::Poll<Self::Output> {
+        std::task::Poll::Ready(())
+    }
+
+    #[cfg(target_arch = "wasm32")]
+    fn poll(
+        mut self: Pin<&mut Self>,
+        cx: &mut std::task::Context<'_>,
+    ) -> std::task::Poll<Self::Output> {
+        if self.frame_passed.get() {
+            std::task::Poll::Ready(())
+        } else {
+            let waker = cx.waker().clone();
+            let frame_passed = self.frame_passed.clone_ref();
+            let closure = Closure::once(move |_| {
+                frame_passed.set(true);
+                waker.wake()
+            });
+            enso_web::window.request_animation_frame_with_closure_or_panic(&closure);
+            self.closure = Some(closure);
+            std::task::Poll::Pending
+        }
+    }
+}
+
+/// Return a future that resolves after one frame.
+pub fn wait_a_frame() -> impl Future<Output = ()> {
+    WaitAFrame::default()
+}