[applets/clipboard]: Fix applet closing itself when clicking on a history item while the tray popup is pinned open
ClosedPublic

Authored by epopov on Mar 29 2020, 7:16 PM.

Details

Summary

When I pin a Klipper widget and then click on a history item, the Klipper widget closes.

To fix this bug, we need to check if Klipped is pinned, but to be able to do this, we also need to "forward" the pinned state from the SystemTray applet to the Klipper applet.

BUG: 416510
FIXED-IN: 5.19.0

Diff Detail

Repository
R120 Plasma Workspace
Lint
Lint Skipped
Unit
Unit Tests Skipped
epopov created this revision.Mar 29 2020, 7:16 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 29 2020, 7:16 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
epopov requested review of this revision.Mar 29 2020, 7:16 PM

As a question, rather than a researched proposal, is it better to check for the pin in?

systemtray / PlasmoidItem.qml

Connections {
    target: applet
    onExpandedChanged: {
applets/systemtray/package/contents/ui/main.qml
158 ↗(On Diff #78805)

What happens when this is set with some other applet that doesn't have a pin config option?

As a question, rather than a researched proposal, is it better to check for the pin in?

systemtray / PlasmoidItem.qml

Connections {
    target: applet
    onExpandedChanged: {

We should be able to hide the pinned applet by clicking in the system tray (at least this can be done now).

epopov updated this revision to Diff 78827.Mar 29 2020, 11:54 PM

Use hideOnWindowDeactivate property instead of configuration.pin

epopov marked an inline comment as done.Mar 29 2020, 11:54 PM
ngraham retitled this revision from Bug 416510: Klipper applet closes itself when I click on a history item while the system tray popup is pinned open to [applets/clipboard]: Fix applet closing itself when clicking on a history item while the tray popup is pinned open.Mar 30 2020, 12:21 AM
ngraham edited the summary of this revision. (Show Details)
ngraham added a reviewer: Plasma.

This needs a rebase on master, as it currently does not apply.

Can't be rebased on master

It can, you just need to fix the merge conflicts manually. If it doesn't apply cleanly to master, that means it can't be landed on master. :)

epopov added a comment.EditedMar 30 2020, 6:22 PM

In Plasma/5.18 branch there was a Containment.onAppletAdded handler that took the applet as a parameter, but I can't find this handler in master branch. Unfortunately, I don’t have enough knowledge of Plasma API to know if I can add this handler manually.

You can add it.

epopov updated this revision to Diff 78926.Mar 30 2020, 7:11 PM

Rebase on master

Hmm, this isn't working for me. When I pin open the system tray popup and click on a Klipper history item, the popup stays open as expected, but the Klipper applet itself closes, so the pop-up is replaced with the applets list.

Is this working for you?

I have no idea how to test code from master branch

You'll need to compile Plasma from git master, and then run your master-branch plasma session. There are instructions here: https://community.kde.org/Get_Involved/development#Plasma

Feel free to ping me or anyone else in Plasma or #kde-devel on Freenode IRC or Matrix (see https://community.kde.org/Get_Involved/development#Communicate_with_the_team) if you need a hand.

epopov updated this revision to Diff 78992.Mar 31 2020, 2:35 PM

Rebase on master, second edition. Can you try this patch?

ngraham accepted this revision.Mar 31 2020, 2:58 PM

Lovely. Works perfectly.

This revision is now accepted and ready to land.Mar 31 2020, 2:58 PM
ngraham edited the summary of this revision. (Show Details)Mar 31 2020, 2:59 PM
This revision was automatically updated to reflect the committed changes.
epopov updated this revision to Diff 79034.EditedApr 1 2020, 9:41 AM

Unfortunately, the previous patch doesn't solve the problem completely, but this one should. Can you check it and if everything works well, then replace the previous one with it?

What was the problem with the previous one? It works great for me.

Can you submit your changes as a new patch rather than updating this one?

epopov added a comment.Apr 1 2020, 1:44 PM

The previous patch updates the pinned state only for the active applet. For instance, if you open the Networks applet and pin it, then switch to the Clipboard applet and click on the history item, then the Clipboard applet closes. This is because we need to use binding instead of assignment.