Handle keyboard in action selector OSD
ClosedPublic

Authored by gladhorn on Jul 16 2018, 6:51 PM.

Details

Summary

The OSD is not all that bad, but it was only usable with the mouse.
Change it so that left-right and tab navigates and space/return confirms
the selection.

BUG: 395804

Diff Detail

Repository
R104 KScreen
Branch
arcpatch-D14165
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 970
Build 983: arc lint + arc unit
gladhorn created this revision.Jul 16 2018, 6:51 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJul 16 2018, 6:51 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
gladhorn requested review of this revision.Jul 16 2018, 6:51 PM
gladhorn added inline comments.
kded/qml/OsdSelector.qml
158

I'd like to hear suggestions how to do this in a nicer fashion, iterating over the children and checking if they are buttons and otherwise skipping them feels pretty ugly.

davidedmundson added inline comments.
kded/qml/OsdSelector.qml
158

Personally I would add a property currentIndex in the repeater, have Keys.right/left manipulate that and then have the delegates compare that to their index.
It would match a ListView pattern

184

This breaks the checked: activeFocus binding in the delegate, which means use of tab after nav keys gets out of sync.

broulik added inline comments.
kded/qml/OsdSelector.qml
155

Also do Keys.onEnterPressed so that enter on numpad works

167

Make sure you take into account right-to-left languages (not sure if the layout mirrors then but it should, check by running the app with -reverse argument)

Awesome! I'll incorporate the ideas, that's great. Any chance to get https://phabricator.kde.org/D14143 in? It's required for this since it changes the window type. I'd also like to make the global shortcut show and hide the osd (right now pressing it again makes the osd fade out and in again).

gladhorn updated this revision to Diff 38003.Jul 18 2018, 9:27 AM
gladhorn marked 4 inline comments as done.

Fixed keyboard bindings breaking all the time, as indicated in review

davidedmundson requested changes to this revision.Jul 18 2018, 9:33 AM
davidedmundson added inline comments.
kded/qml/OsdSelector.qml
107

Given we're doing this manually, we should drop the ButtonRow which is also meddling with what's checked or not.

Row should be a drop in replacement.

162

why do you accept right, but not left?

This revision now requires changes to proceed.Jul 18 2018, 9:33 AM
gladhorn marked an inline comment as done.Jul 19 2018, 9:45 AM
gladhorn added inline comments.
kded/qml/OsdSelector.qml
107

Row does result in different spacing. I wonder if it's better to leave the ButtonRow without the exclusive set, or replace it with row and add spacing. On the other hand, it's just one line.

167

Currently -reverse has no effect on the whole thing afaict.

gladhorn updated this revision to Diff 38073.Jul 19 2018, 9:46 AM

Fix accepting of left key event and use Row instead of ButtonRow

This revision was not accepted when it landed; it landed in state Needs Review.Jul 24 2018, 12:49 PM
This revision was automatically updated to reflect the committed changes.