Closed FitseTLT closed 9 hours ago
@jayeshmangwani 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]
Working on uploading screenshots ...
Upload finished
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and not onIconClick
).src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components using Avatar
have been tested & I retested again)/** comment above it */
this
properly so there are no scoping issues (i.e. for onClick={this.submit}
the method this.submit
should be bound to this
in the constructor)this
are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this);
if this.submit
is never passed to a component event handler like onClick
)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG
)Avatar
is modified, I verified that Avatar
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 the Test
steps.@FitseTLT I'm not sure why, but on Android, a workspace is created twice if it comes from a closed app on onboarding flow.
https://github.com/user-attachments/assets/3ecbc133-2469-4cc0-a4ad-26580fb11eff
I don't think so @jayeshmangwani Maybe you were on a wrong branch. I tried to exactly do what is on your video but couldn't reproduce
https://github.com/user-attachments/assets/39e377be-dffa-4307-b74b-6839b3df9660
Maybe you were on a wrong branch
No, thats not the case, I am on right branch
I am checking on real device, let's see if that my emulator thing
For me, it's still creating two workspaces
@mountiny Can we please generate build here so that we can test it on an Android device?"
@FitseTLT Do you have any idea, How can we open the OD web link in the ND staging version of the app? We are testing locally using npx uri-scheme open
, but how can we verify it on the staging version?
@FitseTLT Do you have any idea, How can we open the OD web link in the ND staging version of the app? We are testing locally using
npx uri-scheme open
, but how can we verify it on the staging version?
@jayeshmangwani Sorry but I am totally not understanding what you meant.
I am totally not understanding what you meant.
Sorry about that! Let me simplify it: how have you tested this PR on Android?
npm run android
Sorry, I didn’t quite get you. npm run android
doesn’t work for me with this PR. Can you please at least show me on video how you trigger the Choose the “1-9” option on the home page (signUpQualifier: vsb)
step on Android?
🚧 @mountiny has triggered a test build. You can view the workflow run here.
:test_tube::test_tube: Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! :test_tube::test_tube: | Android :robot: | iOS :apple: |
---|---|---|
https://ad-hoc-expensify-cash.s3.amazonaws.com/android/52899/index.html | https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/52899/index.html | |
Desktop :computer: | Web :spider_web: | |
https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/52899/NewExpensify.dmg | https://52899.pr-testing.expensify.com | |
:eyes: View the workflow run that generated this build :eyes:
@jayeshmangwani here is the build
Thanks, I’m checking it now
@mountiny , I am trying to test the transition link from OldDot to NewDot on a native Android device for the signup flow when we select the VSB option on OD, but for me, it is always opening in mobile web. Is there any way you are aware of to test on a native Android device? Otherwise, this issue cannot be tested in a real scenario.
I am not sure how to test the short-lived token in your dev setup, sorry 😬 its probably not possible as the deeplinks will be picked up in the actual app, not your dev apk
I think we can proceed with this PR and merge it, as testing on both the production web and mobile app shows that we are not opening the transition deep link on mobile (when creating a new account and selecting the VSB option).
:hand: This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.
Details
Fixed Issues
$ https://github.com/Expensify/App/issues/52894 PROPOSAL: https://github.com/Expensify/App/issues/52894#issuecomment-2491045711
Tests
Offline tests
Same as above
QA Steps
Same as above
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label 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
https://github.com/user-attachments/assets/ef1b80af-ea7d-4ab3-99d8-9f2366601051Android: mWeb Chrome
https://github.com/user-attachments/assets/9429dc86-34a4-4113-9e61-cc7938176a92iOS: Native
https://github.com/user-attachments/assets/925c9ac7-14df-46df-9966-f4b6b1efea38iOS: mWeb Safari
https://github.com/user-attachments/assets/bfe14ae0-60ef-4998-903c-84ab8bf1526aMacOS: Chrome / Safari
https://github.com/user-attachments/assets/2794d79c-ab42-4ce5-963f-4062497594e1MacOS: Desktop
https://github.com/user-attachments/assets/dfba3951-4c9c-4968-a22b-3c7dafde1628