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.

Sign up for GitHub

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

Jump to bottom

feat(Refresh Animation): Improve the refreshing animation #1568

Open
4 tasks done
ILoveOpenSourceApplications opened this issue Dec 13, 2023 · 2 comments · May be fixed by #1859
Open
4 tasks done

feat(Refresh Animation): Improve the refreshing animation #1568

ILoveOpenSourceApplications opened this issue Dec 13, 2023 · 2 comments · May be fixed by #1859
Labels
Feature request Requesting a new feature that's not implemented yet

Comments

@ILoveOpenSourceApplications
Copy link

Feature description

When refreshing Dashboard, the refresh animation that shows up is an incomplete circle animation instead of the traditional circling cycle animation on every other apps. (Don't judge me for phrasing it this way.)

What ReVanced Manager does:

screen-20231016-004038.2.mp4

What it should be doing:

screen-20231016-004355.3.mp4

Also, the refresh icon starts showing up all the way from the notification shade instead of showing up within the manager. It should start from the point in the Dashboard where the padding is when scrolling down.

Without scrolling:
Screenshot_20231213_114911_ReVanced Manager

When scrolled down a bit:
Screenshot_20231213_114730_ReVanced Manager

The refresh icon should start after the padded area of Dashboard, like it's coming from underneath the padded area.

Motivation

I checked out the examples provided in the repository and MaterialClassicHeader seems to be the one which was implemented in ReVanced Manager (or looks like what was intended). Correct me if I'm wrong.
P.S: Attaching the gif for further clarity
material_classic

Additional context

No response

Acknowledgements

  • This request is not a duplicate of an existing issue.
  • I have chosen an appropriate title.
  • All requested information has been provided properly.
  • The issue is solely related to the ReVanced Manager
@ILoveOpenSourceApplications ILoveOpenSourceApplications added the Feature request Requesting a new feature that's not implemented yet label Dec 13, 2023
@ILoveOpenSourceApplications ILoveOpenSourceApplications changed the title feat(Refreshing Animation): Improve the refreshing animation feat(Refresh Animation): Improve the refreshing animation Jan 15, 2024
@ILoveOpenSourceApplications
Copy link
Author

@Domenic-MZS, maybe you can look into this one?

@Domenic-MZS
Copy link
Contributor

Domenic-MZS commented Apr 6, 2024 •
edited

Sorry for the delay, @ILoveOpenSourceApplications. I've been a bit busy lately.

Yup, I'd like to try this one. Let me handle this issue.

TL;DR
The issue seems to be with the current layout, where the refresh indicator wraps the AppBar and Content, causing the subsequent overscroll to show the indicator over the scrollable app bar and content.

Long Version

I think the ReVanced dashboard is using the [RefreshIndicator] widget which needs to wrap a scrollable, that when overscrolls shows a refresh jndicator, in this case it is a [CustomScroll] child having the [SliverAppbar] and then the [SliverChildList], which is the reason for the "issue."

Because the RefreshIndicator by default wraps the custom scroll in its extent, and has an edge property of 0 (which starts showing the indicator in that position), the indicator starts at the start, on the topmost position of the custom scroll (having the appbar and content )

It should be relatively easy to fix; it just needs a gap on the edge property (from which starts showing the refresh indicator) or by changing the layout.

@Domenic-MZS Domenic-MZS linked a pull request Apr 7, 2024 that will close this issue
Open
2 tasks
@validcube validcube linked a pull request Apr 8, 2024 that will close this issue
fix(ui): increase dashboard RefreshIndicator edge offset #1859
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature request Requesting a new feature that's not implemented yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(ui): increase dashboard RefreshIndicator edge offset Domenic-MZS/revanced-manager
2 participants
@Domenic-MZS @ILoveOpenSourceApplications