[Differential] [Accepted] D2760: style and high dpi fixes for screen identification OSD

[broulik (Kai Uwe Broulik)]

broulik accepted this revision.
broulik added a reviewer: broulik.
broulik added a comment.
This revision is now accepted and ready to land.


  Lgtm. Some nitpicks below.
  
  For some reason the right border is smaller than the other ones (has been the case without this patch already), so maybe QML Rectangle border leaks outside the window or so :/

INLINE COMMENTS

> OutputIdentifier.qml:37
>  
> -                text: root.outputName;
> -                wrapMode: Text.WordWrap;
> -                font.pointSize: 50;
> -                color: palette.text;
> -                horizontalAlignment: Text.AlignHCenter;
> -            }
> +    width: childrenRect.width + units.largeSpacing * 4
> +    height: childrenRect.height + units.largeSpacing * 2

width: childrenRect.width + 2 * childrenRect.x to reduce magic numbers

> OutputIdentifier.qml:46
> +        y: units.largeSpacing
> +        font.pointSize: theme.defaultFont.pointSize * 3
> +        anchors {

Why not just a Label since you're not using the one thing that makes Heading special (the font size with "level" magic)

> OutputIdentifier.qml:47-49
> +        anchors {
> +            topMargin: units.largeSpacing * 2
> +        }

Remove, you're not setting a top anchor and you set Y already

REPOSITORY
  rKSCREEN KScreen

BRANCH
  sebas/screenid

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: sebas, #plasma, broulik
Cc: broulik, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160913/fa01704f/attachment-0001.html>