Fix undo/redo selection shortcut/action changing selection history without changing selection

crates/re_viewer/src/misc/selection_state.rs

@@ -7,7 +7,7 @@ use re_data_store::{EntityPath, InstancePath, InstancePathHash, LogDb};
 use re_log_types::{component_types::InstanceKey, EntityPathHash};
 use re_renderer::OutlineMaskPreference;
 
-use crate::ui::{Blueprint, HistoricalSelection, SelectionHistory, SpaceView, SpaceViewId};
+use crate::ui::{Blueprint, SelectionHistory, SpaceView, SpaceViewId};
 
 use super::{Item, ItemCollection};
 
@@ -205,13 +205,17 @@ impl SelectionState {
     }
 
     /// Selects the previous element in the history if any.
-    pub fn select_previous(&mut self) -> Option<HistoricalSelection> {
-        self.history.select_previous()
+    pub fn select_previous(&mut self) {
+        if let Some(selection) = self.history.select_previous() {
+            self.selection = selection;
+        }
     }
 
     /// Selections the next element in the history if any.
-    pub fn select_next(&mut self) -> Option<HistoricalSelection> {
-        self.history.select_next()
+    pub fn select_next(&mut self) {
+        if let Some(selection) = self.history.select_next() {
+            self.selection = selection;
+        }
     }
 
     /// Clears the current selection out.

crates/re_viewer/src/ui/selection_history_ui.rs

@@ -1,7 +1,7 @@
 use egui::RichText;
 use re_ui::Command;
 
-use super::{HistoricalSelection, SelectionHistory};
+use super::SelectionHistory;
 use crate::{misc::ItemCollection, ui::Blueprint, Item};
 
 // ---
@@ -14,15 +14,14 @@ impl SelectionHistory {
         blueprint: &Blueprint,
     ) -> Option<ItemCollection> {
         self.control_bar_ui(re_ui, ui, blueprint)
-            .map(|sel| sel.selection)
     }
 
     fn control_bar_ui(
         &mut self,
         re_ui: &re_ui::ReUi,
         ui: &mut egui::Ui,
         blueprint: &Blueprint,
-    ) -> Option<HistoricalSelection> {
+    ) -> Option<ItemCollection> {
         ui.horizontal_centered(|ui| {
             ui.strong("Selection").on_hover_text("The Selection View contains information and options about the currently selected object(s).");
 
@@ -38,27 +37,23 @@ impl SelectionHistory {
         }).inner
     }
 
-    // TODO(cmc): note that for now, we only check prev/next shortcuts in the UI code that
-    // shows the associated buttons... this means shortcuts only work when the selection panel
-    // is open!
-    // We might want to change this at some point, though the way things are currently designed,
-    // there isn't much point in selecting stuff while the selection panel is hidden anyway.
-
-    pub fn select_previous(&mut self) -> Option<HistoricalSelection> {
+    #[must_use]
+    pub fn select_previous(&mut self) -> Option<ItemCollection> {
         if let Some(previous) = self.previous() {
             if previous.index != self.current {
                 self.current = previous.index;
-                return self.current();
+                return self.current().map(|s| s.selection);
             }
         }
         None
     }
 
-    pub fn select_next(&mut self) -> Option<HistoricalSelection> {
+    #[must_use]
+    pub fn select_next(&mut self) -> Option<ItemCollection> {
         if let Some(next) = self.next() {
             if next.index != self.current {
                 self.current = next.index;
-                return self.current();
+                return self.current().map(|s| s.selection);
             }
         }
         None
@@ -69,7 +64,7 @@ impl SelectionHistory {
         re_ui: &re_ui::ReUi,
         ui: &mut egui::Ui,
         blueprint: &Blueprint,
-    ) -> Option<HistoricalSelection> {
+    ) -> Option<ItemCollection> {
         // undo selection
         if let Some(previous) = self.previous() {
             let response = re_ui
@@ -112,7 +107,7 @@ impl SelectionHistory {
         re_ui: &re_ui::ReUi,
         ui: &mut egui::Ui,
         blueprint: &Blueprint,
-    ) -> Option<HistoricalSelection> {
+    ) -> Option<ItemCollection> {
         // redo selection
         if let Some(next) = self.next() {
             let response = re_ui