-
Notifications
You must be signed in to change notification settings - Fork 665
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
Changes from 3 commits
b49dd02
e05d6bf
d466a05
2a12610
c00e3e8
e95b73d
5e33898
554d1b8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,7 @@ | |
#include <QtCore> | ||
#include <QNetworkReply> | ||
#include <QNetworkProxyFactory> | ||
#include <QPixmap> | ||
|
||
#include "connectionvalidator.h" | ||
#include "account.h" | ||
|
@@ -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))); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing
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.) There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -119,6 +119,7 @@ protected slots: | |
|
||
void slotCapabilitiesRecieved(const QVariantMap&); | ||
void slotUserFetched(const QVariantMap &); | ||
void slotAvatarPixmap(const QPixmap&); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,7 @@ | |
#include <QMutex> | ||
#include <QDebug> | ||
#include <QCoreApplication> | ||
#include <QPixmap> | ||
|
||
#include "json.h" | ||
|
||
|
@@ -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())); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does not work for me with this URL. (owncloud master) @DeepDiver1975 What's the exact URL of the avatars? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
||
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) | ||
{ | ||
|
There was a problem hiding this comment.
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.