ngraham added a comment.

  +1, nice visual improvement. Needs a bit of code polishing first before it 
can land though. See inline comments:

INLINE COMMENTS

> introPage.qml:31
> +    property int iconsRectRadius: 10
>  
>      signal focusNextRequest()

Don't hardcode values here; instead use multiples of Kirigami.Units.gridUnit 
(or some other defined unit)

> introPage.qml:57
> +        Item {
> +            // spacer item
> +            width: Kirigami.Units.largeSpacing

Comment not really necessary; it's obvious that this is a spacer

> introPage.qml:63
> +                text: i18n("KDE Plasma")
> +                font.pointSize: 24 // largest level is too small, so we use 
> pointSize
> +            }

Just use level 1, it's big enough IMO (or make the icon next to the text 
smaller to compensate for the smaller text size)

REPOSITORY
  R124 System Settings

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

To: tienisto, #vdg, GB_2, ngraham
Cc: filipf, anthonyfieroni, davidedmundson, mart, Codezela, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, mglb, Pitel, crozbo, ndavis, 
ZrenBot, firef, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, mbohlender

Reply via email to