D13880: [KMoreTools] Reduce menu hierarchy

This revision was automatically updated to reflect the committed changes. Closed by commit R304:9f54611a472d: [KMoreTools] Reduce menu hierarchy (authored by nicolasfella). REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13880?vs=41581&id=41716 REVISION DETA

D13880: [KMoreTools] Reduce menu hierarchy

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R304 KNewStuff BRANCH arcpatch-D13880 REVISION DETAIL https://phabricator.kde.org/D13880 To: nicolasfella, gregormi, dhaumann, ngraham Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13880: [KMoreTools] Reduce menu hierarchy

nicolasfella updated this revision to Diff 41581. nicolasfella added a comment. - Address comments REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13880?vs=41575&id=41581 BRANCH arcpatch-D13880 REVISION DETAIL https://phabricator.kde.org/D13880 AFFE

D13880: [KMoreTools] Reduce menu hierarchy

dhaumann added inline comments. INLINE COMMENTS > kmoretools.cpp:569 > +if (!mstruct.notInstalledServices.isEmpty()) { > +qDebug() << "notInstalledItems not empty => build 'Not > installed' section"; > +parent->addSection(i18nc("@action:inmenu", "Not installed:"))

D13880: [KMoreTools] Reduce menu hierarchy

dhaumann added a comment. Almost good enough, can you remove the forward declaration again? Just another one-line change. I then don't have any objections anymore. INLINE COMMENTS > kmoretools.h:536 > class KMoreToolsMenuBuilderPrivate; > +class KmtMenuStructure; > Please remove also thi

D13880: [KMoreTools] Reduce menu hierarchy

nicolasfella updated this revision to Diff 41575. nicolasfella added a comment. - Move method to private REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13880?vs=37136&id=41575 BRANCH arcpatch-D13880 REVISION DETAIL https://phabricator.kde.org/D13880

D13880: [KMoreTools] Reduce menu hierarchy

ngraham added a comment. @nicolasfella ping! It would be a shame to lose this since it's already quite a good improvement. REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D13880 To: nicolasfella, gregormi, dhaumann, ngraham Cc: kde-frameworks-devel, michaelh, ngrah

D13880: [KMoreTools] Reduce menu hierarchy

dhaumann added a comment. Ok - then with minor changes, let's move this forward. Who is going to do the work? :-) INLINE COMMENTS > kmoretools.cpp:650 > +if (mstruct.mainItems.isEmpty()) { > +createMoreMenu(mstruct, menu); > +} else { This line then changes to d-

D13880: [KMoreTools] Reduce menu hierarchy

ngraham added a comment. I'm okay with this patch as a first step, FWIW. REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D13880 To: nicolasfella, gregormi, dhaumann, ngraham Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13880: [KMoreTools] Reduce menu hierarchy

nicolasfella added a comment. In D13880#307597 , @dhaumann wrote: > What is the current state of this? > The new private functions should move to the pimpl class behind the d-pointer. > > In general: Looking at the public classes, /all/ th

D13880: [KMoreTools] Reduce menu hierarchy

dhaumann added a comment. What is the current state of this? The new private functions should move to the pimpl class behind the d-pointer. In general: Looking at the public classes, /all/ the private functions should have been hidden behind the pimpl class... REPOSITORY R304 KNewStuf

D13880: [KMoreTools] Reduce menu hierarchy

gregormi added a comment. The spirit goes in the right direction; please proceed :-). Note, that Kate's project plugin also uses KMoreTools. REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D13880 To: nicolasfella, gregormi, dhaumann, ngraham Cc: kde-frameworks-deve

D13880: [KMoreTools] Reduce menu hierarchy

ngraham added a comment. Sounds good to me! It sounds like we're all in agreement? REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D13880 To: nicolasfella, gregormi, dhaumann, ngraham Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13880: [KMoreTools] Reduce menu hierarchy

dhaumann added a comment. I like the idea as well. +1 REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D13880 To: nicolasfella, gregormi, dhaumann, ngraham Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13880: [KMoreTools] Reduce menu hierarchy

nicolasfella added a comment. In D13880#287481 , @gregormi wrote: > +1 for your suggestions. Downside: higher implementation effort. I like it, too, and I am willing to put effort in it, as soon as we agree on a way forward. > A bit

D13880: [KMoreTools] Reduce menu hierarchy

gregormi added a comment. > You are the most awesome person in the world today. +1 :-) > Pro: Very flat hierarchy, code could be simplified a lot > Con: Unwanted and uninstalled tools would be present all the time. Exactly my thoughts. The idea behind the More menu was to hide

D13880: [KMoreTools] Reduce menu hierarchy

ngraham added a comment. If it's important for uninstalled tools to be not always be visible, maybe what we should do is embed the //content// of the KNewStuff menu in Spectacle rather than exposing the whole thing as a sub-menu. Then we could make a few string changes, and it would be like

D13880: [KMoreTools] Reduce menu hierarchy

nicolasfella added a comment. In D13880#286816 , @ngraham wrote: > Or even this: > > Open SimpleScreenRecorder > - > Install Peek > Install Vokoscreen > - > Configur

D13880: [KMoreTools] Reduce menu hierarchy

nicolasfella added a comment. In D13880#286820 , @ngraham wrote: > In D13880#286818 , @nicolasfella wrote: > > > Do you know if/how we can check if there is a handler for appstream://? So we can fal

D13880: [KMoreTools] Reduce menu hierarchy

ngraham added a comment. In D13880#286818 , @nicolasfella wrote: > Do you know if/how we can check if there is a handler for appstream://? So we can fall back to website if none is available I think you added that data and functionality

D13880: [KMoreTools] Reduce menu hierarchy

nicolasfella added a comment. In D13880#286800 , @ngraham wrote: > You are the most awesome person in the world today. Yay! > Here's a radical thought: get rid of the More menu entirely and move all of its content inline, like so:

D13880: [KMoreTools] Reduce menu hierarchy

ngraham added a comment. Or even this: Open SimpleScreenRecorder - Install Peek Install Vokoscreen - Configure... ...And clicking on the menu items for any of the Install options would open the `appstream:/

D13880: [KMoreTools] Reduce menu hierarchy

ngraham added a comment. You are the most awesome person in the world today. Here's a radical thought: get rid of the More menu entirely and move all of its content inline, like so: Installed: SimpleScreenRecorder - Not inst

D13880: [KMoreTools] Reduce menu hierarchy

nicolasfella edited the test plan for this revision. REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D13880 To: nicolasfella, gregormi, dhaumann, ngraham Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13880: [KMoreTools] Reduce menu hierarchy

nicolasfella created this revision. nicolasfella added reviewers: gregormi, dhaumann, ngraham. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. nicolasfella requested review of this revision. REVISION SUMMARY Skip 'More' submenu