Code review comment for lp:~dandrader/unity/phablet_edgeDragGesture

> 818 + // The current state of the directional drag gesture.
> 819 + Q_PROPERTY(GestureState gestureState READ gestureState NOTIFY
> gestureStateChanged)
>
> Could we go with "status" to be consistent with other QML components?

Done.

>
> And I would still like a boolean "dragging" more in favor of dragEnded,
> dragStarted and dragRecognized signals.

Done.

>
> >> Shouldn't dragValue be reset to 0 when going back to WaitingForTouch? Or
> maybe
> >> that's a higher level's responsibility to make sure of that...
> >
> > No, I think it should stay as it is. It's up to the application to decide
> what to do once the gesture ends.
>
> 731 + default: // Rejected
> 732 + Q_EMIT dragRejected();
> 733 + Q_EMIT distanceChanged(distance());
>
> Shouldn't that not emit the distance then? I'm just worried that people
> might rely on it to cancel the movement, but then the dragRejected
> signal (or draggingChanged(false) when you introduce that property)
> should already be enough to say "the gesture is uninteresting, do what
> you need to cancel".

For the sake of consistency with the transition to WaitingForTouch, I made it keep its last valid value while on Rejected state.

>
> >> 1070 + LD_LIBRARY_PATH=${CMAKE_BINARY_DIR}/plugins/Ubuntu/Gestures
> >>
> >> This should be set as an ENVIRONMENT property on the test instead.
> >
> > Why? So you want CMake to pass it as a DEFINE to tst_DirectionalDragArea.cpp
> during compilation?
>
> No, I mean it should be possible to use set_target_properties() to pass
> that environment var to the target, just as we do in add_qml_test() to
> set the QPA platform to "minimal".

The test for DirectionalDragArea is a custom target (using add_custom_target()) not a proper cmake test (using add_test()). A custom target doesn't have (or use) an ENVIRONMENT property.