Code review comment for lp:~zsombi/ubuntu-ui-toolkit/palette

Revision history for this message
Zsombor Egri (zsombi) wrote :

> +++ src/Ubuntu/Components/1.3/Icon.qml 2015-12-08 10:34:40 +0000
>
> + * Copyright (C) 2014 Canonical, Ltd.
>
> It's 2015. And your name is missing from the header.
>
> + if (icon.hasOwnProperty("source"))
> + return icon.source;
>
> This will always be true. It should just be source: icon.source.

All true. Fixed.

>
> +++ src/Ubuntu/Components/1.3/TextArea.qml 2015-12-08 10:34:40 +0000
>
> - color: theme.palette.normal.backgroundText
> 238 + color: "blue"//theme.palette.normal.base
>
> What's this?

A bug :)

>
> +++ src/Ubuntu/Components/1.3/UbuntuColors.qml 2015-12-08 10:34:40 +0000
>
> + Orange. Recommended for branded elements, display progress
> 321 + and intensity, textual links on light backgrounds.
>
> This is wrong. Orange should only be used for keyboard focus visuals now.

Not only. Orange is used for selections too, with transparency.

>
> +++ src/Ubuntu/Components/Themes/Ambiance/1.3/PageHeaderStyle.qml
> 2015-12-08 10:34:40 +0000
>
> + // FIXME: introduce inactiveForegroundColor to PageHeaderStyle
> 810 foregroundColor: pageHeaderStyle.foregroundColor
>
> Is design input missing here? Or is this depending on work in the header? A
> bug should be filed in any case.

I did not wanted to mess up with Tim's work, beside design needs to figure this out as well.

>
> +++ src/Ubuntu/Components/Themes/SuruDark/1.3/TabBarStyle.qml 2015-12-08
> 10:34:40 +0000
>
> -
> 1428 +//![0]
>
> What sorcery is this?

Learned from Merlin ;)

« Back to merge proposal