-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Do not focus the input on screen exit transition #15050
Conversation
c002e66
to
526a449
Compare
@mountiny @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] |
I've reviewed the PR and I'll have the code tested on all platforms. |
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.
Looks good to me, just waiting for the checklist to be completed, thanks Manan!
Reviewer Checklist
Screenshots/VideosWebweb-payment-button-flicker.movMobile Web - Chromemweb-chrome-payment-button-flicker.movMobile Web - Safarimweb-safari-payment-button-flicker.movDesktopdesktop-payment-button-flicker.moviOSios-payment-button-flicker.movAndroidandroid-payment-button-flicker.movThanks for the patience here. This tests well. I can notice some flicker on Android emulator, but not on a physical device. So I am guessing it's just my emulator. |
Thanks everyone! |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
Performance Comparison Report 📊Significant Changes To DurationThere are no entries Meaningless Changes To DurationShow entries
Show details
|
🚀 Deployed to staging by https://github.com/mountiny in version: 1.2.73-0 🚀
|
🚀 Deployed to production by https://github.com/francoisl in version: 1.2.73-1 🚀
|
1 similar comment
🚀 Deployed to production by https://github.com/francoisl in version: 1.2.73-1 🚀
|
Details
Using
event.data.closing
property fromtransitionEnd
event, we can prevent the focus handlers from firing at the page exit animation which will fix button jumping and flicking issues of Keyboard when going back on the native platforms.Fixed Issues
$ #14843
PROPOSAL: #14843 (comment)
Tests
Add Secondory Login page
,Add Paypal Payment method page
,Change password page
one by one.Add Paypal Payment method page
, pressing the back arrow should not cause jumping to theAdd payment method
button.Offline tests
Same as tests.
QA Steps
Add Secondary Login page
andChange password page
one by one.Add Paypal Payment method page
from Settings > Payments > Click Add Payment Method button.Add Payment Method
button does not jump.PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)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
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)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.Screenshots/Videos
Web
screen-2023-02-11_00.55.51.mp4
Mobile Web - Chrome
screen-2023-02-11_01.04.59.mp4
Mobile Web - Safari
screen-2023-02-13_18.16.55.mp4
Desktop
screen-2023-02-13_18.13.57.mp4
iOS
screen-2023-02-13_18.10.53.mp4
Android
screen-2023-02-11_00.56.33.mp4