Changes to global menu applet
ClosedPublic

Authored by chinmoyr on Jan 10 2017, 2:36 PM.

Details

Summary

1> Use applet only if application menu style is set to "Widget". If its not the case show a warning sign instead.
2> When applet is placed in a vertical panel force use the compact view.

and some minor changes.

Diff Detail

Repository
R120 Plasma Workspace
Lint
Lint Skipped
Unit
Unit Tests Skipped
chinmoyr updated this revision to Diff 9981.Jan 10 2017, 2:36 PM
chinmoyr retitled this revision from to Changes to global menu applet.
chinmoyr updated this object.
chinmoyr edited the test plan for this revision. (Show Details)
chinmoyr added a reviewer: davidedmundson.
chinmoyr set the repository for this revision to R120 Plasma Workspace.
Restricted Application added a project: Plasma. · View Herald TranscriptJan 10 2017, 2:36 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript

When applet is placed in a vertical panel force use the compact view.

Why? We can show the buttons below each other if the user wants to

applets/appmenu/lib/appmenuapplet.cpp
109

I think update instead of set is better wording

applets/appmenu/package/contents/config/main.xml
13 ↗(On Diff #10042)

Can't you just use the plugin in the config dialog to avoid this config madness?

applets/appmenu/package/contents/ui/main.qml
71

Don't use "emblem" icons - they are meant for "badges on top of files" for instance

broulik added inline comments.Jan 10 2017, 3:02 PM
applets/appmenu/lib/appmenuapplet.cpp
44

I was wondering if we can't just listen for the presence of the com.canonical.appmenu.regisrar service - if the user places an applet *and* has it on decoration, don't think that's too bad, it's a conscious thing? Not sure.

broulik added inline comments.Jan 11 2017, 11:50 AM
applets/appmenu/lib/appmenuapplet.h
36

Maybe "applicationMenuAppletEnabled" or something like that?

applets/appmenu/package/contents/ui/main.qml
76

Please instead of a separate tooltip area set plasmoid.toolTipSubText, ie. above

Plasmoid.toolTipSubText: !useApplet ? i18n("blabla") : ""
chinmoyr updated this revision to Diff 10042.Jan 11 2017, 1:44 PM

addressed Kai's concerns

chinmoyr updated this revision to Diff 10051.Jan 11 2017, 2:18 PM

got rid of all useless config props

davidedmundson accepted this revision.Jan 11 2017, 2:28 PM
davidedmundson edited edge metadata.
This revision is now accepted and ready to land.Jan 11 2017, 2:28 PM

I just noticed when I change the setting in the KCM the icon changes to the warning icon but the button stays enabled, only if I restart plasma the button is disabled

applets/appmenu/lib/appmenuapplet.h
69

Needs to be a slot:

public slots:
    void updateAppletEnabled();

that's why your connection doesn't work

applets/appmenu/package/contents/ui/main.qml
33

Make it readonly property

122

Can we make it a binding instead of manually onFooChanged?

120 ↗(On Diff #10042)

Go to System Settings → Application Style → Fine Tuning (tab) to enable it. (I would still love if you could add a way for the user to open that KCM, perhaps in context menu?)

chinmoyr updated this revision to Diff 10053.Jan 11 2017, 3:01 PM
chinmoyr edited edge metadata.
broulik accepted this revision.Jan 11 2017, 3:06 PM
broulik added a reviewer: broulik.
chinmoyr closed this revision.Jan 11 2017, 3:28 PM

Closed with commit R120:fa06aac99b2f

ltoscano added inline comments.
applets/appmenu/package/contents/ui/main.qml
55 ↗(On Diff #10042)

A bit late to the party, but if this is the freedesktop name of an icon, then it should not have i18n.