Search in mixer - code review tweaks

Author: mathesoncalum

src/framework/ui/qml/Muse/Ui/CMakeLists.txt

@@ -26,8 +26,6 @@ qt_add_qml_module(muse_ui_qml
     SOURCES
         abstractnavigation.cpp
         abstractnavigation.h
-        filteredflyoutmodel.h
-        filteredflyoutmodel.cpp
         foreign.cpp
         foreign.h
         graphicstestobject.cpp

src/framework/uicomponents/qml/Muse/UiComponents/CMakeLists.txt

@@ -40,6 +40,8 @@ qt_add_qml_module(muse_uicomponents_qml
         dropdownview.h
         filepickermodel.cpp
         filepickermodel.h
+        filteredflyoutmodel.cpp
+        filteredflyoutmodel.h
         filtervalue.cpp
         filtervalue.h
         iconview.cpp

…k/ui/qml/Muse/Ui/filteredflyoutmodel.cpp …use/UiComponents/filteredflyoutmodel.cppsrc/framework/ui/qml/Muse/Ui/filteredflyoutmodel.cpp renamed to src/framework/uicomponents/qml/Muse/UiComponents/filteredflyoutmodel.cpp src/framework/ui/qml/Muse/Ui/filteredflyoutmodel.cpp renamed to src/framework/uicomponents/qml/Muse/UiComponents/filteredflyoutmodel.cpp

@@ -22,7 +22,7 @@
 
 #include "filteredflyoutmodel.h"
 
-using namespace muse::ui;
+using namespace muse::uicomponents;
 
 // Recursively traverse a flyout tree, collect all "leaves" (items without a sub item)...
 static void flattenTreeModel(const QVariant& treeModel, const QString& categoryTitle, QVariantList& result, QVariant& alwaysAppend)
@@ -43,7 +43,7 @@ static void flattenTreeModel(const QVariant& treeModel, const QString& categoryT
             // Found parent - if it's a "filter category" all child leaves under this item
             // will prepend the title of this item to their titles...
             const bool isFilterCategory = menuItem.value("isFilterCategory").toBool();
-            const QString newCategoryTitle = isFilterCategory ? title : categoryTitle;
+            const QString& newCategoryTitle = isFilterCategory ? title : categoryTitle;
             flattenTreeModel(subItems, newCategoryTitle, result, alwaysAppend); // Recursive call...
             continue;
         }
@@ -58,7 +58,7 @@ static void flattenTreeModel(const QVariant& treeModel, const QString& categoryT
             continue;
         }
 
-        QString prefix = categoryTitle.isEmpty() ? muse::qtrc("uicomponents", "Unknown") : categoryTitle;
+        const QString prefix = categoryTitle.isEmpty() ? muse::qtrc("global", "Unknown") : categoryTitle;
         menuItem.insert("title", prefix + " - " + title);
 
         result << menuItem;

…ork/ui/qml/Muse/Ui/filteredflyoutmodel.h …/Muse/UiComponents/filteredflyoutmodel.hsrc/framework/ui/qml/Muse/Ui/filteredflyoutmodel.h renamed to src/framework/uicomponents/qml/Muse/UiComponents/filteredflyoutmodel.h src/framework/ui/qml/Muse/Ui/filteredflyoutmodel.h renamed to src/framework/uicomponents/qml/Muse/UiComponents/filteredflyoutmodel.h

@@ -25,7 +25,7 @@
 #include <QObject>
 #include <qqmlintegration.h>
 
-namespace muse::ui {
+namespace muse::uicomponents {
 class FilteredFlyoutModel : public QObject
 {
     Q_OBJECT

src/framework/uicomponents/qml/Muse/UiComponents/internal/StyledMenu.qml

@@ -49,7 +49,7 @@ MenuView {
 
     function requestFocus() {
         if (root.isSearchable) {
-            searchField.navigation.requestActive()
+            searchLoader.requestFocus()
             return
         }
         var focused = prv.focusOnSelected()
@@ -122,14 +122,12 @@ MenuView {
 
     desiredHeight: {
         const anchorItemHeight = Boolean(root.anchorItem) ? root.anchorItem.height - padding * 2 : Screen.height
-        const searchHeight = root.isSearchable ? searchColumn.height : 0
+        const searchHeight = root.isSearchable ? searchLoader.height : 0
         return Math.min(listView.actualHeight + searchHeight, anchorItemHeight)
     }
 
     desiredWidth: root.menuMetrics?.itemWidth ?? 0
 
-    isSearching: searchField.searchText !== ""
-
     onAboutToClose: function(closeEvent) {
         closeSubMenu()
     }
@@ -146,22 +144,6 @@ MenuView {
 
     readonly property bool isPlacedAbove: root.popupPosition === PopupPosition.Top
 
-    property bool searchNeedsRefocus: false
-    onIsSubMenuOpenChanged: {
-        // The following prevents a crash when typing while a submenu is open
-        if (root.isSubMenuOpen) {
-            if (searchField.inputField.focus) {
-                searchField.inputField.focus = false
-                root.searchNeedsRefocus = true
-            }
-            return
-        }
-        if (root.searchNeedsRefocus) {
-            searchField.inputField.focus = true
-            root.searchNeedsRefocus = false
-        }
-    }
-
     contentItem: PopupContent {
         id: content
 
@@ -275,53 +257,87 @@ MenuView {
             })
         }
 
-        Column {
-            id: searchColumn
+        Loader {
+            id: searchLoader
 
-            visible: root.isSearchable
+            active: root.isSearchable
 
             width: parent.width
+            height: item ? item.implicitHeight : 0
 
             // If the window is placed above then the search goes underneath the list
-            y: root.isPlacedAbove ? parent.height - searchColumn.height : 0
+            y: root.isPlacedAbove ? parent.height - searchLoader.height : 0
 
-            bottomPadding: root.isPlacedAbove ? root.viewMargins : 0
-            topPadding: root.isPlacedAbove ? 0 : root.viewMargins
-            spacing: root.viewMargins
+            signal requestFocus()
 
-            SeparatorLine {
-                visible: root.isPlacedAbove
+            sourceComponent: Column {
                 width: parent.width
-            }
 
-            SearchField {
-                id: searchField
+                bottomPadding: root.isPlacedAbove ? root.viewMargins : 0
+                topPadding: root.isPlacedAbove ? 0 : root.viewMargins
+                spacing: root.viewMargins
 
-                navigation.panel: content.navigationPanel
-                navigation.row: 0
+                SeparatorLine {
+                    visible: root.isPlacedAbove
+                    width: parent.width
+                }
 
-                inputField.activeFocusOnPress: true
+                SearchField {
+                    id: searchField
 
-                anchors {
-                    left: parent.left
-                    right: parent.right
+                    navigation.panel: content.navigationPanel
+                    navigation.row: 0
 
-                    leftMargin: root.viewMargins
-                    rightMargin: root.viewMargins
-                }
+                    inputField.activeFocusOnPress: true
 
-                onSearchTextChanged: {
-                    if (root.isSubMenuOpen) {
-                        // This is a failsafe - see onIsSubmenuOpenChanged...
-                        return
+                    anchors {
+                        left: parent.left
+                        right: parent.right
+
+                        leftMargin: root.viewMargins
+                        rightMargin: root.viewMargins
+                    }
+
+                    onSearchTextChanged: {
+                        if (root.isSubMenuOpen) {
+                            // This is a failsafe - see onIsSubmenuOpenChanged...
+                            return
+                        }
+                        root.isSearching = searchField.searchText !== ""
+                        filteredModel.setFilterText(searchField.searchText)
+                    }
+
+                    property bool searchNeedsRefocus: false
+
+                    Connections {
+                        target: root
+
+                        function onIsSubMenuOpenChanged() {
+                            // The following prevents a crash when typing while a submenu is open
+                            if (root.isSubMenuOpen) {
+                                if (searchField.inputField.focus) {
+                                    searchField.inputField.focus = false
+                                    searchField.searchNeedsRefocus = true
+                                }
+                                return
+                            }
+                            if (searchField.searchNeedsRefocus) {
+                                searchField.inputField.focus = true
+                                searchField.searchNeedsRefocus = false
+                            }
+                        }
+                    }
+
+                    Connections {
+                        target: searchLoader
+                        function onRequestFocus() { searchField.navigation.requestActive() }
                     }
-                    filteredModel.setFilterText(searchField.searchText)
                 }
-            }
 
-            SeparatorLine {
-                visible: !root.isPlacedAbove
-                width: parent.width
+                SeparatorLine {
+                    visible: !root.isPlacedAbove
+                    width: parent.width
+                }
             }
         }
 
@@ -377,14 +393,14 @@ MenuView {
                     if (!root.isSearchable) {
                         return parent.top
                     }
-                    return root.isPlacedAbove ? parent.top : searchColumn.bottom
+                    return root.isPlacedAbove ? parent.top : searchLoader.bottom
                 }
 
                 bottom: {
                     if (!root.isSearchable) {
                         return parent.bottom
                     }
-                    return root.isPlacedAbove ? searchColumn.top : parent.bottom
+                    return root.isPlacedAbove ? searchLoader.top : parent.bottom
                 }
 
                 topMargin: root.viewMargins

src/framework/uicomponents/qml/Muse/UiComponents/menuview.cpp

@@ -29,6 +29,9 @@ using namespace muse::uicomponents;
 
 static const QString MENU_VIEW_CONTENT_OBJECT_NAME("_MenuViewContent");
 
+// Padding so that our menus don't "collide" with the top/bottom of our screen...
+static const int TOP_BOTTOM_EDGE_PADDING = 16;
+
 MenuView::MenuView(QQuickItem* parent)
     : PopupView(parent)
 {
@@ -115,7 +118,7 @@ void MenuView::updateGeometry()
         m_globalPos = parentTopLeft;
     }
 
-    const QRectF paddedAnchorRect = anchorGeometry().adjusted(0, 16, 0, -16);
+    const QRectF paddedAnchorRect = anchorGeometry().adjusted(0, TOP_BOTTOM_EDGE_PADDING, 0, -TOP_BOTTOM_EDGE_PADDING);
     QRectF viewRect = viewGeometry();
 
     //! NOTE: should be after resolving anchor geometry

src/framework/uicomponents/qml/Muse/UiComponents/popupview.h

@@ -93,7 +93,7 @@ class PopupView : public WindowView
 
     QQuickItem* anchorItem() const;
     void setAnchorItem(QQuickItem* anchorItem);
-    Q_INVOKABLE QRectF anchorGeometry() const;
+    QRectF anchorGeometry() const;
 
     PlacementPolicies placementPolicies() const;
     void setPlacementPolicies(PlacementPolicies placementPolicies);

src/playback/qml/MuseScore/Playback/abstractaudioresourceitem.cpp

@@ -60,8 +60,8 @@ bool AbstractAudioResourceItem::isActive() const
 QVariantMap AbstractAudioResourceItem::buildMenuItem(const QString& itemId,
                                                      const QString& title,
                                                      const bool checked,
-                                                     const bool includeInFilteredLists,
                                                      const QVariantList& subItems,
+                                                     const bool includeInFilteredLists,
                                                      const bool isFilterCategory) const
 {
     QVariantMap result;

src/playback/qml/MuseScore/Playback/abstractaudioresourceitem.h

@@ -73,8 +73,9 @@ class AbstractAudioResourceItem : public QObject, public muse::async::Asyncable,
     void availableResourceListResolved(const QVariantList& resources);
 
 protected:
-    QVariantMap buildMenuItem(const QString& itemId, const QString& title, const bool checked, const bool includeInFilteredLists = true,
-                              const QVariantList& subItems = QVariantList(), const bool isFilterCategory = false) const;
+    QVariantMap buildMenuItem(const QString& itemId, const QString& title, const bool checked,
+                              const QVariantList& subItems = QVariantList(), const bool includeInFilteredLists = true,
+                              const bool isFilterCategory = false) const;
 
     QVariantMap buildSeparator() const;