Add alternatives button to applet configuration in panel edit mode
ClosedPublic

Authored by GB_2 on Dec 7 2018, 5:56 PM.

Details

Summary

There is a configure button and an alternatives button in the context menu of an applet, but there is no alternatives button in the popup/overlay of an applet in the panel edit mode, so this adds that button, to make it more consistent and easier to find.

BUG: 405082
FIXED-IN: 5.16.0

Test Plan

Hover over an applet in the panel edit mode.

Diff Detail

Repository
R119 Plasma Desktop
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 10677
Build 10695: arc lint + arc unit
GB_2 created this revision.Dec 7 2018, 5:56 PM
Restricted Application added a subscriber: plasma-devel. · View Herald TranscriptDec 7 2018, 5:56 PM
GB_2 requested review of this revision.Dec 7 2018, 5:56 PM
GB_2 edited the summary of this revision. (Show Details)
GB_2 updated this revision to Diff 47054.Dec 7 2018, 6:00 PM

Update TODO text.

ndavis added a subscriber: ndavis.EditedDec 8 2018, 1:48 AM

I think we should be using a monochrome icon here. An equivalent monochrome icon would be favorite, but I don't think "favorite" is a good symbol for "alternatives". Maybe preferences-other or application-menu (same icon) would be good? It might be outside the scope of this patch anyway since this means the alternatives icon needs to be changed outside the edit mode as well.

ngraham added a subscriber: ngraham.Dec 8 2018, 3:31 AM

I think we should be using a monochrome icon here.

I agree.

It might be outside the scope of this patch anyway since this means the alternatives icon needs to be changed outside the edit mode as well.

Exactly. For now, let's follow the existing pattern, and make that change in another patch.

Neat, it seems to generally work well. I notice that when I click the new menu item, it somewhat unexpectedly exits from panel edit mode. I wonder if we could make it not do that, so it would stay in panel edit mode if invoked while in panel edit mode.

GB_2 added a comment.EditedDec 8 2018, 5:39 PM

Neat, it seems to generally work well. I notice that when I click the new menu item, it somewhat unexpectedly exits from panel edit mode. I wonder if we could make it not do that, so it would stay in panel edit mode if invoked while in panel edit mode.

I can't find where that is implemented and I think it shouldn't be included here, because it exits the panel edit mode when it looses its focus.

In general +++

We need to fix that TODO. I added some rough notes, give that a go, it'll involve reading the C++ and seeing how that all ties together with the QML.

If you get stuck, comment here.

containments/panel/contents/ui/ConfigOverlay.qml
396

As for your TODO:

See in plasma-frameworks applet_p.cpp around line 189

Good news: The "visible" property of this action indicates if we have alternatives or not.
So in theory we would just need to bind it to the ToolButton's visibility.

Probably also worth taking the text and icon source from there too

Bad news: This property is only updated when it receives the signal:
contextualActionsAboutToShow

so that needs to be called somewhere.

GB_2 added a comment.Dec 8 2018, 11:46 PM

I'm trying to emit the signal contextualActionsAboutToShow in the (QML) onVisualParentChanged function, but it doesn't work...

I'm trying to emit the signal contextualActionsAboutToShow in the (QML) onVisualParentChanged function, but it doesn't work...

Use console.log to check your code is being called at the right time.
Also search for contextualActionsAboutToShow in other code and then you'll have a reference you can copy.

GB_2 added a comment.Dec 15 2018, 11:49 AM

I'm trying to emit the signal contextualActionsAboutToShow in the (QML) onVisualParentChanged function, but it doesn't work...

Use console.log to check your code is being called at the right time.
Also search for contextualActionsAboutToShow in other code and then you'll have a reference you can copy.

It is being called at the right time (I can see the output):

onVisualParentChanged: {
    if (visualParent) {
        ...
        console.log("Call contextualActionsAboutToShow");
        currentApplet.applet.contextualActionsAboutToShow();
    }
}

And it should only show the button if there are specific alternatives:

PlasmaComponents.ToolButton {
    id: alternativesButton
    ...
    visible: currentApplet.applet.action("alternatives").visible
    ...
}

But it doesn't work, it prints out Cannot read property 'applet' of null and currentApplet.applet.action("alternatives").visible is always false.

Cannot read property 'applet' of null

Which means currentApplet is null.

When is that set? Were you checking if it was visible afterwards?

GB_2 added a subscriber: broulik.Feb 16 2019, 8:23 AM

It seems like this is quite difficult, me and @broulik couldn't figure it out. If someone wants to take over this revision then please do it.

GB_2 added a comment.EditedMar 1 2019, 7:05 AM

Ping, would be great if we could have this for 5.16.

ngraham edited the summary of this revision. (Show Details)Mar 4 2019, 7:01 PM
GB_2 added a comment.Apr 2 2019, 2:06 PM
This comment was removed by GB_2.
GB_2 edited the summary of this revision. (Show Details)Apr 9 2019, 4:06 PM
Codezela added a subscriber: Codezela.EditedApr 9 2019, 5:27 PM

This icon have a lot of details for this small size we need more clear one I guess

GB_2 added a comment.Apr 9 2019, 5:58 PM

This icon have a lot of details for this small size we need more clear one I guess

The icon was added here: D20367
I think it's still ok, but we can discuss how it can be changed in that revision or the KDE VDG chat channel.

davidedmundson commandeered this revision.Apr 9 2019, 9:12 PM
davidedmundson added a reviewer: GB_2.
davidedmundson edited the summary of this revision. (Show Details)Apr 9 2019, 9:21 PM

Nice, it works great!

containments/panel/contents/ui/ConfigOverlay.qml
391

This should now be widget-alternatives

GB_2 added a comment.Apr 10 2019, 5:11 AM

Nice, thank you so much!

containments/panel/contents/ui/ConfigOverlay.qml
392

And this "Show Alternatives..."

And actually maybe we could programmatically get the name and icon from the action itself, like the Task Manager does:

property QtObject alternativesAction: null

enabled: alternativesAction && alternativesAction.enabled
visible: alternativesAction && alternativesAction.visible

text: alternativesAction ? alternativesAction.text : ""
icon: alternativesAction ? alternativesAction.icon : ""

GB2 do you want to commandeer it back and make those changes?

GB_2 commandeered this revision.Apr 11 2019, 4:45 AM
GB_2 edited reviewers, added: davidedmundson; removed: GB_2.

Sure!

GB_2 added a comment.Apr 11 2019, 5:06 AM

BTW, @davidedmundson or someone else:
How do I get the icon name/source here: iconSource: plasmoid.action("alternatives").icon?
It currently returns a QIcon, but I want the name/source of it from QML.

GB_2 updated this revision to Diff 55958.Apr 11 2019, 5:31 AM
GB_2 edited the summary of this revision. (Show Details)
GB_2 marked 3 inline comments as done.
GB_2 updated this revision to Diff 55959.Apr 11 2019, 5:45 AM
GB_2 updated this revision to Diff 55960.Apr 11 2019, 5:52 AM

Try to fix diff

It currently returns a QIcon, but I want the name/source of it from QML.

What for?

GB_2 added a comment.Apr 12 2019, 3:34 PM

It currently returns a QIcon, but I want the name/source of it from QML.

What for?

So I can use the same icon that is in the context menu

GB_2 added a comment.Apr 12 2019, 3:35 PM

in the toolbutton icon source/name property.

Toolbutton should be able to take a qicon as a source

GB_2 added a comment.Apr 12 2019, 5:40 PM

Toolbutton should be able to take a qicon as a source

Nope, when I use iconSource: plasmoid.action("alternatives").icon this happens:
ConfigOverlay.qml:392:21: Unable to assign QIcon to QString

GB_2 added a comment.EditedApr 12 2019, 5:51 PM

It seems like Elisa has a function to do this: https://lxr.kde.org/source/extragear/multimedia/elisa/src/elisaapplication.h#0083
Maybe something like this could be added to KDE/Plasma Frameworks?

GB_2 added a comment.Apr 13 2019, 7:03 AM

Or we could just leave it how it is now.

broulik requested changes to this revision.Apr 17 2019, 1:41 PM
broulik added inline comments.
containments/panel/contents/ui/ConfigOverlay.qml
228

This is never triggered when you just move the mouse over, you probably want to do that in the onVisualParentChanged handler below

Ideally this was all done using proper bindings instead of imperative code, though.

This revision now requires changes to proceed.Apr 17 2019, 1:41 PM
GB_2 updated this revision to Diff 56435.Apr 17 2019, 1:56 PM

Move currentApplet.applet.prepareContextualActions(); to onVisualParentChanged

GB_2 marked an inline comment as done.Apr 17 2019, 1:56 PM
broulik accepted this revision.Apr 17 2019, 1:57 PM
This revision is now accepted and ready to land.Apr 17 2019, 1:57 PM
ngraham accepted this revision.Apr 17 2019, 1:57 PM
This revision was automatically updated to reflect the committed changes.