-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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: 50485 mWeb/Chrome - IOU- Continue and green button not working in scan page #52087
Conversation
@dominictb 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] |
It's ready for review. I'll complete the screenshots in the meantime |
docs/articles/new-expensify/expenses-&-payments/Create-an-expense.md
Outdated
Show resolved
Hide resolved
Please copy the |
I'm working on it |
@layacat Ping me when you're done. |
Sure. I'm having issue with running the docs project. I'll keep you posted |
@layacat any new updates? |
I'm updating the docs section with your advice. I'll also address all the review comments in one go. |
@dominictb What do you think about this? I linked user to FAQ section if they click mbbrs.mov |
@layacat Looks much better. How about just the FAQ question? The info box seems an extra step and quite redundant to me. |
I updated. Please check |
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.
Please do it very carefully this time to speed up the PR! I already included the changes with detailed text for you.
docs/articles/new-expensify/expenses-&-payments/Create-an-expense.md
Outdated
Show resolved
Hide resolved
docs/articles/new-expensify/expenses-&-payments/Create-an-expense.md
Outdated
Show resolved
Hide resolved
docs/articles/new-expensify/expenses-&-payments/Create-an-expense.md
Outdated
Show resolved
Hide resolved
docs/articles/new-expensify/expenses-&-payments/Create-an-expense.md
Outdated
Show resolved
Hide resolved
docs/articles/new-expensify/expenses-&-payments/Create-an-expense.md
Outdated
Show resolved
Hide resolved
All Done. Please check again. |
@layacat Please update your screenshots and videos as well showing the final flow and text. We need marketing approval on the text. |
Sure. I've added. |
Reviewer Checklist
Screenshots/VideosAndroid: mWeb ChromeTested on emulator using mocked camera. Screen.Recording.2024-11-21.at.17.47.05-compressed.moviOS: mWeb SafariTested on simulator using mocked camera. Screen.Recording.2024-11-21.at.17.37.13-compressed.mov |
@layacat Please update Tests and QA Tests:
|
docs/articles/new-expensify/expenses-&-payments/Create-an-expense.md
Outdated
Show resolved
Hide resolved
…nse.md Update end of line dot. Co-authored-by: Dominic <165644294+dominictb@users.noreply.github.com>
@layacat You missed the QA Steps |
Updated. |
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
@Julesssss Need your help here:
|
@layacat Thanks for your collaborative attitude to get this done ASAP today 🚀 However next time please be more mindful, careful and attentive to details. Refine your changes before requesting review. Think about other similar cases/places. So we can avoid back-to-back conversations for just several very minor changes. |
Hey @dominictb. Yeah that sounds correct looking at internal instructions. We can review it briefly on prod before paying out 👍 |
I requested copy review here. |
@layacat Because QA cannot test Help docs on staging, please update QA Steps as follow:
|
And remove "Same as Tests" from Offline tests. If we're offline, we cannot open Help site. |
Thanks. I updated |
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.
Copy was approved. Lets merge
✋ 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/Julesssss in version: 9.0.68-0 🚀
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.0.68-7 🚀
|
Explanation of Change
Update the docs and explanation for IOU camera flow.
Fixed Issues
$ #50485
PROPOSAL: #50485 (comment)
Tests
[mWeb only]
Precondition: App has never asked for camera permission before. Might need to clear the browser cache to reset permission settings.
Camera access still hasn't been granted, please follow these instructions.
message andContinue
button appearsthese instructions
How can I enable camera permission for a website on mobile browsers?
highlightedOffline tests
N/A
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
How to enable camera permission for a website on mobile browsers?
Google Chrome:
Safari:
[mWeb only]
Precondition: App has never asked for camera permission before. Might need to clear the browser cache to reset permission settings.
Camera access still hasn't been granted, please follow these instructions.
message andContinue
button appearsthese instructions
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop