-
-
Notifications
You must be signed in to change notification settings - Fork 727
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
fix: Increase dashboard RefreshIndicator edge offset #1859
fix: Increase dashboard RefreshIndicator edge offset #1859
Conversation
This change adjusts the edge offset for the RefreshIndicator in the home view dashboard UI with the CustomSliverAppBar `expandedHeight` size, ensuring a better user experience by moving the RefreshIndicator under the SliverAppbar.
Opened as draft as I don't know if using "magic numbers" (actually a static number related to another static magic number) is right or not...
revanced-manager/lib/ui/widgets/shared/custom_sliver_app_bar.dart Lines 20 to 23 in a23f032
|
Can the refresh icon animation be also improved or is it because the manager refreshes too quickly that the animation is getting canceled out? |
The animation is not cancelled; it's just fast because the completion of the promise for refresh happens almost instantly. This occurs because the code doesn't wait for the promises. revanced-manager/lib/ui/views/home/home_viewmodel.dart Lines 494 to 498 in a23f032
|
Is this ready for review? |
This comment was marked as outdated.
This comment was marked as outdated.
@Domenic-MZS Can you try to remove magic numbers if possible? |
While it's technically feasible, the current trade-offs involved wouldn't add significant value. Incorporating this functionality while maintaining the current user interface would unnecessarily increase complexity for this particular case. I can think one of three possible solutions:
However, none of these solutions are particularly appealing, as the requirement is specific to only one file, and the drawbacks outweigh the benefits. In addition, the
Sure, there's still a need to ensure that updates on the shared widget don't cause disruptive changes over time, but most likely, it wouldn't happen. |
# [1.21.0-dev.6](v1.21.0-dev.5...v1.21.0-dev.6) (2024-06-30) ### Bug Fixes * Add missing import to patch options field ([d60f9aa](d60f9aa)) * Follow system theme immediately ([#1942](#1942)) ([694f2a9](694f2a9)) * Handle selecting files and folders for patch options correctly ([#1941](#1941)) ([b26760b](b26760b)) * Increase dashboard RefreshIndicator edge offset ([#1859](#1859)) ([232b702](232b702)) * Select previously applied patches when loading patch selection ([#1865](#1865)) ([7ef8f04](7ef8f04)) * Unsupported patch toast says "patchItem.unsupportedPatchVersion" ([#2011](#2011)) ([3209c0e](3209c0e)) ### Features * Save last patched app ([#1414](#1414)) ([7720408](7720408))
# [1.21.0](v1.20.1...v1.21.0) (2024-07-29) ### Bug Fixes * Add missing import to patch options field ([d60f9aa](d60f9aa)) * Adjust scroll from clipping children form fields in `AlertDialog` from `showSourcesDialog` ([#1782](#1782)) ([bbeb836](bbeb836)) * Cache external API calls ([#1911](#1911)) ([2c3e2e6](2c3e2e6)) * Change problematic translation string ([6b03f3a](6b03f3a)) * Correct architecture to armeabi-v7a ([63c6412](63c6412)) * Download latest integrations non-pre-release ([4a72267](4a72267)) * Follow language update immediately ([#1944](#1944)) ([c13827e](c13827e)) * Follow system theme immediately ([#1942](#1942)) ([694f2a9](694f2a9)) * Handle selecting files and folders for patch options correctly ([#1941](#1941)) ([b26760b](b26760b)) * Increase dashboard RefreshIndicator edge offset ([#1859](#1859)) ([232b702](232b702)) * Patching Screen draw-behind Navigation Bar ([#1945](#1945)) ([f1b25d0](f1b25d0)) * Restore consistency with the app ([ea9654e](ea9654e)) * SecurityException when patching application ([#1856](#1856)) ([e0a6de2](e0a6de2)) * Select previously applied patches when loading patch selection ([#1865](#1865)) ([7ef8f04](7ef8f04)) * Unable to install application regardless of preference ([c7627ce](c7627ce)) * Unsupported patch toast says "patchItem.unsupportedPatchVersion" ([#2011](#2011)) ([3209c0e](3209c0e)) * Update dialog shows dev version & loading gets stuck in certain circumstances ([#1792](#1792)) ([fc52560](fc52560)) ### Features * Add ability to set `null` in patch options ([#1947](#1947)) ([5c68d51](5c68d51)) * Include primary architecture in external search ([#2068](#2068)) ([23690a9](23690a9)) * open browser when clicking on changelog link ([bc300d8](bc300d8)) * Save last patched app ([#1414](#1414)) ([7720408](7720408)) * Support patching on ARMv7a ([a766352](a766352))
🔧🎨 UI Fix: Increase dashboard RefreshIndicator edge offset
This change adjusts the edge offset for the RefreshIndicator in the home view dashboard UI with the CustomSliverAppBar maxExtent size, ensuring a better user experience by moving the RefreshIndicator under the SliverAppbar.
Overview
The HomeView / Dashboard
[CustomScrollView]
refresh functionality has been enhanced by matching the starting refresh position with the[SliverAppBar]
height, and by reducing the displacement range to settle the refresh.[SliverAppBar]
, disturbing the user experience (?).[edgeOffset]
) or a better layout.📓 Description
This PR sets the Dashboard
[edgeOffset]
(indicator starting point) to match the[CustomSliverAppBar]
(title) height and changes the default[displacement]
to execute the refresh callback.✍🏼 Changes Made
[edgeOffset]
)[edge-offset].[displacement]
to settle.Technical Overview
> (For the RefreshIndicator widgtet wrapping the content) - Add the `edgeOffset` property with the `CustomSliverAppBar` hardcoded `extendedHeight`. (~magic number) - Add the `displacement` property to 50. (reduces the needed scroll to settle the "refresh" action)🦯 Testing
Update Tests (ToDo - Not Found nor needed for now...)WhatsApp.Video.2024-04-07.at.19.39.42.mp4
📋 Notes & References
RefreshIndicator - edgeOffset
Current Custom Sliver App Bar - ExpandedHeight
🔬 Reviewers
N/A (any mod is ok)
👾 Feedback, changes, or suggestions are welcome! ✌🏼 🏖️ this time an 🥑 emoji...