Fixed
Status Update
Comments
ap...@google.com <ap...@google.com> #2
Project: platform/frameworks/support
Branch: androidx-master-dev
commit 47ac9fee0e3d43b4da33f53b8a2ff590f0d46079
Author: Jakub Gielzak <jgielzak@google.com>
Date: Tue Aug 20 13:57:06 2019
Addressed issues with EditText causing scroll
EditText methods: bringPointIntoView, and handleFocusGainInternal
trigger requestChildRectangleOnScreen in ViewPager2's internal
RecyclerView.
This can cause:
- unwanted scrolling -- e.g. when typing on some API versions
- a jump to page 0 -- EditText sometimes reports over -1M px mScrollX
This fix bypasses requestChildRectangleOnScreen calculations taking
advantage of the fact that PagerSnapHelper constraints valid scroll
values to a snapped position ones, and as a result, the problem is
reduced to choosing the correct currentItem.
Bug: 138044582
Bug: 139432498
Test: ./gradlew viewpager2:connectedCheck
Change-Id: Ia4d3e4e6734183e64f261c7cc76d669deae78da5
A viewpager2/src/androidTest/java/androidx/viewpager2/widget/EditTextFocusTest.kt
M viewpager2/src/main/java/androidx/viewpager2/widget/ViewPager2.java
https://android-review.googlesource.com/1107013
https://goto.google.com/android-sha1/47ac9fee0e3d43b4da33f53b8a2ff590f0d46079
Branch: androidx-master-dev
commit 47ac9fee0e3d43b4da33f53b8a2ff590f0d46079
Author: Jakub Gielzak <jgielzak@google.com>
Date: Tue Aug 20 13:57:06 2019
Addressed issues with EditText causing scroll
EditText methods: bringPointIntoView, and handleFocusGainInternal
trigger requestChildRectangleOnScreen in ViewPager2's internal
RecyclerView.
This can cause:
- unwanted scrolling -- e.g. when typing on some API versions
- a jump to page 0 -- EditText sometimes reports over -1M px mScrollX
This fix bypasses requestChildRectangleOnScreen calculations taking
advantage of the fact that PagerSnapHelper constraints valid scroll
values to a snapped position ones, and as a result, the problem is
reduced to choosing the correct currentItem.
Bug: 138044582
Bug: 139432498
Test: ./gradlew viewpager2:connectedCheck
Change-Id: Ia4d3e4e6734183e64f261c7cc76d669deae78da5
A viewpager2/src/androidTest/java/androidx/viewpager2/widget/EditTextFocusTest.kt
M viewpager2/src/main/java/androidx/viewpager2/widget/ViewPager2.java
Description
I think we have two issues with misleading behaviour in android.transition.TransitionSet class.
1) setInterpolator of TransitionSet would not propagate an interpolator into the children transitions
Current implementation:
public TransitionSet setInterpolator(TimeInterpolator interpolator) {
return (TransitionSet) super.setInterpolator(interpolator);
}
How it should be (in the same style as setDuration method):
public TransitionSet setInterpolator(TimeInterpolator interpolator) {
super.setInterpolator(interpolator);
if (mTransitions != null) {
int numTransitions = mTransitions.size();
for (int i = 0; i < numTransitions; ++i) {
mTransitions.get(i).setInterpolator(mInterpolator);
}
}
return this;
}
And then in addTransition we should propagate current interpolator, not only duration
public TransitionSet addTransition(Transition transition) {
if (transition != null) {
mTransitions.add(transition);
transition.mParent = this;
if (mDuration >= 0) {
transition.setDuration(mDuration);
}
if (mInterpolator != null) {
transition.setInterpolator(mInterpolator);
}
}
return this;
}
2) Imagine situation when you have a set:
TransitionSet set = new TransitionSet().setDuration(300);
Then you add a lot of transitions into it (that why you want to specify duration not one by one but only once for the set via .setDuration(300)).
And then you also want to add one more transition but with a smaller duration:
Fade fadeIn = new Fade(Fade.IN);
set.addTransition(fadeIn);
fadeIn.setDuration(100);
But it not works, fadeIn is animating with duration 300.
Its a real usecase, sometimes we need a set with for example a lot of ChangeBounds with the same duration and one visibility in animation with a smaller duration(and one visibility out animation with smaller duration)
The reason of it is a clone method:
public TransitionSet clone() {
TransitionSet clone = (TransitionSet) super.clone();
clone.mTransitions = new ArrayList<Transition>();
int numTransitions = mTransitions.size();
for (int i = 0; i < numTransitions; ++i) {
clone.addTransition((Transition) mTransitions.get(i).clone());
}
return clone;
}
Here we call addTransition where we would override the duration of the transition again.
To fix it I suggest to change the line
clone.addTransition((Transition) mTransitions.get(i).clone());
to:
Transition transition = (Transition) mTransitions.get(i).clone();
clone.mTransitions.add(transition);
transition.mParent = clone;
Thanks,
Both issues were found during the development of transitions backport library Transitions-Everywhere