Make QML code more declarative by using properties

By using properties and property bindings the QML code gets more declarative rather than imperative, which is considered better.

This patch:
- Introduces a currentUserId property in UserModel that replaces the equivalent Q_INVOKABLE call
- Introduces an avatar property in User that contains the avatar's image provider url without any fallback
- Introduces new image provider urls for fallback images
- Moves the fallback image selection to QML since we want different fallbacks according to where it is used
- Wires up the necessary signals to propagate a changing avatar

Signed-off-by: Nicolas Fella <nicolas.fella@gmx.de>

4 files changed, 53 insertions, 49 deletions

diff --git a/src/gui/tray/UserLine.qml b/src/gui/tray/UserLine.qml
index d4e8e2edd..90cf8acf1 100644
--- a/src/gui/tray/UserLine.qml
+++ b/src/gui/tray/UserLine.qml
@@ -67,7 +67,7 @@ MenuItem {
                         Layout.leftMargin: 4

                         verticalAlignment: Qt.AlignCenter

                         cache: false

-                        source: ("image://avatars/" + id)

+                        source: model.avatar != "" ? model.avatar : "image://avatars/fallbackBlack"

                         Layout.preferredHeight: (userLineLayout.height -16)

                         Layout.preferredWidth: (userLineLayout.height -16)

                         Rectangle {

diff --git a/src/gui/tray/UserModel.cpp b/src/gui/tray/UserModel.cpp
index 6c6b5f49c..1a76a50c6 100644
--- a/src/gui/tray/UserModel.cpp
+++ b/src/gui/tray/UserModel.cpp
@@ -48,6 +48,8 @@ User::User(AccountStatePtr &account, const bool &isCurrent, QObject *parent)
     connect(FolderMan::instance(), &FolderMan::folderListChanged, this, &User::hasLocalFolderChanged);
 
     connect(this, &User::guiLog, Logger::instance(), &Logger::guiLog);
+
+    connect(_account->account().data(), &Account::accountChangedAvatar, this, &User::avatarChanged);
 }
 
 void User::slotBuildNotificationDisplay(const ActivityList &list)
@@ -463,21 +465,18 @@ QString User::server(bool shortened) const
     return serverUrl;
 }
 
-QImage User::avatar(bool whiteBg) const
+QImage User::avatar() const
 {
-    QImage img = AvatarJob::makeCircularAvatar(_account->account()->avatar());
-    if (img.isNull()) {
-        QImage image(128, 128, QImage::Format_ARGB32);
-        image.fill(Qt::GlobalColor::transparent);
-        QPainter painter(&image);
-
-        QSvgRenderer renderer(QString(whiteBg ? ":/client/theme/black/user.svg" : ":/client/theme/white/user.svg"));
-        renderer.render(&painter);
+    return AvatarJob::makeCircularAvatar(_account->account()->avatar());
+}
 
-        return image;
-    } else {
-        return img;
+QString User::avatarUrl() const
+{
+    if (avatar().isNull()) {
+        return QString();
     }
+
+    return QStringLiteral("image://avatars/") + _account->account()->id();
 }
 
 bool User::hasLocalFolder() const
@@ -575,27 +574,12 @@ Q_INVOKABLE bool UserModel::isUserConnected(const int &id)
     return _users[id]->isConnected();
 }
 
-Q_INVOKABLE QImage UserModel::currentUserAvatar()
-{
-    if (!_users.isEmpty()) {
-        return _users[_currentUserId]->avatar();
-    } else {
-        QImage image(128, 128, QImage::Format_ARGB32);
-        image.fill(Qt::GlobalColor::transparent);
-        QPainter painter(&image);
-        QSvgRenderer renderer(QString(":/client/theme/white/user.svg"));
-        renderer.render(&painter);
-
-        return image;
-    }
-}
-
 QImage UserModel::avatarById(const int &id)
 {
     if (_users.isEmpty())
         return {};
 
-    return _users[id]->avatar(true);
+    return _users[id]->avatar();
 }
 
 Q_INVOKABLE QString UserModel::currentUserServer()
@@ -617,11 +601,20 @@ void UserModel::addUser(AccountStatePtr &user, const bool &isCurrent)
     }
 
     if (!containsUser) {
-        beginInsertRows(QModelIndex(), rowCount(), rowCount());
-        _users << new User(user, isCurrent);
+        int row = rowCount();
+        beginInsertRows(QModelIndex(), row, row);
+
+        User *u = new User(user, isCurrent);
+
+        connect(u, &User::avatarChanged, this, [this, row] {
+           emit dataChanged(index(row, 0), index(row, 0), {UserModel::AvatarRole});
+        });
+
+        _users << u;
         if (isCurrent) {
             _currentUserId = _users.indexOf(_users.last());
         }
+
         endInsertRows();
         ConfigFile cfg;
         _users.last()->setNotificationRefreshInterval(cfg.notificationRefreshInterval());
@@ -748,7 +741,7 @@ QVariant UserModel::data(const QModelIndex &index, int role) const
     } else if (role == ServerRole) {
         return _users[index.row()]->server();
     } else if (role == AvatarRole) {
-        return _users[index.row()]->avatar();
+        return _users[index.row()]->avatarUrl();
     } else if (role == IsCurrentUserRole) {
         return _users[index.row()]->isCurrentUser();
     } else if (role == IsConnectedRole) {
@@ -829,12 +822,25 @@ QImage ImageProvider::requestImage(const QString &id, QSize *size, const QSize &
     Q_UNUSED(size)
     Q_UNUSED(requestedSize)
 
-    if (id == "currentUser") {
-        return UserModel::instance()->currentUserAvatar();
-    } else {
-        int uid = id.toInt();
-        return UserModel::instance()->avatarById(uid);
+    const auto makeIcon = [](const QString &path) {
+        QImage image(128, 128, QImage::Format_ARGB32);
+        image.fill(Qt::GlobalColor::transparent);
+        QPainter painter(&image);
+        QSvgRenderer renderer(path);
+        renderer.render(&painter);
+        return image;
+    };
+
+    if (id == QLatin1String("fallbackWhite")) {
+        return makeIcon(QStringLiteral(":/client/theme/white/user.svg"));
+    }
+
+    if (id == QLatin1String("fallbackBlack")) {
+        return makeIcon(QStringLiteral(":/client/theme/black/user.svg"));
     }
+
+    const int uid = id.toInt();
+    return UserModel::instance()->avatarById(uid);
 }
 
 /*-------------------------------------------------------------------------------------*/
diff --git a/src/gui/tray/UserModel.h b/src/gui/tray/UserModel.h
index fd48f1110..09b42b175 100644
--- a/src/gui/tray/UserModel.h
+++ b/src/gui/tray/UserModel.h
@@ -21,6 +21,7 @@ class User : public QObject
     Q_PROPERTY(QString server READ server CONSTANT)
     Q_PROPERTY(bool hasLocalFolder READ hasLocalFolder NOTIFY hasLocalFolderChanged)
     Q_PROPERTY(bool serverHasTalk READ serverHasTalk NOTIFY serverHasTalkChanged)
+    Q_PROPERTY(QString avatar READ avatarUrl NOTIFY avatarChanged)
 public:
     User(AccountStatePtr &account, const bool &isCurrent = false, QObject* parent = nullptr);
 
@@ -39,17 +40,18 @@ public:
     AccountApp *talkApp() const;
     bool hasActivities() const;
     AccountAppList appList() const;
-    QImage avatar(bool whiteBg = false) const;
-    QString id() const;
+    QImage avatar() const;
     void login() const;
     void logout() const;
     void removeAccount() const;
+    QString avatarUrl() const;
 
 signals:
     void guiLog(const QString &, const QString &);
     void nameChanged();
     void hasLocalFolderChanged();
     void serverHasTalkChanged();
+    void avatarChanged();
 
 public slots:
     void slotItemCompleted(const QString &folder, const SyncFileItemPtr &item);
@@ -89,6 +91,7 @@ class UserModel : public QAbstractListModel
 {
     Q_OBJECT
     Q_PROPERTY(User* currentUser READ currentUser NOTIFY newUserSelected)
+    Q_PROPERTY(int currentUserId READ currentUserId NOTIFY newUserSelected)
 public:
     static UserModel *instance();
     virtual ~UserModel() = default;
@@ -108,12 +111,11 @@ public:
     Q_INVOKABLE void openCurrentAccountLocalFolder();
     Q_INVOKABLE void openCurrentAccountTalk();
     Q_INVOKABLE void openCurrentAccountServer();
-    Q_INVOKABLE QImage currentUserAvatar();
     Q_INVOKABLE int numUsers();
     Q_INVOKABLE QString currentUserServer();
     Q_INVOKABLE bool currentUserHasActivities();
     Q_INVOKABLE bool currentUserHasLocalFolder();
-    Q_INVOKABLE int currentUserId() const;
+    int currentUserId() const;
     Q_INVOKABLE bool isUserConnected(const int &id);
     Q_INVOKABLE void switchCurrentUser(const int &id);
     Q_INVOKABLE void login(const int &id);
diff --git a/src/gui/tray/Window.qml b/src/gui/tray/Window.qml
index f5d23b4a6..a73b3bfad 100644
--- a/src/gui/tray/Window.qml
+++ b/src/gui/tray/Window.qml
@@ -33,10 +33,8 @@ Window {
     }

 

     onVisibleChanged: {

-        currentAccountAvatar.source = ""

-        currentAccountAvatar.source = "image://avatars/currentUser"

         currentAccountStateIndicator.source = ""

-        currentAccountStateIndicator.source = UserModel.isUserConnected(UserModel.currentUserId()) ? "qrc:///client/theme/colored/state-ok.svg" : "qrc:///client/theme/colored/state-offline.svg"

+        currentAccountStateIndicator.source = UserModel.isUserConnected(UserModel.currentUserId) ? "qrc:///client/theme/colored/state-ok.svg" : "qrc:///client/theme/colored/state-offline.svg"

 

         // HACK: reload account Instantiator immediately by restting it - could be done better I guess

         // see also id:accountMenu below

@@ -47,10 +45,8 @@ Window {
     Connections {

         target: UserModel

         onRefreshCurrentUserGui: {

-            currentAccountAvatar.source = ""

-            currentAccountAvatar.source = "image://avatars/currentUser"

             currentAccountStateIndicator.source = ""

-            currentAccountStateIndicator.source = UserModel.isUserConnected(UserModel.currentUserId()) ? "qrc:///client/theme/colored/state-ok.svg" : "qrc:///client/theme/colored/state-offline.svg"

+            currentAccountStateIndicator.source = UserModel.isUserConnected(UserModel.currentUserId) ? "qrc:///client/theme/colored/state-ok.svg" : "qrc:///client/theme/colored/state-offline.svg"

         }

         onNewUserSelected: {

             accountMenu.close();

@@ -336,7 +332,7 @@ Window {
                             Layout.leftMargin: 8

                             verticalAlignment: Qt.AlignCenter

                             cache: false

-                            source: "image://avatars/currentUser"

+                            source: UserModel.currentUser.avatar != "" ? UserModel.currentUser.avatar : "image://avatars/fallbackWhite"

                             Layout.preferredHeight: Style.accountAvatarSize

                             Layout.preferredWidth: Style.accountAvatarSize

 

@@ -355,7 +351,7 @@ Window {
 

                             Image {

                                 id: currentAccountStateIndicator

-                                source: UserModel.isUserConnected(UserModel.currentUserId()) ? "qrc:///client/theme/colored/state-ok.svg" : "qrc:///client/theme/colored/state-offline.svg"

+                                source: UserModel.isUserConnected(UserModel.currentUserId) ? "qrc:///client/theme/colored/state-ok.svg" : "qrc:///client/theme/colored/state-offline.svg"

                                 cache: false

                                 x: currentAccountStateIndicatorBackground.x + 1

                                 y: currentAccountStateIndicatorBackground.y + 1