Fix review coments + changed which image provider to use for coloured…

… SVG. Signed-off-by: alex-z <[email protected]>
nextcloud · Mar 3, 2023 · 7aa7043 · 7aa7043
1 parent 329bca1
commit 7aa7043
Show file tree

Hide file tree

Showing 8 changed files with 19 additions and 45 deletions.
diff --git a/src/gui/filedetails/ShareeDelegate.qml b/src/gui/filedetails/ShareeDelegate.qml
@@ -27,13 +27,9 @@ ItemDelegate {
 
     text: model.display
 
-    contentItem:
-        RowLayout {
-        Layout.fillWidth: true
+    contentItem: RowLayout {
         height: visible ? implicitHeight : 0
 
-        visible: true
-
         Loader {
               id: shareeIconLoader
 
@@ -44,7 +40,8 @@ ItemDelegate {
               sourceComponent: Image {
                   id: shareeIcon
 
-                  Layout.alignment: Qt.AlignVCenter | Qt.AlignLeft
+                  horizontalAlignment: Qt.AlignLeft
+                  verticalAlignment: Qt.AlignVCenter
 
                   width: height
                   height: shareeLabel.height
@@ -57,8 +54,6 @@ ItemDelegate {
                   source: model.icon
 
                   sourceSize: Qt.size(shareeIcon.height * 1.0, shareeIcon.height * 1.0)
-
-                  visible: true
               }
         }
 

diff --git a/src/gui/filedetails/ShareeSearchField.qml b/src/gui/filedetails/ShareeSearchField.qml
@@ -44,10 +44,6 @@ TextField {
         shareeListView.count > 0 ? suggestionsPopup.open() : suggestionsPopup.close();
     }
 
-    function searchGlobally() {
-        shareeModel.searchGlobally();
-    }
-
     placeholderText: qsTr("Search for users or groups…")
     placeholderTextColor: placeholderColor
     color: Style.ncTextColor
@@ -236,7 +232,7 @@ TextField {
                     function selectItem() {
                         if (model.type === Sharee.LookupServerSearch) {
                             shareeListView.currentIndex = -1
-                            root.searchGlobally()
+                            root.shareeModel.searchGlobally()
                         } else {
                             selectSharee()
                         }

diff --git a/src/gui/filedetails/shareemodel.cpp b/src/gui/filedetails/shareemodel.cpp
@@ -30,7 +30,7 @@ ShareeModel::ShareeModel(QObject *parent)
 {
     _searchRateLimitingTimer.setSingleShot(true);
     _searchRateLimitingTimer.setInterval(500);
-    _searchGloballyPlaceholder.reset(new Sharee({}, tr("Search globally"), Sharee::LookupServerSearch, QStringLiteral(":/client/theme/magnifying-glass.svg")));
+    _searchGloballyPlaceholder.reset(new Sharee({}, tr("Search globally"), Sharee::LookupServerSearch, QStringLiteral("magnifying-glass.svg")));
     _searchGloballyPlaceholder->setIsIconColourful(true);
     connect(&_searchRateLimitingTimer, &QTimer::timeout, this, &ShareeModel::fetch);
     connect(Theme::instance(), &Theme::darkModeChanged, this, &ShareeModel::slotDarkModeChanged);
@@ -271,14 +271,17 @@ void ShareeModel::shareesFetched(const QJsonDocument &reply)
 
 void ShareeModel::insertSearchGloballyItem(const QVector<ShareePtr> &newShareesFetched)
 {
-    const auto isAlreadyInserted = std::find_if(std::begin(_sharees), std::end(_sharees), [](const ShareePtr &sharee) {
+    const auto foundIt = std::find_if(std::begin(_sharees), std::end(_sharees), [](const ShareePtr &sharee) {
         return sharee->type() == Sharee::LookupServerSearch || sharee->type() == Sharee::LookupServerSearchResults;
-    }) != std::end(_sharees);
+    });
 
-    if (!isAlreadyInserted) {
-        _sharees.push_back(_searchGloballyPlaceholder);
+    // remove it if it somehow appeared not at the end, to avoid writing complex proxy models for sorting
+    if (foundIt != std::end(_sharees) && (foundIt + 1) != std::end(_sharees)) {
+        _sharees.erase(foundIt);
     }
 
+    _sharees.push_back(_searchGloballyPlaceholder);
+
     if (lookupMode() == LookupMode::GlobalSearch) {
         const auto displayName = newShareesFetched.isEmpty() ? tr("No results found") : tr("Global search results");
         _searchGloballyPlaceholder->setDisplayName(displayName);
@@ -287,22 +290,6 @@ void ShareeModel::insertSearchGloballyItem(const QVector<ShareePtr> &newShareesF
         _searchGloballyPlaceholder->setDisplayName(tr("Search globally"));
         _searchGloballyPlaceholder->setType(Sharee::LookupServerSearch);
     }
-
-    sortSharees();
-}
-
-void ShareeModel::sortSharees()
-{
-    // sort _sharees such that lookup placeholder is always last
-    std::sort(std::begin(_sharees), std::end(_sharees), [](const auto &left, const auto &right) {
-        const auto isRightElementLookupPlaceholder = right->type() == Sharee::LookupServerSearch
-            || right->type() == Sharee::LookupServerSearchResults;
-
-        const auto isLeftElementLookupPlaceholder = left->type() == Sharee::LookupServerSearch
-            || left->type() == Sharee::LookupServerSearchResults;
-
-        return !isLeftElementLookupPlaceholder && isRightElementLookupPlaceholder;
-    });
 }
 
 ShareePtr ShareeModel::parseSharee(const QJsonObject &data) const

diff --git a/src/gui/filedetails/shareemodel.h b/src/gui/filedetails/shareemodel.h
@@ -89,7 +89,6 @@ public slots:
 private slots:
     void shareesFetched(const QJsonDocument &reply);
     void insertSearchGloballyItem(const QVector<ShareePtr> &newShareesFetched);
-    void sortSharees();
     void filterSharees();
     void slotDarkModeChanged();
 

diff --git a/src/gui/filedetails/sharemodel.cpp b/src/gui/filedetails/sharemodel.cpp
@@ -998,8 +998,6 @@ void ShareModel::createNewUserGroupShareWithPassword(const ShareePtr &sharee, co
         return;
     }
 
-    emit startedCretingNewUserGroupShare();
-
     _manager->createShare(_sharePath,
                           Share::ShareType(sharee->type()),
                           sharee->shareWith(),

diff --git a/src/gui/filedetails/sharemodel.h b/src/gui/filedetails/sharemodel.h
@@ -118,7 +118,6 @@ class ShareModel : public QAbstractListModel
     void passwordSetError(const QString &shareId, const int code, const QString &message);
     void requestPasswordForLinkShare();
     void requestPasswordForEmailSharee(const OCC::ShareePtr &sharee);
-    void startedCretingNewUserGroupShare() const;
 
     void sharesChanged();
 

diff --git a/src/gui/sharee.cpp b/src/gui/sharee.cpp
@@ -105,7 +105,7 @@ bool Sharee::updateIconUrl()
 
     const auto iconUrlColoured = _iconUrlColoured;
     _iconColor = (!_isIconColourful || !Theme::instance()->darkMode()) ? QStringLiteral("black") : QStringLiteral("white");
-    _iconUrlColoured = QStringLiteral("image://tray-image-provider/") + _iconUrl + QStringLiteral("/") + _iconColor;
+    _iconUrlColoured = QStringLiteral("image://svgimage-custom-color/") + _iconUrl + QStringLiteral("/") + _iconColor;
 
     return iconUrlColoured != _iconUrlColoured;
 }

diff --git a/src/gui/sharee.h b/src/gui/sharee.h
@@ -49,12 +49,12 @@ class Sharee
     explicit Sharee() = default;
     explicit Sharee(const QString &shareWith, const QString &displayName, const Type type, const QString &iconUrl = {});
 
-    Q_REQUIRED_RESULT QString format() const;
-    Q_REQUIRED_RESULT QString shareWith() const;
-    Q_REQUIRED_RESULT QString displayName() const;
-    Q_REQUIRED_RESULT QString iconUrl() const;
-    Q_REQUIRED_RESULT QString iconUrlColoured() const;
-    Q_REQUIRED_RESULT Type type() const;
+    [[nodiscard]] QString format() const;
+    [[nodiscard]] QString shareWith() const;
+    [[nodiscard]] QString displayName() const;
+    [[nodiscard]] QString iconUrl() const;
+    [[nodiscard]] QString iconUrlColoured() const;
+    [[nodiscard]] Type type() const;
     bool updateIconUrl();
 
     void setDisplayName(const QString &displayName);