-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[NoQA] Bump react-navigation/native and react-navigation/stack version #15928
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
[NoQA] Bump react-navigation/native and react-navigation/stack version #15928
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
@cristipaval 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] |
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
|
Hey @adamgrzybowski ! You have to correctly link the gh issue in the Fixed Issue section in order to get the right people assigned as reviewers to the PR. I edited it for now and manually assigned the right reviewers. |
mountiny
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.
@adamgrzybowski thanks! Can you fill out the videos for all the platforms confirming you have tested it all works now?
@s77rt would you be able to go through all the testing steps, please?
|
|
@s77rt lets test core navigation things, some internal deeplinks (a link in the message), link from outside of the app, just in case you wont notice any regression @adamgrzybowski lets get the commits signed, you can also create a new branch and force push if you cant get the old commits verified. |
Reviewer Checklist
Screenshots/VideosWebweb.mp4Mobile Web - Chromemweb-android.mp4Mobile Web - Safarimweb-safari.mp4Desktopdesktop.mp4iOSios.mp4Androidandroid.mp4 |
|
I have uploaded testing videos. Initial tests look promising, all seem normal. Will perform more tests asap. |
|
@adamgrzybowski Please add those to the PR description (follow the template) and add mWeb as well. |
mountiny
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.
Looking good, let me know @s77rt when its ready to merge
|
Just waiting for testing videos.... |
|
@s77rt Just added. Thanks for your patience and help! 🙇 All that procedures are still new to me |
s77rt
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.
LGTM! 🚀
|
@mountiny Let's merge this 🚀 |
mountiny
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.
Tests passed, thanks!
|
Is this eligible for C+ compensation? |
|
sorted in the linked issue |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 1.3.25-1 🚀
|
|
🚀 Deployed to production by https://github.com/roryabraham in version: 1.3.25-8 🚀
|
Details
Minor bump of react-navigation packages: stack and native.
There is one particular bugfix in the newest version especially important for the navigation refactor.
There is the commit with fix react-navigation/react-navigation@ab66777
Fixed Issues
$ #15846
PROPOSAL: GH_LINK_ISSUE(COMMENT)
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)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
Web
Screen.Recording.2023-03-14.at.19.43.30.mov
Mobile Web - Chrome
Screen.Recording.2023-03-14.at.20.23.23.mov
Mobile Web - Safari
Screen.Recording.2023-03-14.at.20.14.55.mov
Desktop
Screen.Recording.2023-03-14.at.19.58.06.mov
iOS
Screen.Recording.2023-03-14.at.19.50.22.mov
Android
Screen.Recording.2023-03-14.at.19.48.32.mov