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

[Filip Fila]

filipf added a comment.


  In D26880#600026 , @abetts wrote:
  
  > 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:
  
  F7927015: image.png 
  F7927017: image.png 
  
  - 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

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26880

To: filipf, ngraham, #vdg, #plasma, hein, broulik
Cc: abetts, hein, gvgeo, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, ahiemstra, mart

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

[Andres Betts]

abetts added a comment.


  I would suggest a circular button instead of square.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26880

To: filipf, ngraham, #vdg, #plasma, hein, broulik
Cc: abetts, hein, gvgeo, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, ahiemstra, mart

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

[Filip Fila]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:7127464c0d66: [Task Manager] Draw audio icon highlight 
effect behind the icon, not in front… (authored by filipf).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26880?vs=74269=74275

REVISION DETAIL
  https://phabricator.kde.org/D26880

AFFECTED FILES
  applets/taskmanager/package/contents/ui/AudioStream.qml

To: filipf, ngraham, #vdg, #plasma, hein, broulik
Cc: hein, gvgeo, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  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).

REPOSITORY
  R119 Plasma Desktop

BRANCH
  no-hover-overlay (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D26880

To: filipf, ngraham, #vdg, #plasma, hein
Cc: hein, gvgeo, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

[Filip Fila]

filipf updated this revision to Diff 74269.
filipf added a comment.


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

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26880?vs=74267=74269

BRANCH
  no-hover-overlay (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D26880

AFFECTED FILES
  applets/taskmanager/package/contents/ui/AudioStream.qml

To: filipf, ngraham, #vdg, #plasma, hein
Cc: hein, gvgeo, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

[Filip Fila]

filipf added a comment.


  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:
  
  F7923931: Screenshot_20200123_190024.png 

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


REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26880

To: filipf, ngraham, #vdg, #plasma, hein
Cc: hein, gvgeo, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

[Filip Fila]

filipf added a subscriber: hein.
filipf added a comment.


  In D26880#599907 , @gvgeo wrote:
  
  > 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 ?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26880

To: filipf, ngraham, #vdg
Cc: hein, gvgeo, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

[Filip Fila]

filipf updated this revision to Diff 74267.
filipf added a comment.


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

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26880?vs=74261=74267

BRANCH
  no-hover-overlay (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D26880

AFFECTED FILES
  applets/taskmanager/package/contents/ui/AudioStream.qml

To: filipf, ngraham, #vdg
Cc: gvgeo, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

[George Vogiatzis]

gvgeo added a comment.


  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.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26880

To: filipf, ngraham, #vdg
Cc: gvgeo, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

[Filip Fila]

filipf created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
filipf requested review of this revision.

REVISION 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.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  no-hover-overlay (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D26880

AFFECTED FILES
  applets/taskmanager/package/contents/ui/AudioStream.qml

To: filipf
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart