[No QA] Add a special gesture on dev to open the test tool menu#17253
Conversation
This reverts commit 5cbcfbb.
|
@neil-marcellini @mananjadhav One of you needs to 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] |
mananjadhav
left a comment
There was a problem hiding this comment.
@alexxxwork From the first pass of the review, I can see a huge scope of improvement to avoid code duplication between ScreenWrapper/index.js and ScreenWrapper/index.native.js.
Could we please optimize that and then I can review it?
83d4a22 to
dba33d4
Compare
|
@alexxxwork Is it ready for the second pass of the review? |
|
@neil-marcellini @mananjadhav All done |
neil-marcellini
left a comment
There was a problem hiding this comment.
Looking really good! One small thing and then I'm happy with it.
neil-marcellini
left a comment
There was a problem hiding this comment.
Good to go from me!
|
All you @mananjadhav |
|
Currently testing this, I can't seem to get it working on Web. The numberOfActiveTaps count shows as 1 only. But I am separately struggling with iOS issues. So I'll need a day to close the testing for this one. |
Web on desktop? It's probably not possible to test this on devices without a touch screen. I believe there is a method to add a 4 finger gesture with Chrome dev tools, but I don't think it's worth the effort. Lets make sure it works well on mobile and call it good. I tested only on mobile web, accessing the dev build of the app through ngrok. |
Reviewer Checklist
Screenshots/VideosWebweb-test-modal.mp4Mobile Web - Chromemweb-chrome-test-modal.movMobile Web - Safarimweb-safari-test-modal.MP4DesktopNA iOStest-tools-ios.mp4Androidandroid-test-modal.mov |
|
@neil-marcellini yeah I attempted trying DevTools, couldn't get it working. I am also blocked with iOS build errors. @neil-marcellini Could you help complete the iOS part for screencasting? |
Sure, it tests well for me on iOS, but note that I had to enter 'd' in the iOS simulator rather than the terminal window. Also, to build on iOS I had to update and follow the steps here. Hopefully that helps you. test-tools-ios.mp4 |
|
Ok, I marked off the last part of the checklist and I marked this as No QA because the changes can only be tested on dev, and we already tested that. Merging! |
|
Hmm @alexxxwork the jest unit tests are failing and it looks unrelated. Would you please try merging main? |
|
@neil-marcellini jest coudn't render App because it couldn't find react-native-dev-menu. |
77782f7
|
✋ 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/neil-marcellini in version: 1.3.3-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 1.3.3-2 🚀
|
Details
On dev only, you can now open a test tools modal by tapping with 4 fingers, making it easy to toggle force offline from any page.
Fixed Issues
$ #16737
PROPOSAL: #16737 (comment)
Tests
Mobile web (or a device with touchscreen):
Android and iOS:
Offline tests
N/A
QA Steps
No QA
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
2023-04-11.12-24-44.mp4
Mobile Web - Chrome
Screen_Recording_20230411_125450_Chrome.mp4
Mobile Web - Safari
Desktop
iOS
Rec.2023-04-11.12.40.08.mov
Android
Screen_Recording_20230411_125025_New.Expensify.mp4