-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/in app notification center infinite scrolling #217
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
Feat/in app notification center infinite scrolling #217
Conversation
- Add loadingMore status to InAppNotificationCenterStatus enum - Extend InAppNotificationCenterState with pagination properties: - breakingNewsHasMore, breakingNewsCursor, digestHasMore, digestCursor - Update InAppNotificationCenterState copyWith method to include new parameters - Add comments explaining the new pagination-related properties
Add a new event InAppNotificationCenterFetchMoreRequested to be dispatched when the user scrolls to the end of a notification list and more data needs to be fetched.
- Implement fetch more functionality for breaking news and digest tabs - Add loading state for pagination - Update state to include hasMore and cursor for each notification type - Refactor notification filtering into a separate method - Use bloc_concurrency's droppable transformer for fetch more events
- Add loading more indicator and functionality - Improve empty state handling - Optimize tab switching and notification loading logic - Refactor notification list to support infinite scrolling
- Introduce _notificationsFetchLimit constant for notification pagination - Update pagination limit from 20 to _notificationsFetchLimit in both initial fetch and load more operations - Improve code maintainability and readability by using a constant for pagination limit
- Update core dependency in pubspec.yaml and pubspec.lock - Change ref from b01f9b457d4faaa6fab58f67ebc09d93314e6b10 to 8a03af9af8ccf4920388ddce73062aca2fde1bc9 - This update ensures the project uses the latest version of the core dependency
- Update InAppNotificationCenterState to handle null values for notification cursors - Enable explicit setting of error to null - Use nullable wrapper for breakingNewsCursor and digestCursor - Allow redundant argument value for error to support null assignment
- Separate fetching logic for "Breaking News" and "Digests" tabs - Implement concurrent fetching for both tabs on initial load - Use filtering to request specific notification types from the backend - Simplify notification fetching and error handling - Add droppable transformer to subscription request event handling
…cations - Refactor notification fetching to use a centralized error handler - Optimize state updates by atomically updating both tabs' data - Improve pagination by updating the correct tab's state after fetching more notifications - Remove unnecessary parameters and simplify the _fetchNotifications method
Summary of ChangesHello @fulleni, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience of the in-app notification center by implementing infinite scrolling. Instead of loading all notifications at once, the system now fetches them in pages, improving performance and responsiveness, especially for users with a large number of notifications. This change involves updates across the BLoC state management, event handling, and UI components to seamlessly integrate the pagination logic. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request effectively implements infinite scrolling for the in-app notification center. The refactoring of the BLoC is well done, moving filtering logic to the backend, fetching initial data in parallel, and handling pagination state correctly. The UI changes in the _NotificationList are also solid, using a ScrollController to trigger new data fetches. I've identified one high-severity issue in the state's copyWith method that could lead to incorrect state management for pagination cursors, and a medium-severity suggestion to improve the implementation of the scroll listener in the UI. Overall, this is a great enhancement to the feature.
Status
READY/IN DEVELOPMENT/HOLD
Description
Type of Change