Port ResultDelegate to use PlasmaComponents 3.0
Needs RevisionPublic

Authored by apol on Jul 18 2019, 8:01 PM.

Details

Reviewers
broulik
ngraham
Group Reviewers
Plasma
Test Plan

Used it as much extensibly as I knew how to

Diff Detail

Repository
R112 Milou
Branch
pc3 (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 14575
Build 14593: arc lint + arc unit
apol created this revision.Jul 18 2019, 8:01 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJul 18 2019, 8:01 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
apol requested review of this revision.Jul 18 2019, 8:01 PM
apol updated this revision to Diff 62007.Jul 18 2019, 8:01 PM

More into the Layout

apol updated this revision to Diff 62020.Jul 19 2019, 12:51 AM

Added the rest, it wasn't that much

broulik added inline comments.
lib/qml/ResultDelegate.qml
143–145

Move that comment to where it belongs

147

This should be a Svg from the Plasma Theme

160

Where does that random 0.7 come from?

230

QQC2 tooltip stuff doesn't really work for KRunner because they're not separate windows :(

Also, don't use ToolTip attached property as it cannot respect the theme's delay settings

davidedmundson added inline comments.
lib/qml/ResultDelegate.qml
230

Also, don't use ToolTip attached property as it cannot respect the theme's delay settings

Please don't change it.

The alternative (using a ToolTip instance) is ridiculously heavy.

apol marked 2 inline comments as done.Jul 25 2019, 12:15 AM
apol added inline comments.
lib/qml/ResultDelegate.qml
147

There's no separator svg. you suggest creating a new one?

230

QQC2 tooltip stuff doesn't really work for KRunner because they're not separate windows :( it looks good to me.

apol updated this revision to Diff 62517.Jul 25 2019, 12:15 AM

address comments

apol added a comment.Mon, Jul 29, 10:49 AM

Ping? This is blocking D22514

apol updated this revision to Diff 62752.Mon, Jul 29, 5:45 PM

Rebase on top of kai's patch

apol updated this revision to Diff 62753.Mon, Jul 29, 6:14 PM

forgot opacity

broulik requested changes to this revision.Tue, Jul 30, 6:55 AM

The highlight isn't using Plasma theme anymore


The layout explodes for long category names, they should be elided.

The tooltips look quite bad and crammed because as I said they're not separate windows anymore. Also, they show up immediately instead of after a delay.
Clicking results doesn't do anything now, and there can now be both a highlight following the mouse and a selection following the keyboard.

lib/qml/ResultDelegate.qml
147

Use whatever the PlasmaComponents.ListItem uses

234

QQC2 tooltip doesn't seem to handle mnemonics, leading to text like "Run in &Terminal"

This revision now requires changes to proceed.Tue, Jul 30, 6:55 AM
apol marked 2 inline comments as done.Tue, Jul 30, 2:43 PM

The highlight isn't using Plasma theme anymore

Of course it's using the theme, unless PlasmaComponents.ItemDelegate.highlighted doesn't use the theme, which then is a bug in plasma framework.

The layout explodes for long category names, they should be elided.

Fixed

The tooltips look quite bad and crammed because as I said they're not separate windows anymore.

Also, they show up immediately instead of after a delay.

Fixed, we could probably find a more generic fix.

Clicking results doesn't do anything now.

Fixed

and there can now be both a highlight following the mouse and a selection following the keyboard.

Would you prefer to change the current on hover? It's certainly doable but I'm not sure we want that.

lib/qml/ResultDelegate.qml
234

Can't see that, see screenshot.
Also if that's the case, then we should fix it like we did for desktop components.

apol updated this revision to Diff 62796.Tue, Jul 30, 2:44 PM

Address kai's comments

ngraham requested changes to this revision.Thu, Aug 8, 4:29 PM
ngraham added a subscriber: ngraham.

Selection highlights have regressed and text is no longer elided if it's too long, it just overflows.

This revision now requires changes to proceed.Thu, Aug 8, 4:29 PM