[Task Manager] Draw audio icon highlight effect behind the icon, not in front of it
ClosedPublic

Authored by filipf on Jan 23 2020, 4:58 PM.

Details

Summary

Some Plasma themes used filled-style highlight effects which means that their audio icon gets completely covered by the highlight effect.

This patch makes the effect be drawn behind the icon.

Test Plan
  • Carl theme

Before:

After:

  • Breeze

Before:

After:

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
filipf created this revision.Jan 23 2020, 4:58 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJan 23 2020, 4:58 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
filipf requested review of this revision.Jan 23 2020, 4:58 PM
filipf edited the test plan for this revision. (Show Details)Jan 23 2020, 5:00 PM
filipf added reviewers: ngraham, VDG.
gvgeo added a subscriber: gvgeo.Jan 23 2020, 5:28 PM

More sane value like z: 1 in the svg should be enough .

But in this case can MouseArea move before svg? Makes more sense to me, having the code in the draw order .
I'm not a developer, just asking.

filipf updated this revision to Diff 74267.Jan 23 2020, 5:45 PM

don't use "z" hacks, just rearrange the order of items
also port the top item to MouseArea to be more efficient

filipf added a subscriber: hein.Jan 23 2020, 5:46 PM

More sane value like z: 1 in the svg should be enough .

But in this case can MouseArea move before svg? Makes more sense to me, having the code in the draw order .
I'm not a developer, just asking.

Now that you got me thinking, I wonder if the top item can just be switched from Item to MouseArea. Works for me so I updated the diff.

@hein ?

I also just noticed a pre-existing bug - that the hover effect filling the mouse area doesn't work well when the icon is smaller:

The icon overflows the hover effect. I'm going to have the effect fill the icon instead:

filipf updated this revision to Diff 74269.Jan 23 2020, 6:03 PM

have the hover effect fill the icon instead of the mousearea so that the icon doesn't overflow the effect at small sizes

ngraham accepted this revision.Jan 23 2020, 6:12 PM

LGTM but let's wait for a Plasma review too to make sure this is the most technically correct way (I think it is but I'm not 100% sure).

This revision is now accepted and ready to land.Jan 23 2020, 6:12 PM
broulik accepted this revision.Jan 23 2020, 8:18 PM
This revision was automatically updated to reflect the committed changes.
abetts added a subscriber: abetts.Jan 23 2020, 10:00 PM

I would suggest a circular button instead of square.

I would suggest a circular button instead of square.

I also think this would look more attractive. I've already investigated the possibility and here's a few notes:

  • our hover/higlight effect is not made to be circular
  • we'd have to create a new CircularHighlight component or simply a new SVG for this
  • drawback: 3rd party themes wouldn't have this SVG for some time
  • I already tried bending the existing SVG to a circle with an opacity mask but that would only worked with filled-style highlights:


  • alternatively we could use a QML rectangle but I don't really like that approach, and not just because it's unthemable but because we won't be able to guarantee it will be legible with all themes