Controller preferences: add searchbars to mapping tables #11165

ronso0 · 2022-12-29T23:25:15Z

Adds simple searchbars above the mapping control tables:

Actually I used WSearchLineEdits from the beginning, but with those, showing the Controller preferences (first show, overview tab) would randomly slow down the GUI and glitches occured as if something was processed concurrently (or while the model was about to be built), but I just couldn't figure out what exactly was causing this.
The branch is https://github.com/ronso0/mixxx/tree/mapping-table-search_wsearch, in case you have some spare minutes to take a fresh look at it.

…g Return in searchbars

…o base class ... and add respective new Qt6 method

daschuer

Thank you. Just a few comments for improving code.
A test is pending.

daschuer · 2023-01-03T09:17:49Z

src/controllers/controlleroutputmappingtablemodel.cpp

@@ -189,6 +190,53 @@ QVariant ControllerOutputMappingTableModel::data(const QModelIndex& index,
    return QVariant();
 }

+QString ControllerOutputMappingTableModel::displayString(const QModelIndex& index) const {


It is not clear from the function name if the function is an imperative or a getter.
Can we improve it? Something like getStringForSearch()

Alternative we may integrate the code into data() itself using a new user role
e.g. SearchRole = Qt::UserRole + 1

True, let's call it getDisplayString then?

Tbh I'd rather stick to explicit QString getDisplayString instead of having to deal with even more type conversion and more safety checks in data().

Thanks for the idea to use a custom role!

src/controllers/controlleroutputmappingtablemodel.cpp

daschuer · 2023-01-03T09:57:24Z

src/controllers/controllermappingtablemodel.cpp

+            if (tokenMatch) {
+                break;
+            }
+            column++;


This is the main use case for the good old for() loop.

daschuer · 2023-01-03T10:13:21Z

What is the state of https://github.com/ronso0/mixxx/tree/mapping-table-search_wsearch?

Is the issue?

/__w/mixxx/mixxx/src/controllers/controllerinputmappingtablemodel.cpp: In member function ‘virtual QString ControllerInputMappingTableModel::displayString(const QModelIndex&) const’:
Error: /__w/mixxx/mixxx/src/controllers/controllerinputmappingtablemodel.cpp:242:78: error: ‘QAbstractItemDelegate* QAbstractItemView::itemDelegate(const QModelIndex&) const’ is deprecated: Use itemDelegateForIndex instead [-Werror=deprecated-declarations]
  242 |                 qobject_cast<QStyledItemDelegate*>(m_pTableView->itemDelegate(index));
      |                                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~

I think here we need to use itemDelegateForIndex() conditionally when building with Qt6

ronso0 · 2023-01-03T11:18:29Z

What is the state of https://github.com/ronso0/mixxx/tree/mapping-table-search_wsearch?

It's a test / WIP until someone can figure out the cause.

Is the issue?

/__w/mixxx/mixxx/src/controllers/controllerinputmappingtablemodel.cpp: In member function ‘virtual QString ControllerInputMappingTableModel::displayString(const QModelIndex&) const’:
Error: /__w/mixxx/mixxx/src/controllers/controllerinputmappingtablemodel.cpp:242:78: error: ‘QAbstractItemDelegate* QAbstractItemView::itemDelegate(const QModelIndex&) const’ is deprecated: Use itemDelegateForIndex instead [-Werror=deprecated-declarations]
  242 |                 qobject_cast<QStyledItemDelegate*>(m_pTableView->itemDelegate(index));
      |                                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~

I think here we need to use itemDelegateForIndex() conditionally when building with Qt6

No, that's unrelated to WSearchLineedit. Locally with Qt5 it builds and runs fine, only CI with Qt6 failed because of this.
It's really just QLineEdit + Search button pressed vs. WSearchLineEdit::search signal, and that is emitted as expected just after entering a search phrase. I'll try WSearchLineEdit + explicit search button, maybe I can narrow it down.

daschuer · 2023-01-08T22:41:29Z

src/controllers/dlgprefcontroller.cpp

+    connect(m_ui.inputControlSearchBtn,
+            &QPushButton::clicked,
+            this,
+            &DlgPrefController::slotInputControlSearch);


These connections can be done in the constructor.
This is also an issue in https://github.com/ronso0/mixxx/tree/mapping-table-search_wsearch
after a while these controls are 10 times connected and a search is done that often slowing down the process.
After moving them to the constructor there is no longer an extra GUI lag.

Another possible issue is that ControllerMappingTableProxyModel::filterAcceptsRow() is quite slow.
In case of searching during typing, it is called often but is acting always on the same data.
Maybe there is a way to cache the heavy work of retreaving and fromating all strinsg over and over again.

oh, true, I moved the conection to the constructor.

However that is not the issue with WSearchLineEdit since the GUI lockup occurs when the mapping is shown for the first time (only one conection) and no search signal has been fired, yet.
I guess it's related to some show/resize event or a conflicting QWidget/WWidget method, because occasionally I also experience another issue with the WSearchLineEdit in DlgDeveloperTools (dialog is extremly wide, ~10x screen width).
Since the current simple search works well I'm not motivated to debug that further right now.

In case of searching during typing, it is called often but is acting always on the same data.
Maybe there is a way to cache the heavy work of retreaving and fromating all strinsg over and over again.

With the simple search there is no search-as-you-type, it's only triggered by Return or the Search button (and after editing items).
Also, data and the delegates' displayText get called every time the view changes (scroll, show, hide ...) so I'm not sure if custom caching is worth the effort for this view of rather small data models (after all, even with huge mappings, each table holds like ~200 items max?).

daschuer

LGTM, Thank you.

github-actions bot added the ui label Dec 29, 2022

ronso0 added controller mappings preferences labels Dec 29, 2022

ronso0 force-pushed the mapping-table-search branch from 02ac407 to a7fed6b Compare December 30, 2022 00:59

ronso0 added 5 commits January 3, 2023 01:17

Controller mapping table: add custom QSortFilterProxyModel

13f1beb

Controller mapping table: add proxy model filter

58955bb

Controller mapping tables: add searchbars, connect to custom proxy model

07f2662

Controller mapping preferences: don't click Apply button when pressin…

32e7bba

…g Return in searchbars

ControllerMappingTableModel: move finding delegate and qobject_cast t…

2f7a3a6

…o base class ... and add respective new Qt6 method

ronso0 force-pushed the mapping-table-search branch from a7fed6b to 2f7a3a6 Compare January 3, 2023 00:34

daschuer requested changes Jan 3, 2023

View reviewed changes

ronso0 added 2 commits January 3, 2023 15:25

Controller mapping table models: displayString > getDisplayString

831f289

ControllerMappingTableProxyModel::filterAcceptsRow: use for lopp

f040da3

ronso0 mentioned this pull request Jan 3, 2023

Controller mapping table: show readable/translated strings for script bindings #11139

Merged

Merge remote-tracking branch 'mixxx/main' into mapping-table-search

4497ba1

daschuer requested changes Jan 8, 2023

View reviewed changes

Controller mapping tables: connect seach signals in constructor

8c878c4

daschuer approved these changes Jan 10, 2023

View reviewed changes

daschuer merged commit 9c4e2f1 into mixxxdj:main Jan 10, 2023

ronso0 deleted the mapping-table-search branch January 10, 2023 08:23

ronso0 mentioned this pull request Sep 27, 2023

add searchbar to controller mapping tables #11140

Closed

ronso0 mentioned this pull request Feb 4, 2024

Feature: make controller mapping rows more accessible #12720

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Controller preferences: add searchbars to mapping tables #11165

Controller preferences: add searchbars to mapping tables #11165

ronso0 commented Dec 29, 2022

daschuer left a comment

daschuer Jan 3, 2023

ronso0 Jan 3, 2023

daschuer Jan 3, 2023

ronso0 Jan 3, 2023

daschuer Jan 3, 2023

ronso0 Jan 3, 2023

daschuer commented Jan 3, 2023

ronso0 commented Jan 3, 2023 •

edited

Loading

daschuer Jan 8, 2023

ronso0 Jan 9, 2023

ronso0 Jan 9, 2023

daschuer left a comment

Controller preferences: add searchbars to mapping tables #11165

Controller preferences: add searchbars to mapping tables #11165

Conversation

ronso0 commented Dec 29, 2022

daschuer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daschuer commented Jan 3, 2023

ronso0 commented Jan 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daschuer left a comment

Choose a reason for hiding this comment

ronso0 commented Jan 3, 2023 •

edited

Loading