Context menu 3: add "Move to new container" context menu action (#5210)

### What

* Follow-up to #5205
* Part of #4823


![Export-1708013884748](https://github.com/rerun-io/rerun/assets/49431240/c3e6485a-48c6-4f38-b5c2-907f4c546346)

<br/>
<br/>

This PR adds a new "Move to new container" context menu action, that
applies when one or more space views and/or containers are selected. The
new container is created at the location of the clicked item, and all
the selected items are moved into it.

**Note**: one very often runs into this while using this feature:
- #5208

TODO in future PR:
- additional actions
- entity level manipulations
- use ListItem instead of label/WidgetText
- better folder structure
- release checklist update

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using newly built examples:
[app.rerun.io](https://app.rerun.io/pr/5210/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5210/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/5210/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/5210)
- [Docs
preview](https://rerun.io/preview/d842739a5e559177ddc2b0f682b58df6a40076b3/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/d842739a5e559177ddc2b0f682b58df6a40076b3/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

crates/re_viewport/src/context_menu.rs

@@ -1,14 +1,17 @@
-//TODO(ab): use list items to make those context menu nice to look at
+use std::rc::Rc;
 
-use crate::{Contents, ViewportBlueprint};
 use itertools::Itertools;
+
 use re_log_types::{EntityPath, EntityPathFilter};
 use re_space_view::{DataQueryBlueprint, SpaceViewBlueprint};
 use re_viewer_context::{ContainerId, Item, Selection, SpaceViewClassIdentifier, ViewerContext};
 
+use crate::{Contents, ViewportBlueprint};
+
 /// Trait for things that can populate a context menu
 trait ContextMenuItem {
-    //TODO(ab): should probably return `egui::WidgetText` instead
+    // TODO(ab): return a `ListItem` to make those context menu nice to look at. This requires
+    // changes to the context menu UI code to support full-span highlighting.
     fn label(&self, _ctx: &ViewerContext<'_>, _viewport_blueprint: &ViewportBlueprint) -> String {
         String::new()
     }
@@ -34,31 +37,29 @@ trait ContextMenuItem {
 fn context_menu_items_for_selection_summary(
     ctx: &ViewerContext<'_>,
     viewport_blueprint: &ViewportBlueprint,
+    item: &Item,
     selection_summary: SelectionSummary,
 ) -> Vec<Box<dyn ContextMenuItem>> {
     match selection_summary {
         SelectionSummary::SingleContainerItem(container_id) => {
-            let mut items = vec![];
+            // We want all the actions available for collections of contents…
+            let mut items = context_menu_items_for_selection_summary(
+                ctx,
+                viewport_blueprint,
+                item,
+                SelectionSummary::ContentsItems(vec![Contents::Container(container_id)]),
+            );
 
-            // only show/hide and remove if it's not the root container
-            if Some(container_id) != viewport_blueprint.root_container {
-                let contents = vec![Contents::Container(container_id)];
-                items.extend([
-                    ContentVisibilityToggle::item(viewport_blueprint, contents.clone()),
-                    ContentRemove::item(contents),
-                    Separator::item(),
-                ]);
+            if !items.is_empty() {
+                items.push(Separator::item());
             }
 
+            // …plus some more that apply to single container only.
             items.extend([
                 SubMenu::item(
                     "Add Container",
-                    [
-                        AddContainer::item(container_id, egui_tiles::ContainerKind::Tabs),
-                        AddContainer::item(container_id, egui_tiles::ContainerKind::Horizontal),
-                        AddContainer::item(container_id, egui_tiles::ContainerKind::Vertical),
-                        AddContainer::item(container_id, egui_tiles::ContainerKind::Grid),
-                    ],
+                    possible_child_container_kind(viewport_blueprint, container_id)
+                        .map(|kind| AddContainer::item(container_id, kind)),
                 ),
                 SubMenu::item(
                     "Add Space View",
@@ -74,18 +75,49 @@ fn context_menu_items_for_selection_summary(
         SelectionSummary::ContentsItems(contents) => {
             // exclude the root container from the list of contents, as it cannot be shown/hidden
             // nor removed
-            let contents: Vec<_> = contents
-                .into_iter()
-                .filter(|c| Some(*c) != viewport_blueprint.root_container.map(Contents::Container))
-                .collect();
+            let contents: Rc<Vec<_>> = Rc::new(
+                contents
+                    .into_iter()
+                    .filter(|c| {
+                        Some(*c) != viewport_blueprint.root_container.map(Contents::Container)
+                    })
+                    .collect(),
+            );
 
             if contents.is_empty() {
                 vec![]
-            } else {
+            } else if let Some(root_container_id) = viewport_blueprint.root_container {
+                // The new container should be created in place of the right-clicked content, so we
+                // look for its parent and position, and fall back to the root container.
+                let clicked_content = match item {
+                    Item::Container(container_id) => Some(Contents::Container(*container_id)),
+                    Item::SpaceView(space_view_id) => Some(Contents::SpaceView(*space_view_id)),
+                    _ => None,
+                };
+                let (target_container_id, target_position) = clicked_content
+                    .and_then(|c| viewport_blueprint.find_parent_and_position_index(&c))
+                    .unwrap_or((root_container_id, 0));
+
                 vec![
                     ContentVisibilityToggle::item(viewport_blueprint, contents.clone()),
-                    ContentRemove::item(contents),
+                    ContentRemove::item(contents.clone()),
+                    Separator::item(),
+                    SubMenu::item(
+                        "Move to new container",
+                        possible_child_container_kind(viewport_blueprint, target_container_id).map(
+                            |kind| {
+                                MoveContentsToNewContainer::item(
+                                    target_container_id,
+                                    target_position,
+                                    kind,
+                                    contents.clone(),
+                                )
+                            },
+                        ),
+                    ),
                 ]
+            } else {
+                vec![]
             }
         }
         SelectionSummary::Heterogeneous | SelectionSummary::Empty => vec![],
@@ -121,8 +153,12 @@ pub fn context_menu_ui_for_item(
             summarize_selection(ctx.selection())
         };
 
-        let actions =
-            context_menu_items_for_selection_summary(ctx, viewport_blueprint, selection_summary);
+        let actions = context_menu_items_for_selection_summary(
+            ctx,
+            viewport_blueprint,
+            item,
+            selection_summary,
+        );
 
         if actions.is_empty() {
             ui.label(
@@ -139,6 +175,33 @@ pub fn context_menu_ui_for_item(
     });
 }
 
+/// Helper that returns the allowable containers
+fn possible_child_container_kind(
+    viewport_blueprint: &ViewportBlueprint,
+    container_id: ContainerId,
+) -> impl Iterator<Item = egui_tiles::ContainerKind> + 'static {
+    let container_kind = viewport_blueprint
+        .container(&container_id)
+        .map(|c| c.container_kind);
+
+    static ALL_CONTAINERS: &[egui_tiles::ContainerKind] = &[
+        egui_tiles::ContainerKind::Tabs,
+        egui_tiles::ContainerKind::Horizontal,
+        egui_tiles::ContainerKind::Vertical,
+        egui_tiles::ContainerKind::Grid,
+    ];
+
+    ALL_CONTAINERS
+        .iter()
+        .copied()
+        .filter(move |kind| match kind {
+            egui_tiles::ContainerKind::Horizontal | egui_tiles::ContainerKind::Vertical => {
+                container_kind != Some(*kind)
+            }
+            _ => true,
+        })
+}
+
 // ================================================================================================
 // Selection summary
 // ================================================================================================
@@ -252,14 +315,14 @@ impl ContextMenuItem for Separator {
 
 /// Control the visibility of a container or space view
 struct ContentVisibilityToggle {
-    contents: Vec<Contents>,
+    contents: Rc<Vec<Contents>>,
     set_visible: bool,
 }
 
 impl ContentVisibilityToggle {
     fn item(
         viewport_blueprint: &ViewportBlueprint,
-        contents: Vec<Contents>,
+        contents: Rc<Vec<Contents>>,
     ) -> Box<dyn ContextMenuItem> {
         Box::new(Self {
             set_visible: !contents
@@ -280,19 +343,19 @@ impl ContextMenuItem for ContentVisibilityToggle {
     }
 
     fn run(&self, ctx: &ViewerContext<'_>, viewport_blueprint: &ViewportBlueprint) {
-        for content in &self.contents {
+        for content in &*self.contents {
             viewport_blueprint.set_content_visibility(ctx, content, self.set_visible);
         }
     }
 }
 
 /// Remove a container or space view
 struct ContentRemove {
-    contents: Vec<Contents>,
+    contents: Rc<Vec<Contents>>,
 }
 
 impl ContentRemove {
-    fn item(contents: Vec<Contents>) -> Box<dyn ContextMenuItem> {
+    fn item(contents: Rc<Vec<Contents>>) -> Box<dyn ContextMenuItem> {
         Box::new(Self { contents })
     }
 }
@@ -303,7 +366,7 @@ impl ContextMenuItem for ContentRemove {
     }
 
     fn run(&self, ctx: &ViewerContext<'_>, viewport_blueprint: &ViewportBlueprint) {
-        for content in &self.contents {
+        for content in &*self.contents {
             viewport_blueprint.mark_user_interaction(ctx);
             viewport_blueprint.remove_contents(*content);
         }
@@ -386,3 +449,46 @@ impl ContextMenuItem for AddSpaceView {
         viewport_blueprint.mark_user_interaction(ctx);
     }
 }
+
+// ---
+
+/// Move the selected contents to a newly created container of the given kind
+struct MoveContentsToNewContainer {
+    parent_container: ContainerId,
+    position_in_parent: usize,
+    container_kind: egui_tiles::ContainerKind,
+    contents: Rc<Vec<Contents>>,
+}
+
+impl MoveContentsToNewContainer {
+    fn item(
+        parent_container: ContainerId,
+        position_in_parent: usize,
+        container_kind: egui_tiles::ContainerKind,
+        contents: Rc<Vec<Contents>>,
+    ) -> Box<dyn ContextMenuItem> {
+        Box::new(Self {
+            parent_container,
+            position_in_parent,
+            container_kind,
+            contents,
+        })
+    }
+}
+
+impl ContextMenuItem for MoveContentsToNewContainer {
+    fn label(&self, _ctx: &ViewerContext<'_>, _viewport_blueprint: &ViewportBlueprint) -> String {
+        format!("{:?}", self.container_kind)
+    }
+
+    fn run(&self, ctx: &ViewerContext<'_>, viewport_blueprint: &ViewportBlueprint) {
+        viewport_blueprint.move_contents_to_new_container(
+            (*self.contents).clone(),
+            self.container_kind,
+            self.parent_container,
+            self.position_in_parent,
+        );
+
+        viewport_blueprint.mark_user_interaction(ctx);
+    }
+}

crates/re_viewport/src/viewport.rs

@@ -115,6 +115,14 @@ pub enum TreeAction {
         target_position_in_container: usize,
     },
 
+    /// Move one or more [`Contents`] to a newly created container
+    MoveContentsToNewContainer {
+        contents_to_move: Vec<Contents>,
+        new_container_kind: egui_tiles::ContainerKind,
+        target_container: ContainerId,
+        target_position_in_container: usize,
+    },
+
     /// Set the container that is currently identified as the drop target of an ongoing drag.
     ///
     /// This is used for highlighting the drop target in the UI. Note that the drop target container is reset at every
@@ -499,6 +507,36 @@ impl<'a, 'b> Viewport<'a, 'b> {
                     );
                     self.tree_edited = true;
                 }
+                TreeAction::MoveContentsToNewContainer {
+                    contents_to_move,
+                    new_container_kind,
+                    target_container,
+                    target_position_in_container,
+                } => {
+                    let new_container_tile_id = self
+                        .tree
+                        .tiles
+                        .insert_container(egui_tiles::Container::new(new_container_kind, vec![]));
+
+                    let target_container_tile_id = blueprint_id_to_tile_id(&target_container);
+                    self.tree.move_tile_to_container(
+                        new_container_tile_id,
+                        target_container_tile_id,
+                        target_position_in_container,
+                        true, // reflow grid if needed
+                    );
+
+                    for (pos, content) in contents_to_move.into_iter().enumerate() {
+                        self.tree.move_tile_to_container(
+                            content.as_tile_id(),
+                            new_container_tile_id,
+                            pos,
+                            true, // reflow grid if needed
+                        );
+                    }
+
+                    self.tree_edited = true;
+                }
                 TreeAction::SetDropTarget(container_id) => {
                     self.state.candidate_drop_parent_container_id = Some(container_id);
                 }

crates/re_viewport/src/viewport_blueprint.rs

@@ -460,6 +460,22 @@ impl ViewportBlueprint {
         });
     }
 
+    /// Move some [`Contents`] to a newly created container of the given kind.
+    pub fn move_contents_to_new_container(
+        &self,
+        contents: Vec<Contents>,
+        new_container_kind: egui_tiles::ContainerKind,
+        target_container: ContainerId,
+        target_position_in_container: usize,
+    ) {
+        self.send_tree_action(TreeAction::MoveContentsToNewContainer {
+            contents_to_move: contents,
+            new_container_kind,
+            target_container,
+            target_position_in_container,
+        });
+    }
+
     /// Make sure the tab corresponding to this space view is focused.
     pub fn focus_tab(&self, space_view_id: SpaceViewId) {
         self.send_tree_action(TreeAction::FocusTab(space_view_id));

tests/python/release_checklist/check_context_menu.py

@@ -15,6 +15,7 @@
 * Right-click on any space view and check for context menu content:
     - Hide
     - Remove
+    - Move to new container
 * Check both work as expected.
 * Right-click on the viewport and check for context menu content:
     - Add Container
@@ -23,6 +24,7 @@
 * Right-click on the container and check for context menu content:
     - Hide
     - Remove
+    - Move to new container
     - Add Container
     - Add Space View
 
@@ -41,9 +43,17 @@
 * Select multiple space views, right-click, and check for context menu content:
     - Hide
     - Remove
+    - Move to new container
 * Same as above, but with only containers selected.
 * Same as above, but with both space views and containers selected.
 * Same as above, but with the viewport selected as well. The context menu should be identical, but none of its actions should apply to the viewport.
+
+### Invalid sub-container kind
+
+* Single-select a horizontal container, check that it disallow adding an horizontal container inside it.
+* Same for a vertical container.
+* Single select a space view inside a horizontal container, check that it disallow moving to a new horizontal container.
+* Same for a space view inside a vertical container.
 """