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

Revision history for this message
Cris Dywan (kalikiana) wrote :

+++ src/Ubuntu/Components/plugin/ucbottomedge.cpp 2016-01-27 06:39:19 +0000
+++ tests/unit_x11/tst_bottomedge/tst_bottomedge.cpp 2016-01-27 06:39:19 +0000

Unrelated? Probably better in a separate branch?

+ d->m_subtitle = new UCLabel(getSubtitleColor, this);

You're passing a method so the Label gets to evaluate the color lambda-style. But UCLabel.m_defaultColor is right now only called in postThemeChanged. To make this really work you'll also need (the equivalent of) onEnabledChanged and onActiveFocusChanged.
I like the idea, but I feel this should be done cleaner...

This might be the right time to fill a gap in our API. I think we already agree we need state-based colors in some form.

How about getPaletteColor returning a UCPaletteColor(QQuickItem* item, const QString& color) object that has a color based on activeFocus and enabled, so UCLabel just needs to listen to colorChanged?
This would kind of enforce consistent use of the palette, and even states which UCLabel doesn't currently implement, but I think that would be a good thing.

review: Needs Information

« Back to merge proposal