Verified
Status Update
No update yet.
Comments
cc...@google.com
Project: platform/frameworks/support
Branch: androidx-main
Author: George Mount <
Link:
Fix focus move within child AndroidViews
Expand for full commit details
Fix focus move within child AndroidViews
Fixes: 389994198
Fixes: 391378895
Relnote: "Fixes focus travel within child AndroidViews where it
can sometimes jump out of the child to the ComposeView"
Sometimes the native FocusFinder doesn't act properly and finds
the wrong View when searching. This CL changes the FocusFinder to
use a newer copy of FocusFinder for all focus search operations
in AndroidComposeView.
Test: new test, manual testing
Change-Id: I678c864afe086879c5c45b018dd82da4f23be440
Files:
- M
compose/ui/ui/src/androidInstrumentedTest/AndroidManifest.xml - M
compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/focus/FocusListenerTest.kt - A
compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/test/TestActivity2.kt - A
compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/viewinterop/MixedFocusChangeTest.kt - M
compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/AndroidComposeView.android.kt - M
compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/FocusFinderCompat.android.kt
Hash: b028bcc322095634123da91c7146bac9e23c13c7
Date: Tue Jan 14 13:48:11 2025
tw...@googlemail.com
Project: platform/frameworks/support
Branch: androidx-main
Author: Ralston Da Silva <
Link:
Disabling isViewFocusFixEnabled and isRemoveFocusedViewFixEnabled
Expand for full commit details
Disabling isViewFocusFixEnabled and isRemoveFocusedViewFixEnabled
Disabling these flag to unblock 1.8 RC.
These were features/bug-fixes that were not regressions
from 1.7
Bug: 406327273
Bug: 369256395
Bug: 378570682
Bug: 376142752
Bug: 384056227
Bug: 388590015
Bug: 389994198
Bug: 391378895
Test: Ran presubmits and added TODOs to the affected tests
Change-Id: I5ffb7ff27c662838c8b464560d1df830751f015c
Files:
- M
compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/focus/FocusViewInteropTest.kt - M
compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/focus/OwnerFocusTest.kt - M
compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/focus/RequestFocusTest.kt - M
compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/viewinterop/MixedFocusChangeTest.kt - M
compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/ComposeUiFlags.kt
Hash: 1a33d21734ef7f68d4cad37c8d333831f304a339
Date: Tue Apr 01 17:46:50 2025
tw...@googlemail.com
Did the sample help you in any way to verify that the problem exists?
Cheers,
Thomas
Cheers,
Thomas
cc...@google.com
Yes, thank you, the sample clarified the problem,
The problem happens when a new PagedList with a prepend is received by the UI, but the items in the viewport aren’t updated. The stream of PagedLists keeps a guess of what key to use across PagedList generations based on last load position. However, if nothing in the viewport is updated, PagedListAdapter#getItem() isn't called. This means we don’t update that last load position to account for prepended items. This only affects PositionalDataSource, since the old fetch position becomes stale, whereas an item key would not.
Fix will be to update this key when a new PagedList arrives by triggering a fetch.
In the meantime, here's an adapter-side hack to workaround the problem. Add the below to CheeseAdapter.kt, and the sample works fine:
var rv : RecyclerView? = null
override fun onAttachedToRecyclerView(recyclerView: RecyclerView) {
rv = recyclerView // stash ref to RV, so we can get layout manager ref
}
override fun onDetachedFromRecyclerView(recyclerView: RecyclerView) {
rv = null
}
override fun onCurrentListChanged(currentList: PagedList<Cheese>?) {
if (currentList != null && currentList.isNotEmpty()) {
// have to post so findFirstVisibleItemPosition() returns position relative to currentList
rv?.post {
if (getCurrentList() == currentList) {
val visPos = (rv?.layoutManager as LinearLayoutManager)
.findFirstVisibleItemPosition()
val offset = currentList.positionOffset
// loadAround will update the key in the currentList,
// which will be used for next reload from DB
currentList.loadAround(visPos + offset)
}
}
}
}
The problem happens when a new PagedList with a prepend is received by the UI, but the items in the viewport aren’t updated. The stream of PagedLists keeps a guess of what key to use across PagedList generations based on last load position. However, if nothing in the viewport is updated, PagedListAdapter#getItem() isn't called. This means we don’t update that last load position to account for prepended items. This only affects PositionalDataSource, since the old fetch position becomes stale, whereas an item key would not.
Fix will be to update this key when a new PagedList arrives by triggering a fetch.
In the meantime, here's an adapter-side hack to workaround the problem. Add the below to CheeseAdapter.kt, and the sample works fine:
var rv : RecyclerView? = null
override fun onAttachedToRecyclerView(recyclerView: RecyclerView) {
rv = recyclerView // stash ref to RV, so we can get layout manager ref
}
override fun onDetachedFromRecyclerView(recyclerView: RecyclerView) {
rv = null
}
override fun onCurrentListChanged(currentList: PagedList<Cheese>?) {
if (currentList != null && currentList.isNotEmpty()) {
// have to post so findFirstVisibleItemPosition() returns position relative to currentList
rv?.post {
if (getCurrentList() == currentList) {
val visPos = (rv?.layoutManager as LinearLayoutManager)
.findFirstVisibleItemPosition()
val offset = currentList.positionOffset
// loadAround will update the key in the currentList,
// which will be used for next reload from DB
currentList.loadAround(visPos + offset)
}
}
}
}
tw...@googlemail.com
Hi,
thank you for your explanation. It total makes sense.I tried your suggested workaround, however, the list still doesn't keep the correct state. When scrolling a bit around and then pre-pending items, I still get different items.
Cheers,
Thomas
thank you for your explanation. It total makes sense.I tried your suggested workaround, however, the list still doesn't keep the correct state. When scrolling a bit around and then pre-pending items, I still get different items.
Cheers,
Thomas
tw...@googlemail.com
Do you have another suggestions how to workaround the problem?
Cheers,
Thomas
Cheers,
Thomas
cc...@google.com
Experimented more with the sample, was able to get this issue to reproduce:
1) Clear DB
2) Batch prepend x2
3) Scroll to top of loaded content
4) Batch prepend x5
The rest of this issue is as you described in your initial report - the initial load position doesn't account for prepend offsetting list content. (There's also the additional factor of centering at an arbitrary position in the viewport, not the center.)
Both of these make the initial load guess less precise. If any viewport content is outside the bounds of that initial load, those items will disappear until you scroll back to them.
Ideally, the initial load is large enough to account for this. Setting pageSize = 50, or initialLoadSize = 150 make the problem go away, and really the sample should have had a bigger page size like this from the beginning. 20 is only trivially bigger than what's in viewport on a mid-size phone, and we generally recommend around 3 viewports of content as a page size minimum.
The larger of removals/inserts you do with positional data, the larger the initial load needs to be to avoid this disappearing problem. In this case, it's just raising it to a reasonable value.
1) Clear DB
2) Batch prepend x2
3) Scroll to top of loaded content
4) Batch prepend x5
The rest of this issue is as you described in your initial report - the initial load position doesn't account for prepend offsetting list content. (There's also the additional factor of centering at an arbitrary position in the viewport, not the center.)
Both of these make the initial load guess less precise. If any viewport content is outside the bounds of that initial load, those items will disappear until you scroll back to them.
Ideally, the initial load is large enough to account for this. Setting pageSize = 50, or initialLoadSize = 150 make the problem go away, and really the sample should have had a bigger page size like this from the beginning. 20 is only trivially bigger than what's in viewport on a mid-size phone, and we generally recommend around 3 viewports of content as a page size minimum.
The larger of removals/inserts you do with positional data, the larger the initial load needs to be to avoid this disappearing problem. In this case, it's just raising it to a reasonable value.
cc...@google.com
Fix to incorrect initial fetch position in #5 merged, will go out with next AndroidX paging release.
cc...@google.com
Released with Paging 2.0.0-beta01 (Currently revision is 2.0.0 stable)
Description
Version used:all versions
Devices/Android versions reproduced on: all versions
Hi,
thanks for the great work.
I use a slightly modified version of the PagingSample. Instead of just adding one item when tapping on the "Add" button, I add 20 copies of the same item:
fun insert(text: CharSequence) = ioThread {
dao.insert(listOf(
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString()),
Cheese(id = 0, name = text.toString())))
}
When adding items which will be pre-pended to the list, the PositionalDataSource fails to re-load the visible items. This is a general problem.
My guess:
The PositionalDatasource doesn't know if newly inserted items will be pre-pended by the query or appended. Let's say we have the following scenario:
- initially loading items from position 20
- pre-pending 50 new items
- results in datasource invalidation and new initial load from position 20
- now there are completely different items at position 20
- async diff sees changes and triggers animations
Somehow this happens to be related to the page size used. When increasing the page size, it happens after more inserts. When decreasing the page size, this behavior happen after just the first inserts.
My question is:
How to handle pre-pending inserts with the PositionalDataSource?