From 2b05ac25bc096cd9f38da5a5c4dea35344b01371 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Mon, 30 Sep 2024 10:09:28 +0200 Subject: [PATCH 1/3] Do not deselect on ESC when it was used to close some other UI element --- crates/viewer/re_ui/src/modal.rs | 3 ++- crates/viewer/re_viewer/src/app_state.rs | 12 ++++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/viewer/re_ui/src/modal.rs b/crates/viewer/re_ui/src/modal.rs index b2cc9d03820c..ba8048e15a01 100644 --- a/crates/viewer/re_ui/src/modal.rs +++ b/crates/viewer/re_ui/src/modal.rs @@ -164,7 +164,8 @@ impl Modal { ) -> ModalResponse { Self::dim_background(ctx); - let mut open = ctx.input(|i| !i.key_pressed(egui::Key::Escape)); + // We consume such as to avoid the top-level deselect-on-ESC behavior. + let mut open = ctx.input_mut(|i| !i.consume_key(egui::Modifiers::NONE, egui::Key::Escape)); let screen_height = ctx.screen_rect().height(); let modal_vertical_margins = (75.0).at_most(screen_height * 0.1); diff --git a/crates/viewer/re_viewer/src/app_state.rs b/crates/viewer/re_viewer/src/app_state.rs index 4c39a8ae9951..6ecf43220216 100644 --- a/crates/viewer/re_viewer/src/app_state.rs +++ b/crates/viewer/re_viewer/src/app_state.rs @@ -202,10 +202,6 @@ impl AppState { )), ); - if ui.input(|i| i.key_pressed(egui::Key::Escape)) { - selection_state.clear_selection(); - } - let applicable_entities_per_visualizer = space_view_class_registry .applicable_entities_for_visualizer_systems(recording.store_id()); let indicated_entities_per_visualizer = @@ -470,6 +466,14 @@ impl AppState { // This must run after any ui code, or other code that tells egui to open an url: check_for_clicked_hyperlinks(&egui_ctx, ctx.selection_state); + // Deselect on ESC. Must happen after all other UI code to let them capture ESC if needed. + if ui.input(|i| i.key_pressed(egui::Key::Escape)) + && !ui.ctx().is_context_menu_open() + && !ui.memory(|m| m.any_popup_open()) + { + selection_state.clear_selection(); + } + // Reset the focused item. *focused_item = None; } From 68e140df421af6b4b4b6c076d9254d56617ba0d3 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Mon, 30 Sep 2024 10:55:12 +0200 Subject: [PATCH 2/3] Fix code + release checklist --- crates/viewer/re_context_menu/src/lib.rs | 2 +- crates/viewer/re_viewer/src/app_state.rs | 8 ++-- .../check_deselect_on_escape.py | 48 +++++++++++++++++++ 3 files changed, 53 insertions(+), 5 deletions(-) create mode 100644 tests/python/release_checklist/check_deselect_on_escape.py diff --git a/crates/viewer/re_context_menu/src/lib.rs b/crates/viewer/re_context_menu/src/lib.rs index 2ca4209c9f4b..603d503b93cc 100644 --- a/crates/viewer/re_context_menu/src/lib.rs +++ b/crates/viewer/re_context_menu/src/lib.rs @@ -43,7 +43,7 @@ pub fn context_menu_ui_for_item( selection_update_behavior: SelectionUpdateBehavior, ) { item_response.context_menu(|ui| { - if ui.input(|i| i.key_pressed(egui::Key::Escape)) { + if ui.input_mut(|i| i.consume_key(egui::Modifiers::NONE, egui::Key::Escape)) { ui.close_menu(); return; } diff --git a/crates/viewer/re_viewer/src/app_state.rs b/crates/viewer/re_viewer/src/app_state.rs index 6ecf43220216..7e85957a0cbc 100644 --- a/crates/viewer/re_viewer/src/app_state.rs +++ b/crates/viewer/re_viewer/src/app_state.rs @@ -157,6 +157,9 @@ impl AppState { focused_item, } = self; + // check state early, before the UI has a chance to close these popups + let is_any_popup_open = ui.memory(|m| m.any_popup_open()); + // Some of the mutations APIs of `ViewportBlueprints` are recorded as `Viewport::TreeAction` // and must be applied by `Viewport` at the end of the frame. We use a temporary channel for // this, which gives us interior mutability (only a shared reference of `ViewportBlueprint` @@ -467,10 +470,7 @@ impl AppState { check_for_clicked_hyperlinks(&egui_ctx, ctx.selection_state); // Deselect on ESC. Must happen after all other UI code to let them capture ESC if needed. - if ui.input(|i| i.key_pressed(egui::Key::Escape)) - && !ui.ctx().is_context_menu_open() - && !ui.memory(|m| m.any_popup_open()) - { + if ui.input(|i| i.key_pressed(egui::Key::Escape)) && !is_any_popup_open { selection_state.clear_selection(); } diff --git a/tests/python/release_checklist/check_deselect_on_escape.py b/tests/python/release_checklist/check_deselect_on_escape.py new file mode 100644 index 000000000000..f68a78a02464 --- /dev/null +++ b/tests/python/release_checklist/check_deselect_on_escape.py @@ -0,0 +1,48 @@ +from __future__ import annotations + +import os +from argparse import Namespace +from uuid import uuid4 + +import rerun as rr + +README = """\ +# Deselect on Escape + +This checks that the deselect-on-escape works as expected. + +### Actions + +* Base behavior: select the 3D view and hit ESC => the view is no longer selected. + +In all the following cases, the 3D view should *remain selected*. + +* Select the 3D view, open the "Background Kind" dropdown in the selection panel, and hit ESC. +* Select the 3D view, open the "Add Space View or Container" modal, and hit ESC. +* Select the 3D view, right-click on it in the blueprint tree, and hit ESC. +* Select the 3D view, open the Rerun menu, and hit ESC (KNOWN FAIL). +""" + + +def log_readme() -> None: + rr.log("readme", rr.TextDocument(README, media_type=rr.MediaType.MARKDOWN), timeless=True) + + +def log_some_data() -> None: + rr.log("data", rr.Points3D([0, 0, 0])) + + +def run(args: Namespace) -> None: + rr.script_setup(args, f"{os.path.basename(__file__)}", recording_id=uuid4()) + + log_readme() + log_some_data() + + +if __name__ == "__main__": + import argparse + + parser = argparse.ArgumentParser(description="Interactive release checklist") + rr.script_add_args(parser) + args = parser.parse_args() + run(args) From e0a0aa046d395cbeed2375cb0b84c9637dafa047 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Mon, 30 Sep 2024 11:06:01 +0200 Subject: [PATCH 3/3] lint --- tests/python/release_checklist/check_deselect_on_escape.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/python/release_checklist/check_deselect_on_escape.py b/tests/python/release_checklist/check_deselect_on_escape.py index f68a78a02464..fcbb5ca1603e 100644 --- a/tests/python/release_checklist/check_deselect_on_escape.py +++ b/tests/python/release_checklist/check_deselect_on_escape.py @@ -18,7 +18,7 @@ In all the following cases, the 3D view should *remain selected*. * Select the 3D view, open the "Background Kind" dropdown in the selection panel, and hit ESC. -* Select the 3D view, open the "Add Space View or Container" modal, and hit ESC. +* Select the 3D view, open the "Add space view or container" modal, and hit ESC. * Select the 3D view, right-click on it in the blueprint tree, and hit ESC. * Select the 3D view, open the Rerun menu, and hit ESC (KNOWN FAIL). """