Differential D4720

Show icons of account services
ClosedPublic
Actions

Authored by elvisangelaccio on Feb 22 2017, 5:10 PM.

Details

Reviewers
mck182
Group Reviewers
VDG
Commits
R155:4d5d7a97d23a: Show icons of account services
Summary

Account services can provide icons, so we can show them in the checkbox
of the service.

The whole widget should probably become a listview with custom delegate at
some point, but for now there is no reason to not show the icons.

Diff Detail

Repository
R155 KAccounts Integration
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mck182 edited edge metadata.Feb 22 2017, 5:12 PM

What happens when the service provides no icon
name or an icon that cannot be found? Can you
please test that?

elvisangelaccio added a subscriber: andreaska.EditedFeb 22 2017, 5:18 PM
In D4720#88634, @mck182 wrote:

What happens when the service provides no icon
name or an icon that cannot be found? Can you
please test that?

Doesn't look pretty, but on the other hand there is enough time to fix the missing icons (before the next kaccounts-integration release).
I actually cheated in my previous screenshots because Purpose's youtube service has google-youtube, while breeze has only im-youtube.
@andreaska What do you say about this? Does breeze miss a google-youtube symlink or Purpose should use im-youtube instead?

mck182 added a comment.Feb 22 2017, 5:21 PM

Ok, can you perhaps put some other/generic/nice icon
if the iconName() is empty? Just to keep the list consistent?
Or maybe put the icon elsewhere so that it won't affect the
alignment? But I ain't no designer..

elvisangelaccio added a comment.Feb 22 2017, 5:28 PM
In D4720#88637, @mck182 wrote:

Ok, can you perhaps put some other/generic/nice icon
if the iconName() is empty? Just to keep the list consistent?
Or maybe put the icon elsewhere so that it won't affect the
alignment? But I ain't no designer..

Good idea, here is how it looks using the provider icon as fallback:

elvisangelaccio added a comment.Feb 22 2017, 5:33 PM

Or maybe we should use the generic internet-services icon, given that also the provider icon could be missing:

elvisangelaccio updated this revision to Diff 11638.Feb 22 2017, 5:43 PM
  • Use internet-services icon as fallback. Better than the provider icon which could also be missing.
mck182 accepted this revision.Feb 23 2017, 3:59 AM

I think this is good. Might look a bit weird when you
have a whole bunch of services with default icons
but oh well.

This revision is now accepted and ready to land.Feb 23 2017, 3:59 AM
Closed by commit R155:4d5d7a97d23a: Show icons of account services (authored by elvisangelaccio). · Explain WhyFeb 23 2017, 2:49 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/accountwidget.cpp (6 lines)

Diff 11691

View Options

src/accountwidget.cpp

Loading...