fix: add navigating back to Calendar on Chrome mWeb#52872
Merged
Conversation
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
melvin-bot
Bot
requested review from
hungvu193
and removed request for
a team
|
@hungvu193 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Contributor
|
@jacobkim9881 Mind update the QA steps? It's mandatory 😄. Simply update it to |
Contributor
|
I wonder if we also handle back button press on Browser: Screen.Recording.2024-11-21.at.17.49.03.mov |
Contributor
Author
I haven't known it's mandatory 😅 I have updated QA steps. |
Contributor
Author
It also navigates from year picker modal to profile page w/o any fix on Browser. However the modal opens on RHP as users may click back button on the header. |
Contributor
|
I asked for confirmation here: #52383 (comment) |
Contributor
|
I think that it makes sense to use |
Contributor
Author
|
@hungvu193 We can use |
Contributor
yeah, let's do it |
Contributor
Reviewer Checklist
Screenshots/VideosAndroid: Nativeandroid.movAndroid: mWeb ChromemAndroidchrome.moviOS: Nativeios.moviOS: mWeb SafarimSafari.movMacOS: Chrome / SafariChrome.movMacOS: DesktopDesktop.mov |
hungvu193
approved these changes
Nov 27, 2024
Contributor
|
All yours @yuwenmemon |
yuwenmemon
approved these changes
Dec 2, 2024
Contributor
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
48 tasks
Contributor
|
🚀 Deployed to staging by https://github.com/yuwenmemon in version: 9.0.70-0 🚀
|
github-actions
Bot
added
the
DeployBlockerCash
Contributor
Performance Comparison Report 📊Significant Changes To Duration
Show details
Meaningless Changes To DurationShow entries
Show details
|
Contributor
|
@Expensify/mobile-deployers 📣 Please look into this performance regression as it's a deploy blocker. |
Contributor
|
Performance regression is very likely a false positive and can be ignored |
Contributor
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.0.70-9 🚀
|
8 tasks
| onModalHide={onClose} | ||
| hideModalContentWhileAnimating | ||
| useNativeDriver | ||
| shouldHandleNavigationBack |
Contributor
There was a problem hiding this comment.
This change caused the issue "The date picker is reloaded after selecting a year while enabling wallet". We have applied the fix here #70395
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
6 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.
Explanation of Change
This PR fixes clicking device back button on Year Picker Modal at Chrom mWeb. This pr gives a sign to the modal to activate
window.history.back()by event listenerpopstate. This pr only change<YearPickerModal>to give the sign. Rest feature is already added from another contributor.Fixed Issues
$ #52383
PROPOSAL: $ #52383 (comment)
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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
The fix must only work for Android mobile web(Android already has this feature).
Screen_Recording_20241121_204118_New.Expensify.mov
Android: mWeb Chrome
expensify-mWeb-test-1-2024-11-19_09.54.49.mp4
iOS: Native
2024-11-21.8.55.16.mov
There is no back button for iOS devices.
iOS: mWeb Safari
ios.2024-11-21.9.05.44.mov
There is no back button for iOS devices.
MacOS: Chrome / Safari
expensify-test-4-2024-11-27_12.13.54.mp4
MacOS: Desktop
desktop-.2024-11-21.7.00.29.mov