Show root items in kicker menu

Authored by lopatin on Nov 14 2017, 12:41 PM.



BUG: 358291

For some reason kicker appsmodel ignores root items (apps and separators). This patch added support for them.

Diff Detail

R119 Plasma Desktop
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
lopatin created this revision.Nov 14 2017, 12:41 PM
Restricted Application added a project: Plasma. · View Herald TranscriptNov 14 2017, 12:41 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
mart added a subscriber: mart.Nov 20 2017, 4:03 PM

as it changes the behavior, can you provide a screenshot?



ngraham edited the summary of this revision. (Show Details)Nov 30 2017, 5:03 PM
hein accepted this revision.Dec 1 2017, 10:05 AM
hein added a subscriber: hein.

Can you push this or do you need someone to do it for you?

This revision is now accepted and ready to land.Dec 1 2017, 10:05 AM

Where I need to push this? I think that I don't have permissions for this.

ngraham added a subscriber: ngraham.Dec 1 2017, 2:10 PM
hein requested changes to this revision.Dec 1 2017, 2:50 PM

I will push it for you when it's done (you need a developer account to push).

But actually I thought about it some more and I realized I need to ask you for some changes in the patch. I think there may be some existing users (i.e. UIs) of the model which rely on all top-level items in RootModel being categories with child models. That means we need to implement this in a backwards-compatible way. Can you implement a 'showTopLevelApps' (or any better name you come up with) property in AppsModel , default it to false and only show the items when it's set to true? Then we can patch the Kicker and Kickoff UIs to set it to true, but leave e.g. Application Dashboard unchanged.

This revision now requires changes to proceed.Dec 1 2017, 2:50 PM
lopatin updated this revision to Diff 25414.Jan 15 2018, 8:13 PM

Ok, I updated patch and added showTopLevelItems flag.

hein accepted this revision.Jan 16 2018, 1:26 PM
This revision is now accepted and ready to land.Jan 16 2018, 1:26 PM

Do you have commit access?

hein added a comment.Jan 16 2018, 1:39 PM

He doesn't (as discussed above).

This revision was automatically updated to reflect the committed changes.
hein added a comment.Jan 17 2018, 9:38 AM

It's in! Do you want to work on the follow-up patch to Kicker and Kickoff to set the property?

Good! Yes, I do. Some questions:

  1. Should I add an option to enable/disable this flag from config?
  2. Should I create new task in phabricator or post diff here?
  3. Is there any chance that this code will land to Plasma/5.12?
hein added a comment.Jan 17 2018, 1:05 PM

Nah, I think a config option isn't necessary.

New task.

Unfortunately you narrowly missed 5.12. It was branched with the beta release, so new features must go to master (5.13) now.