kicker: add option to make root level icons visible
ClosedPublic

Authored by i.Dark_Templar on Mar 17 2019, 3:45 PM.

Details

Summary

In file applets/kicker/package/contents/config/main.xml an option 'showIconsRootLevel' is mentioned. It has following description:
"Whether to show icons on the root level of the menu."

This change provides implementation for this option.

Test Plan

Manual test:

  1. Change menu to 'classic menu' (kicker), notice root level menu items have no icons
  2. Open menu settings widget
  3. Notice new checkbox 'Show icons on the root level of the menu'
  4. Check that checkbox and hit 'Apply' button
  5. Root menu items should now have icons similar which look similar to icons in 'modern menu' (kickoff)

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
i.Dark_Templar created this revision.Mar 17 2019, 3:45 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 17 2019, 3:45 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
i.Dark_Templar requested review of this revision.Mar 17 2019, 3:45 PM
ngraham added a subscriber: ngraham.

Screenshots are always appreciated for changes to the UI. :)

Attaching screenshot with current look of menu (or with disabled option) and new look with enabled option and also a configuration widget.

hein added a comment.Mar 17 2019, 4:28 PM

Kicker intentionally doesn't have icons on the root level to not conflict with the favorites column and reduce visual noise.

i.Dark_Templar added a comment.EditedMar 17 2019, 4:39 PM

Favorite icons are much larger, it's hard to confuse them with root level menu icons. If it's deemed as informational noise, it's disabled by default. But in case it's not, it may be enabled if this change is applied. And in my opinion it looks more consistent in style with 'modern menu' since 'modern menu' has those icons enabled.

Edit: and it looks more consistent with menu editor which show those root level icons. It looks weird for me when menu editor shows those icons but menu does not.

GB_2 added a subscriber: GB_2.Apr 2 2019, 1:56 PM

Favorite icons are much larger, it's hard to confuse them with root level menu icons. If it's deemed as informational noise, it's disabled by default. But in case it's not, it may be enabled if this change is applied. And in my opinion it looks more consistent in style with 'modern menu' since 'modern menu' has those icons enabled.

Edit: and it looks more consistent with menu editor which show those root level icons. It looks weird for me when menu editor shows those icons but menu does not.

I would personally like to have this feature.

GB_2 added a comment.Apr 13 2019, 6:19 AM
This comment was removed by GB_2.
GB_2 added a comment.Apr 25 2019, 6:46 PM

Can you please update this patch? It doesn't apply anymore. It would also be nice if you used Arcanist/arc in the future: https://community.kde.org/Infrastructure/Phabricator#Using_Arcanist_to_post_patches

In D19829#456234, @GB_2 wrote:

Can you please update this patch? It doesn't apply anymore. It would also be nice if you used Arcanist/arc in the future: https://community.kde.org/Infrastructure/Phabricator#Using_Arcanist_to_post_patches

Is it going to be merged? Otherwise, there's no point in updating it here.

GB_2 added a comment.Apr 29 2019, 11:09 AM

I would accept it.

Rebased to master, but it needs testing.

GB_2 accepted this revision as: VDG, GB_2.Apr 29 2019, 4:28 PM

Works great.

This revision is now accepted and ready to land.Apr 29 2019, 4:28 PM
ngraham added 1 blocking reviewer(s): hein.Apr 29 2019, 4:34 PM

Ultimately it's up to @hein as the maintainer. :)

This revision now requires review to proceed.Apr 29 2019, 4:34 PM
hein accepted this revision.Fri, May 31, 5:51 PM

I'm still a bit grumpy about this and dragging my feet! My gut still says it's both option clutter and UI noise :-)

However:

  • There's a fair amount of people calling for this now.
  • The underlying code is already there.
  • Since the codepath is already in, it's more likely to bitrot when there isn't a GUI option that allows testing it.

It's a go.

This revision is now accepted and ready to land.Fri, May 31, 5:51 PM

@i.Dark_Templar can you land this yourself?

@i.Dark_Templar can you land this yourself?

I don't think I have access for that. Also I don't have Arc at the moment, and there's no indication in web interface that I have capability to do it.

This revision was automatically updated to reflect the committed changes.