Fixed
Status Update
Comments
kl...@google.com <kl...@google.com>
ap...@google.com <ap...@google.com> #2
Project: platform/frameworks/support
Branch: androidx-main
commit b5b29bbc4bef0f1a172742809f0cc00c7ec48391
Author: Zach Klippenstein <klippenstein@google.com>
Date: Fri Aug 05 14:10:46 2022
Prepare BringIntoView internals to be able to handle changing requests.
This change modifies the signature of internal bringIntoView-related
calls to take a function returning a Rect instead of a single Rect, and
updates the bringIntoView modifier to calculate the default size of the
request lazily when this function is called, which is required to
correctly animate nodes that change size during the animation
( b/241591211 ).
This change does NOT actually fix the implementation in Scrollable to
correctly handle changing bounds, that will be done in a follow-up.
It also does NOT change the main public API for bringIntoView, the
BringIntoViewRequester.bringIntoView function – that still takes a
single Rect.
Bug: b/241591211
Test: ./gradlew :compose:f:f:cDAT
Relnote: "`BringIntoViewResponder`s are now able to get the most
up-to-date bounds of a request while processing it."
Change-Id: If86a581463c14db3fa5638720aabca9830720630
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/relocation/FakeScrollable.kt
M compose/foundation/foundation/samples/src/main/java/androidx/compose/foundation/samples/BringIntoViewSamples.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/ContentInViewModifier.kt
M tv/tv-foundation/src/main/java/androidx/tv/foundation/ScrollableWithPivot.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/FocusableTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/relocation/BringIntoView.kt
M compose/foundation/foundation/api/public_plus_experimental_current.txt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/relocation/BringIntoViewRequesterViewIntegrationTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/relocation/BringIntoViewRequester.kt
M compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/relocation/BringIntoViewResponder.android.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/relocation/BringIntoViewResponderTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/relocation/BringIntoViewScrollableInteractionTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/relocation/BringIntoViewResponder.kt
M compose/foundation/foundation/api/public_plus_experimental_1.3.0-beta03.txt
https://android-review.googlesource.com/2175615
Branch: androidx-main
commit b5b29bbc4bef0f1a172742809f0cc00c7ec48391
Author: Zach Klippenstein <klippenstein@google.com>
Date: Fri Aug 05 14:10:46 2022
Prepare BringIntoView internals to be able to handle changing requests.
This change modifies the signature of internal bringIntoView-related
calls to take a function returning a Rect instead of a single Rect, and
updates the bringIntoView modifier to calculate the default size of the
request lazily when this function is called, which is required to
correctly animate nodes that change size during the animation
(
This change does NOT actually fix the implementation in Scrollable to
correctly handle changing bounds, that will be done in a follow-up.
It also does NOT change the main public API for bringIntoView, the
BringIntoViewRequester.bringIntoView function – that still takes a
single Rect.
Bug:
Test: ./gradlew :compose:f:f:cDAT
Relnote: "`BringIntoViewResponder`s are now able to get the most
up-to-date bounds of a request while processing it."
Change-Id: If86a581463c14db3fa5638720aabca9830720630
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/relocation/FakeScrollable.kt
M compose/foundation/foundation/samples/src/main/java/androidx/compose/foundation/samples/BringIntoViewSamples.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/ContentInViewModifier.kt
M tv/tv-foundation/src/main/java/androidx/tv/foundation/ScrollableWithPivot.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/FocusableTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/relocation/BringIntoView.kt
M compose/foundation/foundation/api/public_plus_experimental_current.txt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/relocation/BringIntoViewRequesterViewIntegrationTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/relocation/BringIntoViewRequester.kt
M compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/relocation/BringIntoViewResponder.android.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/relocation/BringIntoViewResponderTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/relocation/BringIntoViewScrollableInteractionTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/relocation/BringIntoViewResponder.kt
M compose/foundation/foundation/api/public_plus_experimental_1.3.0-beta03.txt
na...@google.com <na...@google.com> #3
This bug was linked in a change in the following release(s):
androidx.compose.foundation:foundation:1.3.0-beta03
ap...@google.com <ap...@google.com> #4
Project: platform/frameworks/support
Branch: androidx-main
commit 906b235af229dec2f2e163ee309d5a82426be08d
Author: Zach Klippenstein <klippenstein@google.com>
Date: Thu Sep 22 09:18:37 2022
Merge some almost-redundant scrollable + focusable tests.
Also makes both sets of tests parameterized on both orientation and
scrolling direction.
Bug: b/241591211
Test: ScrollableFocusableInteractionTest
Change-Id: I487344ff1705c5875bd60c25c5b7f456e0b7e65d
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ScrollableFocusableInteractionTest.kt
D compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/focus/FocusInScrollableRowTest.kt
https://android-review.googlesource.com/2227761
Branch: androidx-main
commit 906b235af229dec2f2e163ee309d5a82426be08d
Author: Zach Klippenstein <klippenstein@google.com>
Date: Thu Sep 22 09:18:37 2022
Merge some almost-redundant scrollable + focusable tests.
Also makes both sets of tests parameterized on both orientation and
scrolling direction.
Bug:
Test: ScrollableFocusableInteractionTest
Change-Id: I487344ff1705c5875bd60c25c5b7f456e0b7e65d
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ScrollableFocusableInteractionTest.kt
D compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/focus/FocusInScrollableRowTest.kt
na...@google.com <na...@google.com> #5
This bug was linked in a change in the following release(s):
androidx.compose.foundation:foundation:1.3.0-rc01
androidx.compose.ui:ui:1.3.0-rc01
ap...@google.com <ap...@google.com> #6
Project: platform/frameworks/support
Branch: androidx-main
commit 7e77bb541331ce0d62cabad732e43eca41ac978a
Author: Zach Klippenstein <klippenstein@google.com>
Date: Wed Jul 06 11:42:21 2022
Move BringIntoView queueing into the scrollable implementation.
This change removes the over-complicated coroutine-based queuing of
bringIntoView requests in the BringIntoViewResponder, which now is only
responsible for dispatching the request to the actual responder
interface and propagating it up the modifier local chain.
This gives the actual responder implementation (i.e. in
Modifier.scrollable) all the information about conflicting
requests and manage the queue of ongoing requests with an actual,
explicit queue that is much more straightforward than the previous
coroutine job chaining mess. Each request is stored in an actual list
along with the continuation from the request, and requests are
explicitly resumed or cancelled when needed. This also makes debugging
easier, because there's an actual queue data structure we can inspect,
which contains all the actual information about each request.
It also completely changes how the scroll animation is performed.
On every frame, the scroll target is recomputed to account for
changes in the viewport size, the request size, or the set of active
requests since the last frame, while still keeping the animation
smooth. The animation logic is in its own class, which allows a single
suspend animation call to animate to a target value that can be updated
while the animation is running.
While this change is a valuable cleanup on its own, it also fixes
b/241591211 , and paves the way for replacing scrollable's implicit
focused child tracking with an explicit API for keeping a thing in view.
Fixes: b/241591211
Bug: b/192043120
Bug: b/237190748
Bug: b/230756508
Bug: b/239451114
Test: ScrollableFocusableInteractionTest
Test: androidx.compose.foundation.relocation.*
Test: RebasableAnimationStateTest
Test: BringIntoViewRequestPriorityQueueTest
Relnote: "Rewrote the way scrollables respond to
`bringIntoViewRequesters` and focusables to better model the
complexity of those operations and handle more edge cases."
Change-Id: I2e5fec8c8582a8fe1f191e37fd0f4f9165678664
M compose/foundation/foundation/api/current.txt
M compose/foundation/foundation/api/public_plus_experimental_current.txt
M compose/foundation/foundation/api/restricted_current.txt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/FoundationDemos.kt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/ScrollableFocusedChildDemo.kt
A compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/relocation/BringIntoViewResponderDemo.kt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/relocation/BringRectangleIntoViewDemo.kt
M compose/foundation/foundation/samples/src/main/java/androidx/compose/foundation/samples/BringIntoViewSamples.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ScrollableFocusableInteractionTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListFocusMoveTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/relocation/BringIntoViewResponderTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/relocation/BringIntoViewScrollableInteractionTest.kt
A compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/BringIntoViewRequestPriorityQueue.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/ContentInViewModifier.kt
A compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/UpdatableAnimationState.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/relocation/BringIntoViewResponder.kt
A compose/foundation/foundation/src/test/kotlin/androidx/compose/foundation/gestures/BringIntoViewRequestPriorityQueueTest.kt
A compose/foundation/foundation/src/test/kotlin/androidx/compose/foundation/gestures/UpdatableAnimationStateTest.kt
https://android-review.googlesource.com/2178020
Branch: androidx-main
commit 7e77bb541331ce0d62cabad732e43eca41ac978a
Author: Zach Klippenstein <klippenstein@google.com>
Date: Wed Jul 06 11:42:21 2022
Move BringIntoView queueing into the scrollable implementation.
This change removes the over-complicated coroutine-based queuing of
bringIntoView requests in the BringIntoViewResponder, which now is only
responsible for dispatching the request to the actual responder
interface and propagating it up the modifier local chain.
This gives the actual responder implementation (i.e. in
Modifier.scrollable) all the information about conflicting
requests and manage the queue of ongoing requests with an actual,
explicit queue that is much more straightforward than the previous
coroutine job chaining mess. Each request is stored in an actual list
along with the continuation from the request, and requests are
explicitly resumed or cancelled when needed. This also makes debugging
easier, because there's an actual queue data structure we can inspect,
which contains all the actual information about each request.
It also completely changes how the scroll animation is performed.
On every frame, the scroll target is recomputed to account for
changes in the viewport size, the request size, or the set of active
requests since the last frame, while still keeping the animation
smooth. The animation logic is in its own class, which allows a single
suspend animation call to animate to a target value that can be updated
while the animation is running.
While this change is a valuable cleanup on its own, it also fixes
focused child tracking with an explicit API for keeping a thing in view.
Fixes:
Bug:
Bug:
Bug:
Bug:
Test: ScrollableFocusableInteractionTest
Test: androidx.compose.foundation.relocation.*
Test: RebasableAnimationStateTest
Test: BringIntoViewRequestPriorityQueueTest
Relnote: "Rewrote the way scrollables respond to
`bringIntoViewRequesters` and focusables to better model the
complexity of those operations and handle more edge cases."
Change-Id: I2e5fec8c8582a8fe1f191e37fd0f4f9165678664
M compose/foundation/foundation/api/current.txt
M compose/foundation/foundation/api/public_plus_experimental_current.txt
M compose/foundation/foundation/api/restricted_current.txt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/FoundationDemos.kt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/ScrollableFocusedChildDemo.kt
A compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/relocation/BringIntoViewResponderDemo.kt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/relocation/BringRectangleIntoViewDemo.kt
M compose/foundation/foundation/samples/src/main/java/androidx/compose/foundation/samples/BringIntoViewSamples.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ScrollableFocusableInteractionTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListFocusMoveTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/relocation/BringIntoViewResponderTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/relocation/BringIntoViewScrollableInteractionTest.kt
A compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/BringIntoViewRequestPriorityQueue.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/ContentInViewModifier.kt
A compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/UpdatableAnimationState.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/relocation/BringIntoViewResponder.kt
A compose/foundation/foundation/src/test/kotlin/androidx/compose/foundation/gestures/BringIntoViewRequestPriorityQueueTest.kt
A compose/foundation/foundation/src/test/kotlin/androidx/compose/foundation/gestures/UpdatableAnimationStateTest.kt
na...@google.com <na...@google.com> #7
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.compose.foundation:foundation:1.4.0-alpha04
Description
If the node making the request shrinks during the animation, it will be scrolled too far. If it grows, it won't be scrolled far enough. It also uses the wrong coordinates if the position of the node changes for some reason other than the BIV animation itself.