Status Update
Comments
ma...@google.com <ma...@google.com>
jo...@veeva.com <jo...@veeva.com> #2
+1 to all of this. It looks like PullToRefreshStateImpl
has the facility to satisfy number 1, but instead of calling animateTo(0f)
, it just sets the verticalOffset directly to 0 which causes the indicator to just disappear off the screen instead of the nice animation we had in M2. If PulltoRefreshStateImpl wasn't internal we could extend it and override this behavior.
I've implemented workaround for the endRefresh()
animation in case it's helpful for anyone else:
I added a graphicsLayer(translationY = translationFraction)
modifier to the PullToRefreshContainer where translationFraction is an animated State that I set before calling endRefresh()
. This animated state looks like val translationFraction by animateFloatAsState(if(isEndingRefresh) pullToRefreshState.positionalThreshold.times(-1) else 0f
and then I set isEndingRefresh (a simple MutableState<Boolean>) and delay a bit before calling endRefresh()
. Not ideal, but it works.
ar...@physitrack.com <ar...@physitrack.com> #3
+1 especially to #4. Implementing simple pull to refresh mechanism that observes refreshing state from e.g. ViewModel
and triggers "refresh" method on ViewModel
, which is the most common case, requires a lot of custom code.
ha...@gmail.com <ha...@gmail.com> #4
How did this implementation get through the review? It violates such basic principles as state hoisting and is simply much worse to use than the M2 variant.
an...@sigma.software <an...@sigma.software> #5
ma...@google.com <ma...@google.com>
co...@google.com <co...@google.com>
ap...@google.com <ap...@google.com> #6
Branch: androidx-main
commit a1154475dd101bcf9867c11ceb567f9511dc7c1c
Author: Mariano Martin <marianomartin@google.com>
Date: Fri Apr 05 12:00:06 2024
[PullToRefresh] Update Material 3 PullToRefresh APIs
Test: updated existing tests
Fixes: 314496282, 317177684, 323787138, 324573502, 317177683
Relnote: "New pull-to-refresh APIs:
- Simplified PullToRefreshState to use fractional values instead of Dp units.
- isRefreshing state is controlled by the user instead of PullToRefreshState.
- Separated out the nested scroll connection from PullToRefreshState. It is handled by the new PullToRefreshBox or Modifier.pullToRefresh.
- This update is a breaking change to previous experimental APIs."
Change-Id: I0adeb950063988d1a05aca7aa135ccd982431423
M compose/material3/material3/api/current.txt
M compose/material3/material3/api/restricted_current.txt
M compose/material3/material3/integration-tests/material3-catalog/src/main/java/androidx/compose/material3/catalog/library/model/Examples.kt
M compose/material3/material3/integration-tests/material3-demos/src/main/java/androidx/compose/material3/demos/PullToRefreshDemo.kt
M compose/material3/material3/samples/src/main/java/androidx/compose/material3/samples/PullToRefreshSamples.kt
M compose/material3/material3/src/androidInstrumentedTest/kotlin/androidx/compose/material3/pulltorefresh/PullToRefreshIndicatorScreenshotTest.kt
M compose/material3/material3/src/androidInstrumentedTest/kotlin/androidx/compose/material3/pulltorefresh/PullToRefreshIndicatorTest.kt
M compose/material3/material3/src/androidInstrumentedTest/kotlin/androidx/compose/material3/pulltorefresh/PullToRefreshStateImplTest.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/pulltorefresh/PullToRefresh.kt
Description
I've been testing the new PullToRefresh implementation in M3 and finding a number of issues, both functional and API ergonomics.
1. No exit animation when
endRefresh()
is calledThis one is fairly straightforward and visible in the attached example. I would expect this to animate the progress indicator back to rest.
2. The indicator is always drawn above the content, including on top of other content
This is more serious bug, as it results in what looks like a floating artifact in the UI. The indicator implementation seems to assume that the container is always pushed up against the top of the display, and thus wouldn't worry about drawing the indicator. However, when used in even simple scaffolds with a top app bar of some sort, it will draw the indicator (even when not refreshing) on top of the app bar. This is visible in the attached examples as well.
3. The container intercepts upward scroll events in nested scrolling conditions
This is visible in the attachment as well. In a scaffold with a collapsing/moving top bar, the container will actively intercept upward scroll events in some cases and prevent expansion of the top app bar.
4. The API is awkward and a regression from the M2 implementation
The M2 implementation offered an
onRefresh: () -> Unit
callback that made using this dead simple. It also better aligned with what this UI ultimately is - a glorified refresh button implemented as a gesture. In the new M3 API, there is no such hook. Instead, one has to directly observe theisRefreshing
boolean state. In simple cases (such as the samples) where the UI is directly suspending/blocking on some refresh call to the data layer, this works ok. However, it quickly breaks down when interacting with more real-world cases where a refresh event is signaled to the data layer (such as viaLazyPagingItems.refresh()
) and awaiting an updated load state.The result is that the user has to manually implement this synchronization of states. If it's refreshing but the pager load state isn't loading, you now have to independently track if
I've attached a screenshot example of what this synchronization tedium looks like in practice