Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display Avatar pictures in the Settingsdialog #5482

Closed
wants to merge 8 commits into from
48 changes: 42 additions & 6 deletions src/gui/settingsdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,17 @@ void SettingsDialog::accountAdded(AccountState *s)

bool brandingSingleAccount = !Theme::instance()->multiAccount();

auto accountAction = createColorAwareAction(QLatin1String(":/client/resources/account.png"),
brandingSingleAccount ? tr("Account") : s->account()->displayName());
QAction *accountAction;
QPixmap avatar = s->account()->avatar();
const QString actionText = brandingSingleAccount ? tr("Account") : s->account()->displayName();
if(avatar.isNull()) {
accountAction = createColorAwareAction(QLatin1String(":/client/resources/account.png"),
actionText);
} else {
QIcon icon(avatar);
accountAction = createActionWithIcon(icon, actionText);
}

if (!brandingSingleAccount) {
accountAction->setToolTip(s->account()->displayName());
accountAction->setIconText(s->shortDisplayNameForSettings(height * buttonSizeRatio));
Expand All @@ -207,14 +216,30 @@ void SettingsDialog::accountAdded(AccountState *s)
_ui->stack->insertWidget(0 , accountSettings);
_actionGroup->addAction(accountAction);
_actionGroupWidgets.insert(accountAction, accountSettings);
_actionForAccount.insert(s->account().data(), accountAction);

connect( accountSettings, SIGNAL(folderChanged()), _gui, SLOT(slotFoldersChanged()));
connect( accountSettings, SIGNAL(openFolderAlias(const QString&)),
_gui, SLOT(slotFolderOpenAction(QString)));
connect(s->account().data(), SIGNAL(accountChangedAvatar()), SLOT(slotAccountAvatarChanged()));

slotRefreshActivity(s);
}

void SettingsDialog::slotAccountAvatarChanged()
{
Account *account = static_cast<Account*>(sender());
if( account && _actionForAccount.contains(account)) {
QAction *action = _actionForAccount[account];
if( action ) {
QPixmap pix = account->avatar();
if( !pix.isNull() ) {
action->setIcon( QIcon(pix) );
}
}
}
}

void SettingsDialog::accountRemoved(AccountState *s)
{
for (auto it = _actionGroupWidgets.begin(); it != _actionGroupWidgets.end(); ++it) {
Expand All @@ -236,6 +261,9 @@ void SettingsDialog::accountRemoved(AccountState *s)
}
}

if( _actionForAccount.contains(s->account().data()) ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small detail: No need of the if here. remove does nothing if it is not there.

_actionForAccount.remove(s->account().data());
}
_activitySettings->slotRemoveAccount(s);

// Hide when the last account is deleted. We want to enter the same
Expand Down Expand Up @@ -306,14 +334,22 @@ class ToolButtonAction : public QWidgetAction
}
};

QAction *SettingsDialog::createActionWithIcon(const QIcon& icon, const QString& text, const QString& iconPath)
{
QAction *action = new ToolButtonAction(icon, text, this);
action->setCheckable(true);
if(!iconPath.isEmpty()) {
action->setProperty("iconPath", iconPath);
}
return action;

}

QAction *SettingsDialog::createColorAwareAction(const QString &iconPath, const QString &text)
{
// all buttons must have the same size in order to keep a good layout
QIcon coloredIcon = createColorAwareIcon(iconPath);
QAction *action = new ToolButtonAction(coloredIcon, text, this);
action->setCheckable(true);
action->setProperty("iconPath", iconPath);
return action;
return createActionWithIcon(coloredIcon, text, iconPath);
}

void SettingsDialog::slotRefreshActivity( AccountState* accountState )
Expand Down
7 changes: 7 additions & 0 deletions src/gui/settingsdialog.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public slots:
void showActivityPage();
void slotSwitchPage(QAction *action);
void slotRefreshActivity(AccountState *accountState );
void slotAccountAvatarChanged();

protected:
void reject() Q_DECL_OVERRIDE;
Expand All @@ -73,12 +74,18 @@ private slots:

QIcon createColorAwareIcon(const QString &name);
QAction *createColorAwareAction(const QString &iconName, const QString &fileName);
QAction *createActionWithIcon(const QIcon& icon, const QString& text, const QString& iconPath = QString());

Ui::SettingsDialog * const _ui;

QActionGroup* _actionGroup;
// Maps the actions from the action group to the corresponding widgets
QHash<QAction*, QWidget*> _actionGroupWidgets;

// Maps the action in the dialog to their according account. Needed in
// case the account avatar changes
QHash<Account*, QAction*> _actionForAccount;

QToolBar* _toolBar;

ActivitySettings *_activitySettings;
Expand Down
10 changes: 10 additions & 0 deletions src/libsync/account.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,16 @@ void Account::setDavUser(const QString &newDavUser)
_davUser = newDavUser;
}

QPixmap Account::avatar() const
{
return _avatarPixmap;
}
void Account::setAvatar(const QPixmap& pixmap)
{
_avatarPixmap = pixmap;
emit accountChangedAvatar();
}

QString Account::displayName() const
{
QString dn = QString("%1@%2").arg(_credentials->user(), _url.host());
Expand Down
8 changes: 8 additions & 0 deletions src/libsync/account.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
#include <QSslCipher>
#include <QSslError>
#include <QSharedPointer>
#include <QPixmap>

#include "utility.h"
#include <memory>
#include "capabilities.h"
Expand Down Expand Up @@ -78,6 +80,9 @@ class OWNCLOUDSYNC_EXPORT Account : public QObject {
QString davUser() const;
void setDavUser(const QString &newDavUser);

QPixmap avatar() const;
void setAvatar(const QPixmap& pixmap);

/// The name of the account as shown in the toolbar
QString displayName() const;

Expand Down Expand Up @@ -197,6 +202,8 @@ class OWNCLOUDSYNC_EXPORT Account : public QObject {

void serverVersionChanged(Account* account, const QString& newVersion, const QString& oldVersion);

void accountChangedAvatar();

protected Q_SLOTS:
void slotHandleSslErrors(QNetworkReply*,QList<QSslError>);
void slotCredentialsFetched();
Expand All @@ -209,6 +216,7 @@ protected Q_SLOTS:
QWeakPointer<Account> _sharedThis;
QString _id;
QString _davUser;
QPixmap _avatarPixmap;
QMap<QString, QVariant> _settingsMap;
QUrl _url;
QList<QSslCertificate> _approvedCerts;
Expand Down
11 changes: 11 additions & 0 deletions src/libsync/connectionvalidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <QtCore>
#include <QNetworkReply>
#include <QNetworkProxyFactory>
#include <QPixmap>

#include "connectionvalidator.h"
#include "account.h"
Expand Down Expand Up @@ -252,7 +253,17 @@ void ConnectionValidator::slotUserFetched(const QVariantMap &json)
QString user = json.value("ocs").toMap().value("data").toMap().value("id").toString();
if (!user.isEmpty()) {
_account->setDavUser(user);

AvatarJob *job = new AvatarJob(_account, this);
QObject::connect(job, SIGNAL(avatarPixmap(QPixmap)), this, SLOT(slotAvatarPixmap(QPixmap)));

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing

    job->setTimeout(timeoutToUseMsec);

The default timeout is 5 minutes, and that's way to long for the connection validator which must finish much earlier in case of bad connection.

(And actually, since we increased the amount of jobs, timeoutToUseMsec might need to be lowered too. But I'm wondering if it would be possible to fetch the capabilities, user name, and the avatar in parallel.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess capabilities fetching could run in parallel to fetching the username, and following on that, the avatar, because the avatar needs the username. Maybe the fetch job of the avatar (at least) should be owned by the account, and could become fully async?

job->start();
}
}

void ConnectionValidator::slotAvatarPixmap(const QPixmap& pixmap)
{
_account->setAvatar(pixmap);
reportResult(Connected);
}

Expand Down
1 change: 1 addition & 0 deletions src/libsync/connectionvalidator.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ protected slots:

void slotCapabilitiesRecieved(const QVariantMap&);
void slotUserFetched(const QVariantMap &);
void slotAvatarPixmap(const QPixmap&);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ASCII art chart need to be updated at the beginning of this class.


private:
void reportResult(Status status);
Expand Down
37 changes: 37 additions & 0 deletions src/libsync/networkjobs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <QMutex>
#include <QDebug>
#include <QCoreApplication>
#include <QPixmap>

#include "json.h"

Expand Down Expand Up @@ -589,6 +590,42 @@ bool PropfindJob::finished()

/*********************************************************************************************/

AvatarJob::AvatarJob(AccountPtr account, QObject *parent)
: AbstractNetworkJob(account, QString(), parent)
{
_avatarUrl = Utility::concatUrlPath(account->url(), QString("remote.php/dav/avatars/%1/128.png").arg(account->davUser()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does not work for me with this URL. (owncloud master)
The URL that works for me is index.php/avatar/%1/128

@DeepDiver1975 What's the exact URL of the avatars?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I think the pull request that is implementing the avatar images is not yet merged in core: owncloud/core#26872

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then let's delay this PR to master/2.4? sorry for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, sure.

}

void AvatarJob::start()
{
QNetworkRequest req;
setReply(davRequest("GET", _avatarUrl, req));
setupConnections(reply());
AbstractNetworkJob::start();
}

bool AvatarJob::finished()
{
int http_result_code = reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();

QPixmap avPixmap;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to cause the FolderManTest to fail on your PR, since that needs a QGuiApplication. And you don't seem to need a QPixmap either, since you're creating a new one in circleMask anyway, so storing it as a QImage could be enough to fix that test.


if (http_result_code == 200) {

QByteArray pngData = reply()->readAll();
if( pngData.size() ) {

if( avPixmap.loadFromData(pngData) ) {
qDebug() << "Retrieved Avatar pixmap!";
}
}
}
emit(avatarPixmap(avPixmap));
return true;
}

/*********************************************************************************************/

ProppatchJob::ProppatchJob(AccountPtr account, const QString &path, QObject *parent)
: AbstractNetworkJob(account, path, parent)
{
Expand Down
30 changes: 30 additions & 0 deletions src/libsync/networkjobs.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,36 @@ private slots:
QList<QByteArray> _properties;
};


/**
* @brief The AvatarJob class
*
* Retrieves the account users avatar from the server using a GET request.
*
* If the server does not have the avatar, the result Pixmap is empty.
*
* @ingroup libsync
*/
class OWNCLOUDSYNC_EXPORT AvatarJob : public AbstractNetworkJob {
Q_OBJECT
public:
explicit AvatarJob(AccountPtr account, QObject *parent = 0);
void start() Q_DECL_OVERRIDE;

signals:
/**
* @brief avatarPixmap - returns either a valid pixmap or not.
*/

void avatarPixmap(QPixmap);

private slots:
virtual bool finished() Q_DECL_OVERRIDE;

private:
QUrl _avatarUrl;
};

/**
* @brief Send a Proppatch request
*
Expand Down