From 9e2e5922837585608bfab22a9954eeec40d5a409 Mon Sep 17 00:00:00 2001 From: Johannes Schlatow Date: Wed, 5 Mar 2025 15:14:58 +0100 Subject: [PATCH] sculpt_manager: touch control of popup dialog When trying to control the popup dialog with touch events, the dialog was immediately closed (on any touch). This was a consequence of evaluating the dialog's hover state without waiting for the corresponding hover report. For regular motiong events, the hover report is updated when the pointer moves. For touch events, however, the hover report is only updated when the touch occurs. We therefore need to wait for the hover report that corresponds to the touch event before deciding about closing the popup dialog. Since menu_view's hover report is not updated for clicks/touches outside any dialog, we are now using nitpicker's hover report, which also got augmented by the sequence number in order to be correlated with the click/touch event. Fixes #5485 --- repos/gems/sculpt/leitzentrale/default | 5 +- .../sculpt_manager/dialog/distant_runtime.h | 2 +- repos/gems/src/app/sculpt_manager/main.cc | 64 +++++++++++++++---- repos/os/src/server/nitpicker/main.cc | 2 +- repos/os/src/server/nitpicker/user_state.cc | 11 +++- repos/os/src/server/nitpicker/user_state.h | 1 + 6 files changed, 67 insertions(+), 18 deletions(-) diff --git a/repos/gems/sculpt/leitzentrale/default b/repos/gems/sculpt/leitzentrale/default index c8f5f7632a..d4b6001af7 100644 --- a/repos/gems/sculpt/leitzentrale/default +++ b/repos/gems/sculpt/leitzentrale/default @@ -87,6 +87,7 @@ + @@ -116,6 +117,7 @@ + @@ -133,7 +135,7 @@ report="manager -> panel_dialog"/> - @@ -240,6 +242,7 @@ + diff --git a/repos/gems/src/app/sculpt_manager/dialog/distant_runtime.h b/repos/gems/src/app/sculpt_manager/dialog/distant_runtime.h index f0063b9f2f..55bfe34691 100644 --- a/repos/gems/src/app/sculpt_manager/dialog/distant_runtime.h +++ b/repos/gems/src/app/sculpt_manager/dialog/distant_runtime.h @@ -63,7 +63,7 @@ class Dialog::Distant_runtime : Noncopyable Top_level_dialog::Name _hovered_dialog { }; Sculpt::Rom_handler _hover_rom { - _env, "hover", *this, &Distant_runtime::_handle_hover }; + _env, "menu_hover", *this, &Distant_runtime::_handle_hover }; void _handle_hover(Xml_node const &); diff --git a/repos/gems/src/app/sculpt_manager/main.cc b/repos/gems/src/app/sculpt_manager/main.cc index c7133a4989..79f995c6d3 100644 --- a/repos/gems/src/app/sculpt_manager/main.cc +++ b/repos/gems/src/app/sculpt_manager/main.cc @@ -762,12 +762,35 @@ struct Sculpt::Main : Input_event_handler, Rom_handler
_nitpicker_hover_handler { _env, "nitpicker_hover", *this, &Main::_handle_nitpicker_hover }; + Rom_handler
_hover_handler { + _env, "hover", *this, &Main::_handle_hover }; + Expanding_reporter _gui_fb_config { _env, "config", "gui_fb_config" }; Constructible _pointer_pos { }; Fb_connectors::Name _hovered_display { }; + void _handle_hover(Xml_node const &hover) + { + using Label = String<128>; + + Label label { hover.attribute_value("label", Label()) }; + Label suffix { "popup_dialog" }; + + _popup_hovered = false; + if (label.length() >= suffix.length()) { + size_t const offset = label.length() - suffix.length(); + + if (!strcmp(label.string() + offset, suffix.string())) + _popup_hovered = true; + } + + _hover_seq_number = { hover.attribute_value("seq_number", 0U) }; + + _try_handle_click(); + } + void _handle_nitpicker_hover(Xml_node const &hover) { if (hover.has_attribute("xpos")) @@ -942,7 +965,11 @@ struct Sculpt::Main : Input_event_handler, /* used to prevent closing the popup immediatedly after opened */ Input::Seq_number _popup_opened_seq_number { }; + /* used to correlate clicks and hover reports */ + Input::Seq_number _hover_seq_number { }; Input::Seq_number _clicked_seq_number { }; + Input::Seq_number _last_clicked_seq_number { }; + bool _popup_hovered { false }; /** * Input_event_handler interface @@ -962,23 +989,11 @@ struct Sculpt::Main : Input_event_handler, * Detect clicks outside the popup dialog (for closing it) */ if (ev.key_press(Input::BTN_LEFT) || ev.touch()) { - _clicked_seq_number = _global_input_seq_number; - - bool const popup_opened = - (_popup_opened_seq_number.value == _clicked_seq_number.value); - - bool const popup_hovered = - _popup_dialog.if_hovered([&] (Hovered_at const &) { return true; }); - - if (!popup_hovered && !popup_opened) { - if (_popup.state == Popup::VISIBLE) { - _close_popup_dialog(); - discard_construction(); - } - } + _try_handle_click(); } + bool need_generate_dialog = false; ev.handle_press([&] (Input::Keycode, Codepoint code) { @@ -1156,6 +1171,27 @@ struct Sculpt::Main : Input_event_handler, _update_window_layout(); } + void _try_handle_click() + { + /* skip if already handled */ + if (_last_clicked_seq_number.value == _clicked_seq_number.value) + return; + + /* wait for hover to be updated */ + if (_clicked_seq_number.value != _hover_seq_number.value) + return; + + _last_clicked_seq_number = _clicked_seq_number; + + bool const popup_opened = + (_popup_opened_seq_number.value == _clicked_seq_number.value); + + if ((_popup.state == Popup::VISIBLE) && !_popup_hovered && !popup_opened) { + _close_popup_dialog(); + discard_construction(); + } + } + void _refresh_panel_and_window_layout() { _panel_dialog.refresh(); diff --git a/repos/os/src/server/nitpicker/main.cc b/repos/os/src/server/nitpicker/main.cc index f1a24ad859..66d77af7b5 100644 --- a/repos/os/src/server/nitpicker/main.cc +++ b/repos/os/src/server/nitpicker/main.cc @@ -960,7 +960,7 @@ void Nitpicker::Main::handle_input_events(User_state::Input_batch batch) _view_stack.update_all_views(); } - if (result.hover_changed) + if (result.hover_changed || result.last_seq_changed) _hover_count++; /* report mouse-position updates */ diff --git a/repos/os/src/server/nitpicker/user_state.cc b/repos/os/src/server/nitpicker/user_state.cc index 91ceb57af1..61c68a1555 100644 --- a/repos/os/src/server/nitpicker/user_state.cc +++ b/repos/os/src/server/nitpicker/user_state.cc @@ -342,6 +342,8 @@ User_state::handle_input_events(Input_batch batch) View_owner const * const old_input_receiver = _input_receiver; View_owner const * const old_last_clicked = _last_clicked; unsigned const old_clicked_count = _clicked_count; + unsigned const old_seq_number = _last_seq_number.constructed() + ? _last_seq_number->value : 0; bool button_activity = false; @@ -424,6 +426,9 @@ User_state::handle_input_events(Input_batch batch) [&] (Nowhere) { return _pointer.ok(); }); }; + bool const last_seq_changed = _last_seq_number.constructed() + && _last_seq_number->value != old_seq_number; + return { .hover_changed = _hovered != old_hovered, .focus_changed = (_focused != old_focused) || @@ -432,7 +437,8 @@ User_state::handle_input_events(Input_batch batch) .button_activity = button_activity, .motion_activity = pointer_changed() || touch_occurred, .key_pressed = _key_pressed(), - .last_clicked_changed = last_clicked_changed + .last_clicked_changed = last_clicked_changed, + .last_seq_changed = last_seq_changed }; } @@ -460,6 +466,9 @@ void User_state::report_hovered_view_owner(Xml_generator &xml, bool active) cons _hovered->report(xml); if (active) xml.attribute("active", "yes"); + + if (_last_seq_number.constructed()) + xml.attribute("seq_number", _last_seq_number->value); } diff --git a/repos/os/src/server/nitpicker/user_state.h b/repos/os/src/server/nitpicker/user_state.h index 5e688b849f..2296aba7fc 100644 --- a/repos/os/src/server/nitpicker/user_state.h +++ b/repos/os/src/server/nitpicker/user_state.h @@ -252,6 +252,7 @@ class Nitpicker::User_state bool const motion_activity; bool const key_pressed; bool const last_clicked_changed; + bool const last_seq_changed; }; Handle_input_result handle_input_events(Input_batch);