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

Add autoCapitalize prop to avoid first letter duplication on android #6916

Merged

Conversation

CamilaRivera
Copy link
Contributor

@CamilaRivera CamilaRivera commented Dec 28, 2021

Details

Add autoCapitalize prop for TextInputWithPrefix and TextInputWithLabel (drill)
Pass autoCapitalize="none" prop to TextInputWithLabel in WorkspaceNewRoomPage

Fixed Issues

$ #6725

Tests | QA Steps

  1. Log in with an Expensify account
  2. Click on global create
  3. Create new chat room
  4. Start typing on room name

Expected result: You should not get the first letter duplicated

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Screen.Recording.2021-12-28.at.8.39.09.PM.mov

Mobile Web

Desktop

iOS

Screen.Recording.2021-12-28.at.8.37.55.PM.mov

Android

XRecorder_Edited_28122021_122837.mp4

@CamilaRivera CamilaRivera marked this pull request as ready for review December 28, 2021 23:37
@CamilaRivera CamilaRivera requested a review from a team as a code owner December 28, 2021 23:37
@MelvinBot MelvinBot removed the request for review from a team December 28, 2021 23:37
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.

@parasharrajat Now I am not sure how our linter is set up, but will it not throw error in case we do not define the the props? If that is not a problem, I agree with removing the props.

@parasharrajat
Copy link
Member

Yeah, @mountiny. It won't throw an error.

Copy link
Member

@parasharrajat parasharrajat left a comment

Choose a reason for hiding this comment

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

LGTM.

Cc: @mountiny

@CamilaRivera
Copy link
Contributor Author

LGTM.

Thanks, @parasharrajat! You were right, I didn't need to specify the props, I tested it and it works fine :)

Copy link
Member

@parasharrajat parasharrajat left a comment

Choose a reason for hiding this comment

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

Oops. I missed that. Could you please sign all the commits and we should be able to see your username for the signed GPG key.

@CamilaRivera
Copy link
Contributor Author

Oops. I missed that. Could you please sign all the commits and we should be able to see your username for the signed GPG key.

Oops, I forgot about that. Would you recommend just doing a fresh new branch/PR? or try to amend the commits to add the signature?

@parasharrajat
Copy link
Member

You can just force push and rebase the commits. That should be less time-consuming.

CamilaRivera and others added 9 commits December 29, 2021 15:07
Add autoCapitalize value none for WorkspaceNewRoomPage
Co-authored-by: Rajat Parashar <[email protected]>
Co-authored-by: Rajat Parashar <[email protected]>
Co-authored-by: Rajat Parashar <[email protected]>
Co-authored-by: Rajat Parashar <[email protected]>
Co-authored-by: Rajat Parashar <[email protected]>
Co-authored-by: Rajat Parashar <[email protected]>
@CamilaRivera CamilaRivera force-pushed the camilarivera-autocapitalize-prop branch from eca8bb0 to a79ad5b Compare December 29, 2021 18:08
@CamilaRivera
Copy link
Contributor Author

You can just force push and rebase the commits. That should be less time-consuming.

Done! Thanks @parasharrajat

Copy link
Member

@parasharrajat parasharrajat left a comment

Choose a reason for hiding this comment

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

Thanks. LGTM

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.

Thank you both for such a great cooperation! Thank you @CamilaRivera for this fix 🙌

Merging :shipit:

@mountiny mountiny merged commit 8942314 into Expensify:main Dec 29, 2021
@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.

@OSBotify
Copy link
Contributor

OSBotify commented Jan 4, 2022

🚀 Deployed to staging by @mountiny in version: 1.1.24-19 🚀

platform result
🤖 android 🤖 failure ❌
🖥 desktop 🖥 failure ❌
🍎 iOS 🍎 failure ❌
🕸 web 🕸 failure ❌

@OSBotify
Copy link
Contributor

OSBotify commented Jan 4, 2022

🚀 Deployed to production by @francoisl in version: 1.1.24-19 🚀

platform result
🤖 android 🤖 failure ❌
🖥 desktop 🖥 failure ❌
🍎 iOS 🍎 failure ❌
🕸 web 🕸 failure ❌

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