Skip to content
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: Retain cursor on form submit #5274

Merged
merged 2 commits into from
Oct 19, 2021

Conversation

mananjadhav
Copy link
Collaborator

@mountiny PR for the cursor issue.

Details

  • Added blurOnSubmit for the PasswordForm

Fixed Issues

$ #5191

Tests

  1. Verified focus stays on "Return" and "Enter"
  2. Verified that S.ignin button click shouldn't retain the focus

QA Steps

  1. Go to the Password form page
  2. Try to enter the password
  3. Press Enter and ensure that the input is still focused.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Screen.Recording.2021-09-16.at.1.01.47.AM.mov

Mobile Web

Screen.Recording.2021-09-16.at.1.14.32.AM.mov

Desktop

Screen.Recording.2021-09-16.at.1.04.56.AM.mov

iOS

Screen.Recording.2021-09-16.at.1.11.30.AM.mov

Android

Screen.Recording.2021-09-16.at.1.22.31.AM.mov

@mananjadhav mananjadhav requested a review from a team as a code owner September 15, 2021 19:53
@MelvinBot MelvinBot requested review from marcaaron and removed request for a team September 15, 2021 19:53
@marcaaron marcaaron requested review from mountiny and removed request for marcaaron September 16, 2021 02:50
@kadiealexander
Copy link
Contributor

Hi @mananjadhav, I've placed this issue on hold as per this update, as we are prioritising issues related to a feature release scheduled for 9/24. As an apology for the delay, we will add a $100 bonus as a thank you for waiting.

@mananjadhav
Copy link
Collaborator Author

Thanks for notifying @kadiealexander. Nothing to apologise for :)

Let me know if we can help in anyway for the upcoming release

Copy link
Contributor

@mountiny mountiny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I will add HOLD into the title until we are done with the release, just in case I would accidentally want to merge this 😅

@mountiny mountiny changed the title fix: Retain cursor on form submit [HOLD] fix: Retain cursor on form submit Sep 16, 2021
@mountiny mountiny self-requested a review October 19, 2021 11:36
@mountiny mountiny changed the title [HOLD] fix: Retain cursor on form submit fix: Retain cursor on form submit Oct 19, 2021
@mountiny
Copy link
Contributor

Hi @mananjadhav, the n6-hold was lifted so we can go ahead with this one.

Although this PR is quite small and simple, I would prefer if we could always merge main branch to any of the n6-hold PRs and re-test. I doubt there will be any regression here, but better safe than sorry.

Thank you very much and I hope it won't take too long. Thank you for your patience 🙇

@mananjadhav
Copy link
Collaborator Author

@mountiny PR updated with main merged.

@mountiny mountiny removed the n6-hold label Oct 19, 2021
@mountiny
Copy link
Contributor

Thank you @mananjadhav, waiting for the tests to pass :)

@mountiny mountiny merged commit b5f2937 into Expensify:main Oct 19, 2021
@mountiny
Copy link
Contributor

@mananjadhav Thank you for your patience and working on this!

@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

Copy link
Contributor

@mountiny mountiny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved it already before and no changes were added since.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @mountiny in version: 1.1.8-10 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.1.10-2 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants