-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Fix #29705 - Show error alert for no internet in SUMO page #31541
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
base: main
Are you sure you want to change the base?
Conversation
Foxbolts
left a comment
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.
Hey @MarkKirichko2002 - thanks for tackling this issue 🥳
Looks like this is your first contribution to the project, so first of all, welcome!
I'd like to start by pointing you to our Contribution guidelines where you will find everything you need to know before contributing to the project, including things like our code style guidelines and pull request naming style guidelines.
It looks like you may have missed our pull request description template which we use for all pull requests.
I have investigated this issue previously and was only able to reproduce the blank page on the simulator after successfully loading the SUMO article, disabling network connectivity, and re-opening the SUMO article. I was not able to reproduce this on a physical device. Were you able to reproduce on a physical device?
As far as the actual resolution, I believe we would want to solve this a bit differently. As you can see in this line of PrivacyPolicyViewController, we have a specific error page we want to load when there is no network connectivity. We'd like to continue using this error page as opposed to a UIAlertViewController, as this is already the precedent set and it contains localized error messages.
It appears you also contributed some unnecessary files like local changes to project.pbxproj, version.xcconfig, and package-lock.json - all of these should be reverted as they do not pertain to the issue you worked on.
Finally, I recommend running swiftlint --config ../.swiftlint.yml --fix locally as there seems to be quite a few warnings pertaining to trailing whitespace
If you have any questions, please let me know!
- Fixed blank page when opening SUMO article offline - Replaced UIAlertViewController with existing error page - Removed trailing whitespace (swiftlint --fix) - Reverted unrelated files (project.pbxproj, version.xcconfig, package-lock.json)
|
This pull request has conflicts when rebasing. Could you fix it @MarkKirichko2002? 🙏 |
|
Hey @MarkKirichko2002 thanks for pushing some new changes. I pulled down your branch and noticed a couple of issues when testing it:
Furthermore, I noticed there are still changes to the |
- Show custom "The internet connection appears to be offline" error instead of system "The operation couldn't be completed" message - Fix network state detection to clear error when connection is restored - Revert project.pbxproj and package-lock.json to match main branch version
|
@MarkKirichko2002 thanks for the additional changes, looks like you have some conflicts, hopefully that resolves the changes to the project files as those seem unrelated to this change. @dicarobinho are you familiar in this area? Would mind taking a look at this solution? |
|
This pull request has conflicts when rebasing. Could you fix it @MarkKirichko2002? 🙏 |
|
This pull request has conflicts when rebasing. Could you fix it @MarkKirichko2002? 🙏 |
📜 Tickets
Jira ticket
Github issue
💡 Description
PrivacyPolicyViewController, instead of UIAlertController or blank page.swiftlint --config ../.swiftlint.yml --fixlocally — all trailing whitespace and warnings resolved.📝 Checklist
Notes: