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

Revision history for this message
Tim Peeters (tpeeters) wrote :

> > void UCListItem::setHighlightColor(const QColor &color)
> > {
> > Q_D(UCListItem);
> > if (d->highlightColor == color) {
> > return;
> > }
> > d->highlightColor = color;
> > // no more theme change watch
> > d->customColor = true;
> > update();
> > Q_EMIT highlightColorChanged();
> > }
> > void UCListItem::resetHighlightColor()
> > {
> > Q_D(UCListItem);
> > d->customColor = false;
> > d->highlightColor = getPaletteColor("selected", "background");
> > update();
> > Q_EMIT highlightColorChanged();
> > }
> >
> >
> > if you call setHighlightColor(color) // color == getPaletteColor("selected",
> > "background") then customColor is not set to true, and you are still
> watching
> > for palette updates, while it may be purely coincidence that you set the
> color
> > to the palette color. So customColor = true should be moved before the if()
> > (maybe update() too).
>
> Well, if a property's default value is set again as value, there will be no
> change done either.

But the link to the theme color should be broken, when you set a manual color (same as the theme color), the color should not be updated automatically any more when the theme color changes.

« Back to merge proposal