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

fix: crash when internet disconnected right after opening a course #30

Merged

Conversation

dixidroid
Copy link
Collaborator

@dixidroid dixidroid commented Aug 8, 2024

The crash was fixed + added UI handlers for the Home and Videos tabs to stop infinite loading

Copy link

@HamzaIsrar12 HamzaIsrar12 left a comment

Choose a reason for hiding this comment

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

Hey @dixidroid, the underline issue is resolved. 👏🏻

I noticed a minor issue: we're loading the Video tab content from the cache, but not the Home tab content.

Steps to reproduce:

  • Open a course and let it load.
  • Return to the Main Dashboard.
  • Open the course again and quickly switch to airplane mode.
  • You’ll see that the Home Tab shows a "No Content" error, while the Videos Tab loads as expected.
Screenrecorder-2024-08-13-19-48-08-381.mp4

@dixidroid
Copy link
Collaborator Author

@HamzaIsrar12 Hi!
This happens because for the Home tab we need to send 2 more requests to get the course status and course dates, but for the Video tab we only need to get the course structure which was cached.

Copy link

@HamzaIsrar12 HamzaIsrar12 left a comment

Choose a reason for hiding this comment

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

Sounds good! Approving this for now, but let's address it with a proper Cache-First approach in a separate PR. 😄

@HamzaIsrar12
Copy link

Merging with a single approval since we need to include it in the release.

@HamzaIsrar12 HamzaIsrar12 merged commit 590a78c into edx:2U/develop Aug 15, 2024
4 checks passed
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