-
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
Chrome mWeb - scan button provides no feedback #30094
Chrome mWeb - scan button provides no feedback #30094
Conversation
I tested on android chrome but I am not requested to grant/deny permission anymore. XRecorder_21102023_040017.mp4 |
@situchan this happens only on android chrome, or anywhere else? |
android chrome. On iOS, there's only option to allow/reject permission. No way to dismiss. Once rejected, it's expected not to show prompt again. |
@situchan yep, this is how iOS deals with permissions |
Bug still exists on iOS chrome. RPReplay_Final1697826416.MP4 |
I think below is 100% reproducible step.
|
We should find the real root cause why this is reproducible only after refresh. |
@situchan try now |
Tried that now and it works on my iOS Chrome |
Still doesn't work for me |
@situchan one more little fix |
Works on iOS chrome. Checking android |
I think we're almost close. Can we fix infinite loading as well? Repro step:
|
@situchan fixed the infinite loader after refresh |
Sure, I've triggered one for you. It should post here shortly. |
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪 |
@lukemorawski I tested latest changes. camera doesn't show at all on native |
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.
Not ready to merge yet
Please also pull main. The branch is very old |
* @param {Object} params - The parameters object. | ||
* @param {number} params.tabIndex - The index of the tab for which focus status is being determined. | ||
* @returns {boolean} Returns `true` if the tab is both animation-focused and screen-focused, otherwise `false`. |
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.
Would you mind applying this change, then I think it should be all ready to merge.
@tgolen done |
A few conflicts popped up now, sorry! |
@tgolen ok, merged with main. Had to remove a HOC that was wrapping up |
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, just a final lint error.
@tgolen done, sorry about that |
One sec, I will do final test in a few min |
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.
Tested on all platforms. Looks good
amen! |
✋ 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 production by https://github.com/mountiny in version: 1.4.0-3 🚀
|
Details
This addreses an issue where a web scan flow would not work on iOS Chrome.
Root cause was mounting and unmounting a video element by the tab navigator when navigating trough tabs in receipt selector.
For some unknown reason chrome was not handling this well.
Now the camera, through an additional hook, is listening to tab navigation events and renders the webcam compononent when tab is fully focused and transition animation has ended.
Fixed Issues
$ #29990
PROPOSAL: no proposal
Tests
Offline tests
QA Steps
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)/** comment above it */
this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)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
iOS: mWeb Chrome
web.ios.chrome.480.mov
Android: mWeb Chrome
web.android.chrome.mov
iOS: mWeb Safari
web.ios.safari.mov