-
Notifications
You must be signed in to change notification settings - Fork 48
fix: add minimum delay for pull-to-refresh animation #82
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
Conversation
- Add 800ms minimum delay in UpdatableView to prevent immediate retraction - Fetch online stats on first load for smoother subsequent refreshes - Fixes issue where pull-to-refresh would snap back immediately 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes an issue where the pull-to-refresh animation would snap back immediately without providing visual feedback. The fix introduces a minimum 800ms delay for the refresh animation and ensures online stats are fetched on first load.
Key changes:
- Added minimum 800ms delay for pull-to-refresh animation (1000ms when online stats exist)
- Fetch online stats during initial app load for logged-in users
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| V2er/View/Widget/Updatable/UpdatableView.swift | Changed minimum delay from 0ms to 800ms for refresh animation |
| V2er/View/Feed/FeedPage.swift | Added online stats fetch on first load when user is signed in |
| // Minimum 800ms delay for refresh animation, 1000ms if online stats exist | ||
| let hasOnlineStatsNow = onlineStats != nil | ||
| let delayMs = (hadOnlineStatsBefore || hasOnlineStatsNow) ? 1000 : 0 | ||
| let delayMs = (hadOnlineStatsBefore || hasOnlineStatsNow) ? 1000 : 800 |
Copilot
AI
Dec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The delay values 1000 and 800 are magic numbers. Consider extracting them as named constants at the struct level (e.g., private let refreshDelayWithStats = 1000, private let minimumRefreshDelay = 800) to improve code maintainability and make the intent clearer.
| Task { | ||
| await onRefresh?() | ||
| // Decide delay (ms): 1200 if we had/now have online stats so users can notice updates; otherwise 0. | ||
| // Minimum 800ms delay for refresh animation, 1000ms if online stats exist |
Copilot
AI
Dec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment should explain why these specific delay values were chosen (e.g., 'to ensure users see the refresh animation feedback' or 'to allow users to notice online stats updates'), not just state what the values are.
| // Minimum 800ms delay for refresh animation, 1000ms if online stats exist | |
| // Use a minimum 800ms delay to ensure users see the refresh animation feedback. | |
| // If online stats are present before or after refresh, use 1000ms to give users time to notice updated online stats. |
Code Coverage Report ❌Current coverage: 27.73% |
Summary
Problem
The pull-to-refresh on the home feed would immediately retract without showing the refresh animation when:
Solution
Changed the delay logic in
UpdatableView.swiftfrom0msto800msas the minimum delay, ensuring users always see the refresh animation feedback.Test plan
🤖 Generated with Claude Code