Code review comment for lp:~zsombi/ubuntu-ui-toolkit/33-listitem-attached

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

> 540 + if ((d->globalDisabled && disable) || (!d->globalDisabled &&
> disable)) {
> 541 + // disabling or re-disabling
> 542 + d->disablerItem = item;
> 543 + if (d->globalDisabled == disable) {
> 544 + // was already disabled, leave
> 545 + return;
> 546 + }
> 547 + d->globalDisabled = disable;
> 548 + } else if (d->globalDisabled && !disable && d->disablerItem ==
> item) {
> 549 + // the one disabled it will enable
> 550 + d->globalDisabled = disable;
> 551 + d->disablerItem.clear();
> 552 + } else {
> 553 + // none of the above, leave
> 554 + return;
> 555 + }
>
> same as code below, which is simpler:
>
> if (disable) {
> d->disablerItem = item; // what is this for?
> if (d->globalDisabled) {
> return;
> }
> } else if (d->globalDisabled && item == d->disablerItem) {
> d->globalDisabled = false;
> d->disablerItem.clear();
> } else {
> // !disabled && (!globalDisabled || item != d->disablerItem)
> return;
> }

Right, changed it.
The disablerItem is there to know that we should re-enable the interactive flag only if the same item which disabled requests it. Example, you have a Flickable with ListItems, all having their own ListItemAction declared (not a shared one!!), one ListItem swiped in. You start to swipe an other one, in that moment the previous one will initiate rebounding, and the one you swiped will lock the Flickable and start swiping. At this point this will be the one who requested the lock. The other one finishes the rebound animation, and as part of the animation end, the lock is released. But that should not happen as you may still be dragging the other ListItem, right? So the member makes sure that we only unlock the Flickable when the one locking it requests it.

« Back to merge proposal