Stop hiding data when there is no connection#10848
Merged
Conversation
|
You can test the changes on this Pull Request by downloading the APK here. |
develric
approved these changes
Nov 25, 2019
Contributor
develric
left a comment
There was a problem hiding this comment.
Hi @planarvoid , the approach seems reasonable and the code looks good! 👍
I was not able either to fully reproduce it, but I verified the desired difference in behavior with the following steps:
- Activate the Today widget and check that it shows the correct today stats
- turn on air mode
- click on the widget to open the stats in the app
- come back with back button to the home screen
Doing the above you get the no network message before your modification whereas the stats are preserved as it should be using your modification. AFAIU this is the correct expected behavior, so good job! 😎
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #10836
The idea behind this change is to not allow the
errorstate of no connection to override the actual data shown. The widget refreshes the data periodically and I think there might actually be a fake no-connection response. I couldn't actually reproduce this error.Once you have a widget data, we save a flag into the shared prefs for the given widget and never replace the data with the error (if the user still has the access token).
To test:
PR submission checklist:
I have considered adding unit tests where possible.
I have considered if this change warrants user-facing release notes and have added them to
RELEASE-NOTES.txtif necessary.