Commit 7edc13a5 authored by Ryan Landay's avatar Ryan Landay Committed by Commit Bot

Clean up some redundant logic in the Android tab picker

In StackLayout, which implements the Android tab picker, we currently have two
places that implement essentially the same piece of logic for deciding whether
a drag event should be interpreted as switching between the two stacks, or
closing a tab in the current stack. Besides the redundancy making the code
unnecessarily confusing, there are three other problems:

- The inline logic in computeInputMode() computes switchDelta using x + dx or
  y + dy, but isDraggingStackInWrongDirection() uses x or y without the delta
  term. This introduces a small amount of judder when dragging a tab back and
  forth between the "swipe to close" and "swipe to change stacks" directions.

- isDraggingStackInWrongDirection() always prohibits dragging if the horizontal
  movement is smaller than the vertical movement...even in landscape mode. This
  means swiping between stacks barely works in landscape mode (it only works if
  you put your finger on the other tab to start; this case works because it
  hits the "currentIndex != getViewportParameters().getStackIndexAt(x, y)"
  logic earlier in the method.

- isDraggingStackInWrongDirection() is broken for RTL mode.

This CL makes the code easier to follow and fixes these three problems.

Change-Id: Idb507b75f2977859c561a93d13c7737dfd84b97a
Reviewed-on: https://chromium-review.googlesource.com/935450Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Commit-Queue: Ryan Landay <rlanday@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539224}
parent d3fbbb41
......@@ -797,12 +797,11 @@ public class StackLayout extends Layout implements Animatable<StackLayout.Proper
^ (getOrientation() == Orientation.PORTRAIT
&& LocalizationUtils.isLayoutRtl())) {
return SwipeMode.SEND_TO_STACK;
} else {
return SwipeMode.SWITCH_STACK;
}
}
if (isDraggingStackInWrongDirection(
mLastOnDownX, mLastOnDownY, x, y, dx, dy, getOrientation(), currentIndex)) {
return SwipeMode.SWITCH_STACK;
}
// Not moving the finger
if (time - mLastOnDownTimeStamp > THRESHOLD_TIME_TO_SWITCH_STACK_INPUT_MODE) {
return SwipeMode.SEND_TO_STACK;
......@@ -946,31 +945,6 @@ public class StackLayout extends Layout implements Animatable<StackLayout.Proper
}
}
/**
* Check if we are dragging stack in a wrong direction.
*
* @param downX The X coordinate on the last down event.
* @param downY The Y coordinate on the last down event.
* @param x The current X coordinate.
* @param y The current Y coordinate.
* @param dx The amount of change in X coordinate.
* @param dy The amount of change in Y coordinate.
* @param orientation The device orientation (portrait / landscape).
* @param stackIndex The index of stack tab.
* @return True iff we are dragging stack in a wrong direction.
*/
@VisibleForTesting
public static boolean isDraggingStackInWrongDirection(float downX, float downY, float x,
float y, float dx, float dy, int orientation, int stackIndex) {
float switchDelta = orientation == Orientation.PORTRAIT ? x - downX : y - downY;
// Should not prevent scrolling even when switchDelta is in a wrong direction.
if (Math.abs(dx) < Math.abs(dy)) {
return false;
}
return (stackIndex == 0 && switchDelta < 0) || (stackIndex == 1 && switchDelta > 0);
}
private void scrollStacks(float delta) {
cancelAnimation(this, Property.STACK_SNAP);
float fullDistance = getFullScrollDistance();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment