fix:68051: Rerender map after being online again#68802
fix:68051: Rerender map after being online again#68802Valforte merged 1 commit intoExpensify:mainfrom
Conversation
|
Hi @brunovjk I had to use a slightly different approach than the main one (just changing the key), because this approach causes an error in the console, and one of the items on the checklist is no errors in the console. I used a different approach, which also demounts (like the key, it remounts) the component depending on whether the user is online. If you have any comments, please write, and we can discuss it here. Thank you. |
|
Great! I will review it ASAAP :D Thank you. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp68802_android_native.movAndroid: mWeb Chrome68802_android_web.moviOS: HybridApp68802_ios_native.moviOS: mWeb Safari68802_ios_web.movMacOS: Chrome / Safari68802_web_chrome.movMacOS: Desktop68802_web_desktop.mov |
|
LGTM! Thank you @Eskalifer1, but do you think we can create a unit test for this component? Thanks. |
|
To be honest, I'm not very good at writing tests for components, and I'm not sure if you can use lazy and online checking in tests to write a test for this case. If you think it's possible and it's really necessary, then let me know. Thanks |
|
I understand, I could be wrong, but I believe we can, but I'm not sure if we should. I'll check. Thank you. |
|
@Eskalifer1 for now would you mind updating the PR Details, Explanation of Change topic? Thank you. |
|
Imo, this is a tough case to test, maybe you can help with a suggested case @brunovjk ? I'm willing to skip a test case here if it gets too complicated |
brunovjk
left a comment
There was a problem hiding this comment.
You're right, I've been trying to come up with a test, but it's actually more complicated than it seems. Let's move on to the PR; the changes work well in my tests. Thank you.
|
✋ 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 https://github.com/Valforte in version: 9.1.99-0 🚀
|
|
🚀 Deployed to production by https://github.com/thienlnam in version: 9.1.99-11 🚀
|
Explanation of Change
This PR adds changes for rendering the map in the browser. Since we load this component via lazy loading, we get an unexpected error when switching from offline to online mode. In this PR, we add logic to re-render the component when switching from offline to online mode.
Fixed Issues
$#68051
PROPOSAL:#68051 (comment)
Tests
Offline tests
No offline steps as need to be online in one of the steps to reproduce the bug
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
68051-android-native.mp4
Android: mWeb Chrome
68051-android-web.mp4
iOS: Native
68051-ios-native.mp4
iOS: mWeb Safari
68051-ios-web.mp4
MacOS: Chrome / Safari
68051-web.mp4
MacOS: Desktop
68051-desktop.mp4