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

W dniu 17.05.2013 20:37, Daniel d'Andrada pisze:
>> I wonder if DirectionalDragArea::state (or status to avoid confusion) should
>> be exposed to QML? Or maybe a "dragging" boolean?
>
> Yeah, we could expose the state variable indeed.
> Done.

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?

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

>> 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".

>> 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".

--
Michał Sawicz <email address hidden>
Canonical Services Ltd.