Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SwipeRefresh] Add swipe up to refresh feature #541

Closed
wants to merge 27 commits into from

Conversation

sitatec
Copy link

@sitatec sitatec commented Jul 7, 2021

Make the swipe to refresh bidirectional, as proposed in #516

  • Add a bottom refresh indicator that rotates in the counterclockwise sense (as the top indicator rotates in the clockwise sense) when the user swipes up at the bottom of the list.
  • Make both the top and bottom indicators optional, so the user will use either the top, the bottom, or both (but he must choose one).
  • Add a Sample for the bottom indicator and both top and bottom indicator used at the same time.
  • Update the tests
  • Update doc

    - make topSwipeRefreshState optional so the user will have the
      choice to use topSwipeRefreshState, bottomSwipeRefreshState
      or both.
    - The bottom indicator wasn't following the user's gesture when
      he swipes down. The indicators are supposed to follow the user
      gesture until he stops swiping or the indicator reaches the
      max offset
    - Fix it by consumming the scroll in the `SwipeRefreshNestedScrollConnection`
    - update the doc to take into account the parameters related
      to the bottom SwipeRefreshIndicator
@google-cla google-cla bot added the cla: yes label Jul 7, 2021
@chrisbanes chrisbanes enabled auto-merge (squash) July 7, 2021 07:41
@chrisbanes chrisbanes disabled auto-merge July 7, 2021 07:41
    - change topSwipeRefreshState and bottomSwipeRefreshState
      to topRefreshIndicatorState and bottomRefreshIndicatorState
    - Change the Bottom position sample's activity title
    - Create docsamples package and create docsamples/BidirectionalSwipeRefreshSample.kt file
    - rename DocsSamples.kt to docsamples/SimpleSample.kt
    - Add doc and code sample for bottom and bidirectional swipe refresh
    - add demo video for bottom refresh indicator
@sitatec
Copy link
Author

sitatec commented Jul 11, 2021

Review needed

docs/swiperefresh.md Outdated Show resolved Hide resolved
   - update the APIs of SwipeRefresh and SwipeRefreshIndicator to accept one state
   - use the positin parameter to specify the SwipeRefresh position
   - update the SwipeRefreshIndicator api to accept a senseOfRotation
     parameter
    - Update the indicator alignment parameter to accept only
      horizontal alignment.
@sitatec sitatec requested a review from chrisbanes July 18, 2021 13:52
Copy link
Contributor

@chrisbanes chrisbanes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nearly there! Just some comments about the API.

As this is changing the API, we need to update the API files. Please run: ./gradlew metalavaGenerateSignature

    - reset the `SwipeRefresh` signature
    - update test and docs
@sitatec sitatec requested a review from chrisbanes August 1, 2021 17:56
@JoseAlcerreca
Copy link
Collaborator

Some unresolved comments and conflicts.

# Conflicts:
#	gradle/libs.versions.toml
#	swiperefresh/src/sharedTest/kotlin/com/google/accompanist/swiperefresh/SwipeRefreshTest.kt
  - Add explicit android:exported="true" in AndroidManifest.xml for
    Android 12 support (for activities SwipeRefreshBottomPositionSample
    and BidirectionalSwipeRefreshSample
  - Replace rememberGlidePainter by rememberImagePainter in
    BidirectionalSwipeRefreshSample and SwipeRefreshBottomPositionSample
  - Some tests had been removed while merging from the main brance (fixed
    that).
  - Replace 'SemanticsNodeInteraction.performGesture' (which is
    deprecaded) by 'SemanticsNodeInteraction.performTouchInput'.
@sitatec
Copy link
Author

sitatec commented Feb 5, 2022

Some unresolved comments and conflicts.

@JoseAlcerreca, I resolved them.

@sitatec sitatec changed the title Add swipe up to refresh feature [SwipeRefresh] Add swipe up to refresh feature Feb 15, 2022
@PhilipDukhov
Copy link

SwipeRefresh the change in line 301 looks like an unused expression.

@phamsarah
Copy link

Hi, is this going to be implemented? I would like to swipe up from the bottom.

@sitatec
Copy link
Author

sitatec commented Aug 7, 2022

@phamsarah lt actually have been implemented since a year now, just not merged yet. If the maintainers will review and merge it, I will fix the conflict.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants