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

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

> > 2526 + Component { id: customStyle; ListItemStyle {} }
> > 2527 +
> > 2528 + function test_style_reset() {
> > 2529 + testItem.style = customStyle;
> > 2530 + testItem.style = undefined;
> > 2531 + verify(testItem.style != 0, "Style set back to theme")
> > 2532 + }
> >
> > ; missing
> >
> > You only verify here that the style is not 0. Can we get the type of the
> style
> > and validate that?
>
> Fixed in revno 1402.

All changes are good, except this one. What I meant is to check the actual type of the component, but that can be difficult. The approach with the objectName can work, but I propose to use a more descriptive objectName ("ListItemThemeStyle"?), and to add a comment in uclistitem.cpp when you set the objectName, saying that you use it in tst_listitem.qml because from reading only the cpp now it is not clear why this code is there. Perhaps even comment in tst_listitem.qml that the objectName is set in UCListItemPrivate.resetStyle().

« Back to merge proposal