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
2 changes: 1 addition & 1 deletion src/3rdparty/qtmacgoodies
67 changes: 61 additions & 6 deletions src/gui/settingsdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
#include <QPixmap>
#include <QImage>
#include <QWidgetAction>
#include <QPainter>
#include <QPainterPath>

namespace {
const char TOOLBAR_CSS[] =
Expand All @@ -54,6 +56,23 @@ namespace {

namespace OCC {

static QIcon circleMask( const QImage& avatar )
{
int dim = avatar.width();

QPixmap fixedImage(dim, dim);
fixedImage.fill(Qt::transparent);

QPainter imgPainter(&fixedImage);
QPainterPath clip;
clip.addEllipse(0, 0, dim, dim);
imgPainter.setClipPath(clip);
imgPainter.drawImage(0, 0, avatar);
imgPainter.end();

return QIcon(fixedImage);
}

//
// Whenever you change something here check both settingsdialog.cpp and settingsdialogmac.cpp !
//
Expand Down Expand Up @@ -196,8 +215,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;
QImage avatar = s->account()->avatar();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this will never be called since accountAdded is called before the ConnectionValidator can run?

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

if (!brandingSingleAccount) {
accountAction->setToolTip(s->account()->displayName());
accountAction->setIconText(s->shortDisplayNameForSettings(height * buttonSizeRatio));
Expand All @@ -207,14 +235,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 ) {
QImage pix = account->avatar();
if( !pix.isNull() ) {
action->setIcon( circleMask(pix) );
}
}
}
}

void SettingsDialog::accountRemoved(AccountState *s)
{
for (auto it = _actionGroupWidgets.begin(); it != _actionGroupWidgets.end(); ++it) {
Expand All @@ -236,6 +280,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 +353,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
38 changes: 38 additions & 0 deletions src/gui/settingsdialogmac.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,30 @@
#include <QPushButton>
#include <QDebug>
#include <QSettings>
#include <QPainter>
#include <QPainterPath>

namespace OCC {

// Duplicate in settingsdialog.cpp
static QIcon circleMask( const QImage& avatar )
{
int dim = avatar.width();

QPixmap fixedImage(dim, dim);
fixedImage.fill(Qt::transparent);

QPainter imgPainter(&fixedImage);
QPainterPath clip;
clip.addEllipse(0, 0, dim, dim);
imgPainter.setClipPath(clip);
imgPainter.drawImage(0, 0, avatar);
imgPainter.end();

return QIcon(fixedImage);
}


//
// Whenever you change something here check both settingsdialog.cpp and settingsdialogmac.cpp !
//
Expand Down Expand Up @@ -125,6 +146,8 @@ void SettingsDialogMac::accountAdded(AccountState *s)
connect( accountSettings, &AccountSettings::folderChanged, _gui, &ownCloudGui::slotFoldersChanged);
connect( accountSettings, &AccountSettings::openFolderAlias, _gui, &ownCloudGui::slotFolderOpenAction);

connect(s->account().data(), SIGNAL(accountChangedAvatar()), this, SLOT(slotAccountAvatarChanged()));

slotRefreshActivity(s);
}

Expand All @@ -147,4 +170,19 @@ void SettingsDialogMac::slotRefreshActivity( AccountState* accountState )
}
}

void SettingsDialogMac::slotAccountAvatarChanged()
{
Account *account = static_cast<Account*>(sender());
auto list = findChildren<AccountSettings*>(QString());
foreach(auto p, list) {
if (p->accountsState()->account() == account) {
int idx = indexForPanel(p);
QImage pix = account->avatar();
if (!pix.isNull()) {
setPreferencesPanelIcon(idx, circleMask(pix));
}
}
}
}

}
1 change: 1 addition & 0 deletions src/gui/settingsdialogmac.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public slots:
private slots:
void accountAdded(AccountState *);
void accountRemoved(AccountState *);
void slotAccountAvatarChanged();
private:
void closeEvent(QCloseEvent *event);

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;
}

QImage Account::avatar() const
{
return _avatarImg;
}
void Account::setAvatar(const QImage &img)
{
_avatarImg = img;
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);

QImage avatar() const;
void setAvatar(const QImage& img);

/// 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;
QImage _avatarImg;
QMap<QString, QVariant> _settingsMap;
QUrl _url;
QList<QSslCertificate> _approvedCerts;
Expand Down
12 changes: 12 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,18 @@ 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);
job->setTimeout(20*1000);
Copy link
Contributor

Choose a reason for hiding this comment

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

(I think it should use the same timeout as all the other jobs in this file, for consistency)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ogoffart wouldn''t it make more sense to set it in the constructor of AvatarJob anyway?

QObject::connect(job, SIGNAL(avatarPixmap(QImage)), this, SLOT(slotAvatarImage(QImage)));

job->start();
}
}

void ConnectionValidator::slotAvatarImage(const QImage& img)
{
_account->setAvatar(img);
reportResult(Connected);
}

Expand Down
6 changes: 5 additions & 1 deletion src/libsync/connectionvalidator.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ namespace OCC {
+-> fetchUser
PropfindJob
|
+-> slotUserFetched --> X
+-> slotUserFetched
AvatarJob
|
+-> slotAvatarImage --> reportResult()

\endcode
*/
Expand Down Expand Up @@ -119,6 +122,7 @@ protected slots:

void slotCapabilitiesRecieved(const QVariantMap&);
void slotUserFetched(const QVariantMap &);
void slotAvatarImage(const QImage &img);

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();

QImage avImage;

if (http_result_code == 200) {

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

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

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

ProppatchJob::ProppatchJob(AccountPtr account, const QString &path, QObject *parent)
: AbstractNetworkJob(account, path, parent)
{
Expand Down
Loading