Code review comment for lp:~nick-dedekind/unity8/lp1283191

Revision history for this message
Andrea Cimitan (cimi) wrote :

> > 24 + onServerValueChanged: {
> > 25 + // value can be changed by slider, so a binding
> won't
> > work.
> > 26 + value = serverValue;
> > 27 + }
> >
> > I'd probably do like
> > if (menuData && menuData.actionState) value = serverValue;
> >
> > or property var serverValue: menuData && menuData.actionState || -1
> > and then if (serverValue > -1) value = serverValue;
> >
> > (pick up the one that works with the service)
> >
> > so only if the serverValue is a valid value, otherwise I'd keep the value in
> > value (imagine this service that crashes and reboots, I want the value to be
> > preserved, not being set to 0.0), what you think?
> >
> > Rest is fine
>
> Doesn't really matter in this case, as if the service restarts, the whole menu
> will be reinitialised.
> I've changed it to be undefined.

Even better with undefined.
At this point you might add a line here or at the end of the test

96 + slider.value = data.manualValue;
97 + compare(loader.item.value, data.manualValue, "Slider value does not match manual set value");

that unsets actionState or the whole menuData and checks that the previous value is kept

« Back to merge proposal