-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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 web app not recognising device being online on chromebook connected via instant tethering #6500
Fix web app not recognising device being online on chromebook connected via instant tethering #6500
Conversation
This change looks good to me. I'm unsure why the GH actions are failing - but it doesn't look related to this. |
Would you mind listing out some tests in the removed "QA Steps" section? We have all PRs manually tested by our QA team and they will only test what we explicitly ask them to test. Just noticed it only mentions Chromebook there which I'm not sure they will be able to test. Thanks! |
I've renamed "Tests" section to "QA Steps". Those are the two steps I used to verify that the problem with instant tethering is resolved.
It also has to be a fairly modern chromebook that supports instant tethering, not all of them are. This article is inaccurate. |
Tests is what your reviewer should do to verify the changes. QA Steps are for QA. They should not be combined.
Mostly just test that offline is detected? The test is not much different. Just use chrome dev tools network tab, turn off wifi, turn off cell data, etc. |
Added a separate QA steps entry
I've added these steps to QA. I've also tested it on mobile web, I don't have access to other platforms. |
Ah we need to run |
No, I don't have a mac |
Ok, are you able to test on Android? Let us know if we can help get you set up there. I would be happy to cover the iOS testing and changes for you once this is merged. |
Tested on Desktop and iOS and verified everything is still working. |
I can do that, i'll need a couple of hours to set-up the environment
Thanks |
Tested on Android, everything works as expected |
Thanks @eVoloshchak let's |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by @marcaaron in version: 1.1.17-8 🚀
|
🚀 Deployed to production by @roryabraham in version: 1.1.18-3 🚀
|
Details
This is a migration to 7.3.1 version of @react-native-community/netinfo, which accommodates this pull-request.
Fixed Issues
#4910
Tests
QA Steps
Tested On
Screenshots
HP chromebook 11 G6
Screen.recording.2021-11-29.18.29.26.mp4
Note
We are migrating 2 major versions (from 5 to 7), so there could be some breaking changes. This needs to be properly tested on all platforms.