Close the system tray on escape - system tray
ClosedPublic

Authored by davidedmundson on Nov 4 2016, 9:40 AM.

Details

Summary

A lot of applets have independent handling making escape close the
expanded representation. However not all of them do.

This leads to both duplicated code and inconsistencies, so we should
tackle this at the source.

If an applet was already using escape internally it will be accepting
the event, and so this code will not cause any regressions.

This patch also removes the stackview forwarding events to the active
child as this doesn't make sense - the active child will already have
focus and get the key event first, so this just created a recursive
chain (which Qt was thankfully fixing for us). Removing it is needed for
this to work.

BUG: 362657

Test Plan

Pressed escape on various applets (e.g plasma-pa)
Pressed escape on the expander applet list view

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
davidedmundson retitled this revision from to Close the system tray on escape - system tray.
davidedmundson updated this object.
davidedmundson edited the test plan for this revision. (Show Details)
davidedmundson added a reviewer: Plasma.
Restricted Application added a project: Plasma. · View Herald TranscriptNov 4 2016, 9:40 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
mart added a subscriber: mart.Nov 4 2016, 9:52 AM
mart added inline comments.
applets/systemtray/package/contents/ui/PlasmoidPopupsContainer.qml
30

wouldn't this break keyboard navigation in an item if supported? (like going up and down in the device list in the device notifier)

davidedmundson added inline comments.Nov 4 2016, 10:04 AM
applets/systemtray/package/contents/ui/PlasmoidPopupsContainer.qml
30

It shouldn't. See paragraph 4 of description.

However, when testing I can't get keyboard nav of device notifier to work pre-patch, and it continuued not working after this patch.

mart accepted this revision.Nov 4 2016, 10:13 AM
mart added a reviewer: mart.
This revision is now accepted and ready to land.Nov 4 2016, 10:13 AM
This revision was automatically updated to reflect the committed changes.
broulik added a subscriber: broulik.Jan 5 2017, 2:41 PM
broulik added inline comments.
applets/systemtray/package/contents/ui/PlasmoidPopupsContainer.qml
30

It did break it. I can no longer automatically type in Klipper or use the shortcuts in Media Controller. Pressing Escape to close still works with this.

davidedmundson added inline comments.Jan 5 2017, 2:45 PM
applets/systemtray/package/contents/ui/PlasmoidPopupsContainer.qml
30

Interestingly, it works the first time, but not on reactivation.

I'd like to fix that properly as it's a different bug (that was previously just hidden) which might be causing other problems.

Worst case if it's not done before 5.9 we can revert this