-
Notifications
You must be signed in to change notification settings - Fork 3k
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: add navigating back to Calendar on Chrome mWeb #52872
Conversation
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
@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] |
@jacobkim9881 Mind update the QA steps? It's mandatory 😄. Simply update it to |
I wonder if we also handle back button press on Browser: Screen.Recording.2024-11-21.at.17.49.03.mov |
I haven't known it's mandatory 😅 I have updated QA steps. |
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. |
I asked for confirmation here: #52383 (comment) |
I think that it makes sense to use |
@hungvu193 We can use |
yeah, let's do it |
Reviewer Checklist
Screenshots/VideosAndroid: Nativeandroid.movAndroid: mWeb ChromemAndroidchrome.moviOS: Nativeios.moviOS: mWeb SafarimSafari.movMacOS: Chrome / SafariChrome.movMacOS: DesktopDesktop.mov |
All yours @yuwenmemon |
✋ 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/yuwenmemon in version: 9.0.70-0 🚀
|
Performance Comparison Report 📊Significant Changes To Duration
Show details
Meaningless Changes To DurationShow entries
Show details
|
@Expensify/mobile-deployers 📣 Please look into this performance regression as it's a deploy blocker. |
Performance regression is very likely a false positive and can be ignored |
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.0.70-9 🚀
|
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 Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.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