Normalized android touch events #8064
Conversation
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
1 similar comment
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
|
|
||
| int maskedAction = event.getActionMasked(); | ||
| int pointerChange = getPointerChangeForAction(event.getActionMasked()); | ||
| List<PointerAction> requiredActions = new ArrayList<PointerAction>(); |
There was a problem hiding this comment.
nit: I'd rename this. It's unclear what they are required for. Maybe just actions, or actionsForFramework.
|
|
||
| // ACTION_HOVER_MOVE always applies to a single pointer only. | ||
| addPointerForIndex(event, event.getActionIndex(), pointerChange, 0, packet); | ||
| List<PointerAction> requiredActions = expandActions(event, event.getActionIndex(), pointerChange, 0); |
| pressed = true; | ||
| } | ||
| } | ||
| private class PointerAction { |
There was a problem hiding this comment.
nit: add blank line above this line.
| private long device; | ||
| private double dx; | ||
| private double dy; | ||
| private boolean pressed; |
There was a problem hiding this comment.
call this "down" since that what it is?
| this.dy = dy; | ||
| } | ||
|
|
||
| public boolean isDown() { |
There was a problem hiding this comment.
should pressed just be non-private instead of having this getter and the two setter?
| state.setLastLocation(dx, dy); | ||
| break; | ||
| default: | ||
| // unknown action |
There was a problem hiding this comment.
This should probably throw or assert?
| } | ||
|
|
||
| private void addToPacket(MotionEvent event, int pointerIndex, int pointerChange, | ||
| int pointerData, ByteBuffer packet) { |
There was a problem hiding this comment.
Please fix the indentation on this line.
| } | ||
|
|
||
| private void addPointerForIndex(MotionEvent event, int pointerIndex, int pointerChange, | ||
| private List<PointerAction> expandActions(MotionEvent event, int pointerIndex, int pointerChange, |
There was a problem hiding this comment.
If you factor out expandActions into its own class (just like it was in the framework in converter.dart) - could you write simple unit tests then where you pass in face MotionEvents and assert that you get the expected List of PointerActions out?
| private final int actionType; | ||
| private final int batched; | ||
|
|
||
| public PointerAction(int pointerIndex, int actionType, int batched) { |
There was a problem hiding this comment.
I'd maybe call this FrameworkPointerEvent since this is going to be something the framework understands?
| return requiredAction; | ||
| } | ||
|
|
||
| private void addToPacket(MotionEvent event, int pointerIndex, int pointerChange, |
There was a problem hiding this comment.
Could this be a method on PainterAction that gets passed the packet and adds itself to it?
|
Same question as with the iOS side of this change: after discussion with @chinmaygarde is the plan to continue with this PR as-is, or should we implement the subset not implemented in the core engine state machine, or close? |
|
Closed after talking to @chunhtai |
This PR is part of the change flutter/flutter#28972