Fix hardcoded private browsing icon
ClosedPublic

Authored by GB_2 on Dec 1 2018, 7:50 PM.

Details

Summary

BUG: 397078

Fixes the hardcoded private browsing icon in the Falkon application menu.

Test Plan

Open the Falkon application menu on the top right.

Diff Detail

Repository
R875 Falkon
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
GB_2 created this revision.Dec 1 2018, 7:50 PM
Restricted Application added a subscriber: falkon. · View Herald TranscriptDec 1 2018, 7:50 PM
GB_2 requested review of this revision.Dec 1 2018, 7:50 PM
drosca added a subscriber: drosca.Dec 1 2018, 8:09 PM

im-user has completely different meaning than "private browsing".

GB_2 added a comment.Dec 1 2018, 8:11 PM

im-user has completely different meaning than "private browsing".

But it looks very similar the original one.
What other icon should it be?

drosca added a comment.Dec 1 2018, 8:12 PM
In D17288#369538, @GB_2 wrote:

But it looks very similar the original one.
What other icon should it be?

Maybe in your icon theme.
There is no "standard" icon for this action, so that's why it uses hardcoded icon.

GB_2 added a comment.EditedDec 1 2018, 8:22 PM

Maybe in your icon theme.
There is no "standard" icon for this action, so that's why it uses hardcoded icon.

It looks like the original one in Breeze too...
Is the icon "visibility" or "hint" better?
The hardcoded icon is a problem, when you use a dark theme.
From the HIG:
"Use icons from the system icon theme whenever possible. Avoid using custom icons. New icons should be added to an icon theme."

Yes, so there should be new icon in Breeze theme (eg. private-browsing or maybe better private-mode so it is more generic) and then Falkon can use it.

GB_2 added a comment.Dec 2 2018, 10:16 AM

Ok, but what should it look like, because the existing Breeze icons I told you about already look like a "private browsing/private mode" icon.

Then just use this icon, but make a symlink to private-mode.

GB_2 added a comment.Dec 2 2018, 10:26 AM

Good idea!
But which of the 3 icons now?

  • im-user
  • visibilty
  • hint

That's for discussion with Breeze devs.

ngraham added a subscriber: ngraham.Dec 2 2018, 7:16 PM
drosca accepted this revision.Dec 9 2018, 7:31 PM
This revision is now accepted and ready to land.Dec 9 2018, 7:31 PM
ngraham added a comment.EditedDec 9 2018, 7:37 PM

@drosca, you'll need to land this for @GB_ with authorship information Björn Feber <feber70@gmail.com>.

GB_2, have you considered applying for a Developer account so you can land your own patches? See https://community.kde.org/Infrastructure/Get_a_Developer_Account

This revision was automatically updated to reflect the committed changes.