From 2c772d184882dbdce0ec532f59c87743819f2e0d Mon Sep 17 00:00:00 2001 From: Jelle Raaijmakers Date: Tue, 25 May 2021 22:48:43 +0200 Subject: [PATCH] LibGUI/AbstractView: Remove `on_selection` Since the introduction of multi-select, we have had both `on_selection` and `on_selection_change`, the latter of which was only invoked when a change in selection came in through the model. This removes `AbstractView::on_selection` and replaces it usage with the more explicit `on_selection_change` everywhere. --- Userland/Applications/Browser/InspectorWidget.cpp | 6 ++++-- Userland/Applications/FileManager/main.cpp | 3 ++- Userland/Applications/Help/main.cpp | 15 ++++++++------- .../Applications/Spreadsheet/CellTypeDialog.cpp | 3 ++- Userland/DevTools/HackStudio/ClassViewWidget.cpp | 3 ++- .../HackStudio/Debugger/DebugInfoWidget.cpp | 3 ++- Userland/DevTools/HackStudio/Git/GitWidget.cpp | 3 ++- Userland/DevTools/Profiler/main.cpp | 6 ++++-- Userland/Libraries/LibGUI/AbstractView.cpp | 6 ++---- Userland/Libraries/LibGUI/AbstractView.h | 1 - Userland/Libraries/LibGUI/ComboBox.cpp | 3 ++- Userland/Libraries/LibGUI/FontPicker.cpp | 9 ++++++--- 12 files changed, 36 insertions(+), 25 deletions(-) diff --git a/Userland/Applications/Browser/InspectorWidget.cpp b/Userland/Applications/Browser/InspectorWidget.cpp index a8053d8c4e9..115f1ed2818 100644 --- a/Userland/Applications/Browser/InspectorWidget.cpp +++ b/Userland/Applications/Browser/InspectorWidget.cpp @@ -41,13 +41,15 @@ InspectorWidget::InspectorWidget() auto& top_tab_widget = splitter.add(); m_dom_tree_view = top_tab_widget.add_tab("DOM"); - m_dom_tree_view->on_selection = [this](auto& index) { + m_dom_tree_view->on_selection_change = [this] { + const auto& index = m_dom_tree_view->selection().first(); auto* node = static_cast(index.internal_data()); set_inspected_node(node); }; m_layout_tree_view = top_tab_widget.add_tab("Layout"); - m_layout_tree_view->on_selection = [this](auto& index) { + m_layout_tree_view->on_selection_change = [this] { + const auto& index = m_layout_tree_view->selection().first(); auto* node = static_cast(index.internal_data()); set_inspected_node(node->dom_node()); }; diff --git a/Userland/Applications/FileManager/main.cpp b/Userland/Applications/FileManager/main.cpp index 61a7ddf8a70..9c7692724a4 100644 --- a/Userland/Applications/FileManager/main.cpp +++ b/Userland/Applications/FileManager/main.cpp @@ -1049,7 +1049,8 @@ int run_in_windowed_mode(RefPtr config, String initial_locatio } }; - tree_view.on_selection = [&](const GUI::ModelIndex& index) { + tree_view.on_selection_change = [&] { + const auto& index = tree_view.selection().first(); if (directories_model->m_previously_selected_index.is_valid()) directories_model->update_node_on_selection(directories_model->m_previously_selected_index, false); diff --git a/Userland/Applications/Help/main.cpp b/Userland/Applications/Help/main.cpp index 9a7f8fdaef0..8f61bcc0cd4 100644 --- a/Userland/Applications/Help/main.cpp +++ b/Userland/Applications/Help/main.cpp @@ -179,24 +179,25 @@ int main(int argc, char* argv[]) GUI::MessageBox::Type::Error); } }; - search_list_view.on_selection = [&](auto index) { + search_list_view.on_selection_change = [&] { + const auto& index = search_list_view.selection().first(); if (!index.is_valid()) return; - if (auto model = search_list_view.model()) { - auto& search_model = *static_cast(model); - index = search_model.map(index); - } else { + auto view_model = search_list_view.model(); + if (!view_model) { page_view.load_empty_document(); return; } - String path = model->page_path(index); + auto& search_model = *static_cast(view_model); + const auto& mapped_index = search_model.map(index); + String path = model->page_path(mapped_index); if (path.is_null()) { page_view.load_empty_document(); return; } tree_view.selection().clear(); - tree_view.selection().add(index); + tree_view.selection().add(mapped_index); history.push(path); update_actions(); open_page(path); diff --git a/Userland/Applications/Spreadsheet/CellTypeDialog.cpp b/Userland/Applications/Spreadsheet/CellTypeDialog.cpp index 4153a05b40a..a1b6b9b75e4 100644 --- a/Userland/Applications/Spreadsheet/CellTypeDialog.cpp +++ b/Userland/Applications/Spreadsheet/CellTypeDialog.cpp @@ -141,7 +141,8 @@ void CellTypeDialog::setup_tabs(GUI::TabWidget& tabs, const Vector& po auto& type_list = left_side.add(); type_list.set_model(*GUI::ItemListModel::create(g_types)); type_list.set_should_hide_unnecessary_scrollbars(true); - type_list.on_selection = [&](auto& index) { + type_list.on_selection_change = [&] { + const auto& index = type_list.selection().first(); if (!index.is_valid()) { m_type = nullptr; return; diff --git a/Userland/DevTools/HackStudio/ClassViewWidget.cpp b/Userland/DevTools/HackStudio/ClassViewWidget.cpp index 4f05ac67f6e..9e8c9f6e4df 100644 --- a/Userland/DevTools/HackStudio/ClassViewWidget.cpp +++ b/Userland/DevTools/HackStudio/ClassViewWidget.cpp @@ -19,7 +19,8 @@ ClassViewWidget::ClassViewWidget() set_layout(); m_class_tree = add(); - m_class_tree->on_selection = [this](auto& index) { + m_class_tree->on_selection_change = [this] { + const auto& index = m_class_tree->selection().first(); if (!index.is_valid()) return; diff --git a/Userland/DevTools/HackStudio/Debugger/DebugInfoWidget.cpp b/Userland/DevTools/HackStudio/Debugger/DebugInfoWidget.cpp index f058c7d4065..e626ff8a2ac 100644 --- a/Userland/DevTools/HackStudio/Debugger/DebugInfoWidget.cpp +++ b/Userland/DevTools/HackStudio/Debugger/DebugInfoWidget.cpp @@ -65,7 +65,8 @@ DebugInfoWidget::DebugInfoWidget() variables_tab_widget.add_widget("Variables", build_variables_tab()); variables_tab_widget.add_widget("Registers", build_registers_tab()); - m_backtrace_view->on_selection = [this](auto& index) { + m_backtrace_view->on_selection_change = [this] { + const auto& index = m_backtrace_view->selection().first(); auto& model = static_cast(*m_backtrace_view->model()); // Note: The reconstruction of the register set here is obviously incomplete. diff --git a/Userland/DevTools/HackStudio/Git/GitWidget.cpp b/Userland/DevTools/HackStudio/Git/GitWidget.cpp index d2dc23ff21b..39fb4a87f3a 100644 --- a/Userland/DevTools/HackStudio/Git/GitWidget.cpp +++ b/Userland/DevTools/HackStudio/Git/GitWidget.cpp @@ -44,7 +44,8 @@ GitWidget::GitWidget(const LexicalPath& repo_root) m_unstaged_files = unstaged.add( [this](const auto& file) { stage_file(file); }, Gfx::Bitmap::load_from_file("/res/icons/16x16/plus.png").release_nonnull()); - m_unstaged_files->on_selection = [this](const GUI::ModelIndex& index) { + m_unstaged_files->on_selection_change = [this] { + const auto& index = m_unstaged_files->selection().first(); const auto& selected = index.data().as_string(); show_diff(LexicalPath(selected)); }; diff --git a/Userland/DevTools/Profiler/main.cpp b/Userland/DevTools/Profiler/main.cpp index 6f85fef7a00..c40c95b1399 100644 --- a/Userland/DevTools/Profiler/main.cpp +++ b/Userland/DevTools/Profiler/main.cpp @@ -143,7 +143,8 @@ int main(int argc, char** argv) auto& disassembly_view = bottom_splitter.add(); disassembly_view.set_visible(false); - tree_view.on_selection = [&](auto& index) { + tree_view.on_selection_change = [&] { + const auto& index = tree_view.selection().first(); profile->set_disassembly_index(index); disassembly_view.set_model(profile->disassembly_model()); }; @@ -161,7 +162,8 @@ int main(int argc, char** argv) samples_table_view.set_model(profile->samples_model()); auto& individual_sample_view = samples_splitter.add(); - samples_table_view.on_selection = [&](const GUI::ModelIndex& index) { + samples_table_view.on_selection_change = [&] { + const auto& index = samples_table_view.selection().first(); auto model = IndividualSampleModel::create(*profile, index.data(GUI::ModelRole::Custom).to_integer()); individual_sample_view.set_model(move(model)); }; diff --git a/Userland/Libraries/LibGUI/AbstractView.cpp b/Userland/Libraries/LibGUI/AbstractView.cpp index ba1951b6275..1180054bf0f 100644 --- a/Userland/Libraries/LibGUI/AbstractView.cpp +++ b/Userland/Libraries/LibGUI/AbstractView.cpp @@ -110,8 +110,8 @@ void AbstractView::did_update_selection() { if (!model() || selection().first() != m_edit_index) stop_editing(); - if (model() && on_selection && selection().first().is_valid()) - on_selection(selection().first()); + if (model() && on_selection_change) + on_selection_change(); } void AbstractView::did_scroll() @@ -194,8 +194,6 @@ void AbstractView::activate_selected() void AbstractView::notify_selection_changed(Badge) { did_update_selection(); - if (on_selection_change) - on_selection_change(); if (!m_suppress_update_on_selection_change) update(); } diff --git a/Userland/Libraries/LibGUI/AbstractView.h b/Userland/Libraries/LibGUI/AbstractView.h index 065beeff64d..5aa529354d5 100644 --- a/Userland/Libraries/LibGUI/AbstractView.h +++ b/Userland/Libraries/LibGUI/AbstractView.h @@ -98,7 +98,6 @@ public: Function on_selection_change; Function on_activation; - Function on_selection; Function on_context_menu_request; Function on_drop; diff --git a/Userland/Libraries/LibGUI/ComboBox.cpp b/Userland/Libraries/LibGUI/ComboBox.cpp index a3ac80f17f1..fa676520b33 100644 --- a/Userland/Libraries/LibGUI/ComboBox.cpp +++ b/Userland/Libraries/LibGUI/ComboBox.cpp @@ -101,8 +101,9 @@ ComboBox::ComboBox() m_list_view->set_hover_highlighting(true); m_list_view->set_frame_thickness(1); m_list_view->set_frame_shadow(Gfx::FrameShadow::Plain); - m_list_view->on_selection = [this](auto& index) { + m_list_view->on_selection_change = [this] { VERIFY(model()); + const auto& index = m_list_view->selection().first(); m_list_view->set_activates_on_selection(true); if (m_updating_model) selection_updated(index); diff --git a/Userland/Libraries/LibGUI/FontPicker.cpp b/Userland/Libraries/LibGUI/FontPicker.cpp index 140c094f955..ea592bef79f 100644 --- a/Userland/Libraries/LibGUI/FontPicker.cpp +++ b/Userland/Libraries/LibGUI/FontPicker.cpp @@ -56,7 +56,8 @@ FontPicker::FontPicker(Window* parent_window, const Gfx::Font* current_font, boo }); quick_sort(m_families); - m_family_list_view->on_selection = [this](auto& index) { + m_family_list_view->on_selection_change = [this] { + const auto& index = m_family_list_view->selection().first(); m_family = index.data().to_string(); m_weights.clear(); Gfx::FontDatabase::the().for_each_typeface([&](auto& typeface) { @@ -76,7 +77,8 @@ FontPicker::FontPicker(Window* parent_window, const Gfx::Font* current_font, boo update_font(); }; - m_weight_list_view->on_selection = [this](auto& index) { + m_weight_list_view->on_selection_change = [this] { + const auto& index = m_weight_list_view->selection().first(); bool font_is_fixed_size = false; m_weight = index.data(ModelRole::Custom).to_i32(); m_sizes.clear(); @@ -130,7 +132,8 @@ FontPicker::FontPicker(Window* parent_window, const Gfx::Font* current_font, boo update_font(); }; - m_size_list_view->on_selection = [this](auto& index) { + m_size_list_view->on_selection_change = [this] { + const auto& index = m_size_list_view->selection().first(); m_size = index.data().to_i32(); m_size_spin_box->set_value(m_size.value()); update_font();