Rename colorful user gallery avatar
ClosedPublic

Authored by GB_2 on Apr 15 2019, 3:50 AM.

Details

Summary

In D20536 we rename the colorful user avatar to user-man. This patch changes that avatar to the name "Man" to match the new name.

Test Plan

Open the avatar gallery.

Diff Detail

Repository
R128 User Manager
Branch
change-default-user-avatar (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 10886
Build 10904: arc lint + arc unit
GB_2 created this revision.Apr 15 2019, 3:50 AM
Restricted Application added a project: Plasma. · View Herald TranscriptApr 15 2019, 3:50 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
GB_2 requested review of this revision.Apr 15 2019, 3:50 AM
GB_2 added a comment.Apr 15 2019, 4:04 AM

Ok, the problem is that now the default user icon doesn't work with a dark theme. I think we should just provide it in breeze-icons, so it is the default user icon that also gets selected when you click Clear Avatar.

GB_2 updated this revision to Diff 56265.Apr 15 2019, 4:08 AM

Remove monochrome icon

ngraham accepted this revision.Apr 15 2019, 4:35 AM
ngraham added a subscriber: ngraham.

This works and makes sense to me, but I'm not as familiar with the user-manager codbease and would appreciate a Plasma review too, in case this might have difficult-to-see consequences.

This revision is now accepted and ready to land.Apr 15 2019, 4:35 AM
davidedmundson requested changes to this revision.Apr 20 2019, 11:21 PM
davidedmundson added a subscriber: davidedmundson.

Ping

For future, the commit message is important.

The person reviewing doesn't have the same context you have when you're making the change, if you can save the reviewer having to dig around user-manager to understand what we're doing it makes everything easier. You won't have me asking potentially stupid questions :D

So from what I can tell:

  • UserManager by default uses an icon from breeze
QIcon::fromTheme(QStringLiteral("user-identity"))

It completely ignores anything from the gallery we ship with user-manager /o\

  • The "User.png" gallery icon matches the large version of the "user-identity" icon in the breeze theme

The part I don't understand, how does renaming this file change anything?

This revision now requires changes to proceed.Apr 20 2019, 11:21 PM
GB_2 retitled this revision from Change default user avatar to Rename default user gallery avatar.Apr 21 2019, 11:03 AM
GB_2 edited the summary of this revision. (Show Details)
GB_2 retitled this revision from Rename default user gallery avatar to Rename colorful user gallery avatar.
GB_2 added a comment.Apr 21 2019, 11:13 AM

Ping

For future, the commit message is important.

The person reviewing doesn't have the same context you have when you're making the change, if you can save the reviewer having to dig around user-manager to understand what we're doing it makes everything easier. You won't have me asking potentially stupid questions :D

So from what I can tell:

  • UserManager by default uses an icon from breeze `QIcon::fromTheme(QStringLiteral("user-identity"))`

    It completely ignores anything from the gallery we ship with user-manager /o\
  • The "User.png" gallery icon matches the large version of the "user-identity" icon in the breeze theme

    The part I don't understand, how does renaming this file change anything?

I changed the title and description, it should be clearer now.
D20536 renames the colorful user avatar to user-man and adds a monochrome icon called user-identity, so that the monochrome icon is the new default user icon. This patch just renames the "User" avatar to "Man" to match the name change in D20536.

davidedmundson accepted this revision.Apr 21 2019, 7:09 PM

So this isn't trying to have any behavioural changes?

Explains why I can't find them :D

This revision is now accepted and ready to land.Apr 21 2019, 7:09 PM
This revision was automatically updated to reflect the committed changes.