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

feat: notifications inbox screen error handling #117

Open
wants to merge 7 commits into
base: 2U/develop
Choose a base branch
from

Conversation

mta452
Copy link

@mta452 mta452 commented Feb 4, 2025

LEARNER-10340: Notifications Inbox Screen Error Handling

Description

Following are the highlights of this PR:

  • PaginationManager: Introduced a new class to reuse the data fetching logic for a paginated API. It has been designed to cover as many use cases as possible while keeping the public API as simple as possible.
  • BackNavigationButton: Added support for insets to increase the tappable area of the button.
  • ScreenStateView: Created a custom view to show the current state of a screen.
  • NotificationsInboxView: Implemented pull to refresh, state and error presentation in the view.

Edge Cases

Swipe-to-Refresh Implementation:

  • Added pull-to-refresh functionality to the Notifications Inbox screen for seamless updates.
  • On pull-to-refresh, retain existing notifications until new data is fetched successfully. If the fetch fails, show an error message at the bottom.

UI States and Messages:

  • No error message when unable to mark notifications as seen (background operation).
  • Show the empty state when there are no notifications.
  • Show the network error state when fetching the very first page fails due to network problems.
  • Show the server error state when fetching the very first page fails due to an issue on the server.
  • On a second-page fetch error, show an error message at the bottom.
  • On "mark as read" or "mark all as read" failure, show an error message at the bottom.

Preview

iPhone

No Internet Server Error No Data
Simulator Screenshot - iPhone 16 Pro - 2025-02-04 at 12 41 36 Simulator Screenshot - iPhone 16 Pro - 2025-02-04 at 12 51 25 Simulator Screenshot - iPhone 16 Pro - 2025-02-04 at 12 40 28
Simulator Screenshot - iPhone 16 Pro - 2025-02-04 at 12 41 26 Simulator Screenshot - iPhone 16 Pro - 2025-02-04 at 12 51 31 Simulator Screenshot - iPhone 16 Pro - 2025-02-04 at 12 40 39

iPad

No Internet Server Error No Data
Simulator Screenshot - iPad Pro 11-inch (M4) - 2025-02-04 at 12 58 10 Simulator Screenshot - iPad Pro 11-inch (M4) - 2025-02-04 at 12 55 35 Simulator Screenshot - iPad Pro 11-inch (M4) - 2025-02-04 at 12 57 07
Simulator Screenshot - iPad Pro 11-inch (M4) - 2025-02-04 at 12 58 05 Simulator Screenshot - iPad Pro 11-inch (M4) - 2025-02-04 at 12 55 30 Simulator Screenshot - iPad Pro 11-inch (M4) - 2025-02-04 at 12 57 14

@mta452 mta452 requested review from rnr and shafqat-muneer February 4, 2025 08:40
@rnr
Copy link
Collaborator

rnr commented Feb 4, 2025

@mta452 Thank you. Could you please fix tests.

Copy link
Collaborator

@rnr rnr left a comment

Choose a reason for hiding this comment

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

Great work! LGTM

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

Successfully merging this pull request may close these issues.

2 participants