D27160: [mediacontroller] WIP: Visually refresh media controller plasmoid

[Kai Uwe Broulik]

broulik added a comment.


  When there is no player it completely falls apart. It also doesn't work when there is no album art. The fallback player icon is gone.
  F8086262: Screenshot_20200205_093259.PNG <https://phabricator.kde.org/F8086262>
  
  The line spacing also seems a little excessive. Since there's a massive right padding to match the one on the left, you often end up with awkward line breaks:
  F8086264: Screenshot_20200205_093515.PNG <https://phabricator.kde.org/F8086264>
  Note how the "seh" could have easily fitted in the same line. It now also looks a bit awkward when you can't seek where there's no slider handle.
  
  It looks really lovely with short texts
  F8086272: Screenshot_20200205_093836.PNG <https://phabricator.kde.org/F8086272>
  but you know...
  F8086274: Screenshot_20200205_094303.PNG <https://phabricator.kde.org/F8086274>
  And obviously I would prefer a better look for the player selection combo :)

INLINE COMMENTS

> ExpandedRepresentation.qml:137
> +            Layout.margins: units.smallSpacing
> +            Layout.alignment: Qt.AlignHCenter
> +            Layout.fillWidth: true

Superfluous since you fill both width and height?

> ExpandedRepresentation.qml:145-148
> +                sourceSize.width: 512 /* Setting a source size means the item doesn't need
> +                                        to recompute blur as the user resizes the plasmoid
> +                                        Additionally, it puts a bit of a cap on how large the
> +                                        buffer getting blurred can be, saving resources.

Do you have a source for this claim? From what I know blur merely blurs the actual item texture which is rendered, no the raw image source?

> ExpandedRepresentation.qml:205
> +                        
> +                        color: softwareRendering ? undefined : "white"
>  

Does assigning `undefined` work? I don't see a `RESET` property for it. Probably want to assign color scope text color instead?

> ExpandedRepresentation.qml:212
>  
> -            source: mpris2Source.currentData["Desktop Icon Name"]
> -            visible: !albumArt.visible
> +                        Layout.maximumWidth: expandedRepresentation.width * (2/5)
> +                    }

Make sure to `Math.round` random factors of a size

> ExpandedRepresentation.qml:289
> +            Layout.fillWidth: true
> +            Layout.maximumWidth: Math.min(800, expandedRepresentation.width*(7/10))
> +

No magic numbers, please

> ExpandedRepresentation.qml:306
>                  font: theme.smallestFont
> +                color: softwareRendering ? undefined : "white"
>              }

The labels are shown outside the album art, so they are unreadable now

REPOSITORY
  R120 Plasma Workspace

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

To: cblack, #vdg, #plasma, manueljlin
Cc: broulik, gvgeo, davidedmundson, ngraham, manueljlin, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200205/5197b052/attachment-0001.html>