D9215: Make Appmenu work based on the presence of a visual representation

2017-12-12 Thread Andres Betts
abetts added a comment. In https://phabricator.kde.org/D9215#178733, @davidedmundson wrote: > > Is there a screenshot of this change? > > It's effectively non visual Love it! ha! REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D9215 To: mart,

D9215: Make Appmenu work based on the presence of a visual representation

2017-12-12 Thread David Edmundson
davidedmundson added a comment. > Is there a screenshot of this change? It's effectively non visual REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D9215 To: mart, #plasma, davidedmundson Cc: abetts, davidedmundson, ngraham, mvourlakos, broulik, plasma-d

D9215: Make Appmenu work based on the presence of a visual representation

2017-12-12 Thread Andres Betts
abetts added a comment. Is there a screenshot of this change? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D9215 To: mart, #plasma, davidedmundson Cc: abetts, davidedmundson, ngraham, mvourlakos, broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-

D9215: Make Appmenu work based on the presence of a visual representation

2017-12-12 Thread Marco Martin
This revision was automatically updated to reflect the committed changes. Closed by commit R120:19ab6f3bbe55: Make Appmenu work based on the presence of a visual representation (authored by mart). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D9215?vs=23806&id=23810#toc REPOSITORY R120

D9215: Make Appmenu work based on the presence of a visual representation

2017-12-12 Thread David Edmundson
davidedmundson accepted this revision. This revision is now accepted and ready to land. REPOSITORY R120 Plasma Workspace BRANCH arcpatch-D9215 REVISION DETAIL https://phabricator.kde.org/D9215 To: mart, #plasma, davidedmundson Cc: davidedmundson, ngraham, mvourlakos, broulik, plasma-devel

D9215: Make Appmenu work based on the presence of a visual representation

2017-12-12 Thread Marco Martin
mart marked an inline comment as done. mart added inline comments. INLINE COMMENTS > davidedmundson wrote in appmenuapplet.cpp:60 > I don't understand what this else clause is about it registers or unregisters the service when the destroyed value of the applet change, and not in the dtor, becau

D9215: Make Appmenu work based on the presence of a visual representation

2017-12-12 Thread Marco Martin
mart updated this revision to Diff 23806. mart marked an inline comment as done. mart added a comment. - better comments REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9215?vs=23600&id=23806 BRANCH arcpatch-D9215 REVISION DETAIL https://phabr

D9215: Make Appmenu work based on the presence of a visual representation

2017-12-12 Thread David Edmundson
davidedmundson added inline comments. INLINE COMMENTS > appmenuapplet.cpp:60 > +//if we're the first, regster the service > +if (++s_refs == 1) { > + > QDBusConnection::sessionBus().interface()->registerService(s_viewService, I don't understand what this e

D9215: Make Appmenu work based on the presence of a visual representation

2017-12-09 Thread Marco Martin
mart added a dependent revision: D9267: remove menubar settings. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D9215 To: mart, #plasma Cc: mvourlakos, broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D9215: Make Appmenu work based on the presence of a visual representation

2017-12-07 Thread Marco Martin
mart updated this revision to Diff 23600. mart added a comment. remove dead code REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9215?vs=23536&id=23600 BRANCH phab/autoappmenu REVISION DETAIL https://phabricator.kde.org/D9215 AFFECTED FILES

D9215: Make Appmenu work based on the presence of a visual representation

2017-12-07 Thread Marco Martin
mart added inline comments. INLINE COMMENTS > mvourlakos wrote in appmenu.cpp:211 > is this ok? isnt that return blocks any code below? eww, right, a leftover.. it means that the whole content of the method should get droppped now as there is no such thing as reloading configuration. the method

D9215: Make Appmenu work based on the presence of a visual representation

2017-12-06 Thread Michail Vourlakos
mvourlakos added inline comments. INLINE COMMENTS > appmenu.cpp:211 > hideMenu(); // hide window decoration menu if exists > - > +return; > KConfigGroup > config(KSharedConfig::openConfig(QStringLiteral("kdeglobals")), > QStringLiteral("Appmenu Style")); is this ok? isnt that return

D9215: Make Appmenu work based on the presence of a visual representation

2017-12-06 Thread Michail Vourlakos
mvourlakos added a comment. +1 very good... REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D9215 To: mart, #plasma Cc: mvourlakos, broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D9215: Make Appmenu work based on the presence of a visual representation

2017-12-06 Thread Kai Uwe Broulik
broulik added a comment. +1 to the idea REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D9215 To: mart, #plasma Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D9215: Make Appmenu work based on the presence of a visual representation

2017-12-05 Thread Marco Martin
mart created this revision. mart added a reviewer: Plasma. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY drop the internal settings InApp,Menu, Decoration but instead export the menu based on the presence of a dbus ser