Skip to content

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

Merged
mountiny merged 9 commits into
Expensify:mainfrom
CamilaRivera:camilarivera-autocapitalize-prop
Dec 29, 2021
Merged

Add autoCapitalize prop to avoid first letter duplication on android#6916
mountiny merged 9 commits into
Expensify:mainfrom
CamilaRivera:camilarivera-autocapitalize-prop

Conversation

@CamilaRivera

@CamilaRivera CamilaRivera commented Dec 28, 2021

Copy link
Copy Markdown
Contributor

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
Comment thread src/components/TextInputWithPrefix/index.js Outdated
Comment thread src/components/TextInputWithPrefix/index.android.js Outdated
Comment thread src/components/TextInputWithPrefix/index.android.js Outdated
Comment thread src/components/TextInputWithPrefix/index.android.js Outdated
Comment thread src/components/TextInputWithLabel.js Outdated
Comment thread src/components/TextInputWithLabel.js Outdated
Comment thread src/components/TextInputWithPrefix/index.js Outdated

@mountiny mountiny left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Member

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

@parasharrajat parasharrajat left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
Contributor Author

LGTM.

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

@parasharrajat parasharrajat left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
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 <parasharrajat@users.noreply.github.com>
Co-authored-by: Rajat Parashar <parasharrajat@users.noreply.github.com>
Co-authored-by: Rajat Parashar <parasharrajat@users.noreply.github.com>
Co-authored-by: Rajat Parashar <parasharrajat@users.noreply.github.com>
Co-authored-by: Rajat Parashar <parasharrajat@users.noreply.github.com>
Co-authored-by: Rajat Parashar <parasharrajat@users.noreply.github.com>
Co-authored-by: Rajat Parashar <parasharrajat@users.noreply.github.com>
@CamilaRivera CamilaRivera force-pushed the camilarivera-autocapitalize-prop branch from eca8bb0 to a79ad5b Compare December 29, 2021 18:08
@CamilaRivera

Copy link
Copy Markdown
Contributor Author

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

Done! Thanks @parasharrajat

@parasharrajat parasharrajat left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks. LGTM

@mountiny mountiny left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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

OSBotify commented Jan 4, 2022

Copy link
Copy Markdown
Contributor

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

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

@OSBotify

OSBotify commented Jan 4, 2022

Copy link
Copy Markdown
Contributor

🚀 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