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

fixed issue where offline user was not able to see anything in featured circuits screen #108

Closed
wants to merge 1 commit into from

Conversation

roopak99
Copy link
Contributor

fixed issue where offline user was not able to see anything in featured circuits screen

Fixes #84

Describe the changes you have made in this PR - i have added a condition to check if the model.FETCH_FEATURED_PROJECTS has an error and show that the user if offline if it is true

Screenshots of the changes (If any) - Screenshot

Note: Please check Allow edits from maintainers. if you would like us to assist in the PR.

@manjotsidhu
Copy link
Member

manjotsidhu commented Jul 10, 2021

@roopak99 thanks for contributing to CircuitVerse, we appreciate the effort. There are few points I would like to mention :

  1. Missing Failed to load/ Error widget  #84, focuses and requires Errors to be thrown in every page where ViewModel fetches content from API and not just for Featured Circuits.
  2. The approach of this Issue requires errors for every kind of exceptions. Check ApiUtils for all possible exceptions.
  3. Your implementation just checks if there is any error while fetching featured projects and throws "Looks like your Offline" error to user irrespective of the nature of the error. There can be several types of errors that can be thrown and this is just a trivial case of offline. We would like to see errors for all types.
  4. Typo "Looks like youre offline" -> "Looks like your offline".

@manjotsidhu
Copy link
Member

manjotsidhu commented Jul 10, 2021

If you are creative, try to reduce duplicate lines of code while implementing the functionality.
Hint: Maybe you could try and have a CVException Widget which wraps any widget and throws errors for all exceptions according to ViewModel's Failure response. This widget then can be used in every page.

This will make it scalable in future also, since we are soon going to have a redesign of the whole App

@roopak99
Copy link
Contributor Author

Thanks for your valuable mentions. I will try my best to implement what you suggested.

@manjotsidhu
Copy link
Member

Closing, PR being stale. Feel free to open another PR anytime.

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.

Missing Failed to load/ Error widget
2 participants