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

Added translations for CompanyStep #4765

Merged
merged 5 commits into from
Aug 23, 2021

Conversation

akshayasalvi
Copy link
Contributor

@akshayasalvi akshayasalvi commented Aug 20, 2021

Details

Added missing translations for CompanyStep modal.

Fixed Issues

$ #4744

Tests

  1. Tested by switching language and checking the forms.
  2. Tested other strings in the same form. Couldn't find any other missing translations.

QA Steps

  1. Set your language to Spansih
  2. Navigate to /bank-account/company page
  3. Enter the details of the company form and keep one or two fields invalid, say Tax ID.
  4. Click on Submit & continue
  5. Modal with Spanish text should appear.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Screenshot 2021-08-20 at 7 07 13 PM

Mobile Web

Screenshot 2021-08-20 at 7 12 10 PM

Desktop

Screenshot 2021-08-20 at 8 44 39 PM

iOS

Screenshot 2021-08-20 at 8 59 40 PM

Android

Screenshot 2021-08-20 at 9 15 29 PM

@akshayasalvi akshayasalvi requested a review from a team as a code owner August 20, 2021 15:47
@MelvinBot MelvinBot requested review from iwiznia and removed request for a team August 20, 2021 15:47
@akshayasalvi
Copy link
Contributor Author

@iwiznia PR raised.

I also found one more problem due to which the form wasn't loading in Android. I've fixed that too. Please validate this commit too.

@@ -517,6 +517,9 @@ export default {
listOfRestrictedBusinesses: 'lista de negocios restringidos',
incorporationDatePlaceholder: 'Fecha de inicio (aaaa-mm-dd)',
companyPhonePlaceholder: '10 dígitos, sin guiones',
confirmModalTitle: '¿Está seguro?',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
confirmModalTitle: '¿Está seguro?',
confirmModalTitle: '¿Estás seguro?',

Copy link
Contributor

Choose a reason for hiding this comment

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

These changes are because we use the informal tu instead of the formal usted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed. Thanks for informing.

@@ -517,6 +517,9 @@ export default {
listOfRestrictedBusinesses: 'lista de negocios restringidos',
incorporationDatePlaceholder: 'Fecha de inicio (aaaa-mm-dd)',
companyPhonePlaceholder: '10 dígitos, sin guiones',
confirmModalTitle: '¿Está seguro?',
confirmModalPrompt: 'Por favor, compruebe los campos resaltados e inténtelo de nuevo.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
confirmModalPrompt: 'Por favor, compruebe los campos resaltados e inténtelo de nuevo.',
confirmModalPrompt: 'Por favor, comprueba los campos resaltados e inténtalo de nuevo.',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed

@@ -303,7 +303,7 @@ class CompanyStep extends React.Component {
: ''}
/>
<ExpensiTextInput
autoCompleteType="new-password"
autoCompleteType="password"
Copy link
Contributor

Choose a reason for hiding this comment

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

So this fixes android? What is new-password and password and what's the difference between them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For TextInput in Android, the autoCompleteType has to be one of the values listed in this link:

https://reactnative.dev/docs/textinput#autocompletetype-android

If you see password exists to denote the password field. It isn't an id/name field where we can put new-password

@@ -517,6 +517,9 @@ export default {
listOfRestrictedBusinesses: 'lista de negocios restringidos',
incorporationDatePlaceholder: 'Fecha de inicio (aaaa-mm-dd)',
companyPhonePlaceholder: '10 dígitos, sin guiones',
confirmModalTitle: '¿Está seguro?',
confirmModalPrompt: 'Por favor, compruebe los campos resaltados e inténtelo de nuevo.',
confirmModalConfirmText: 'Lo tengo',
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a literal translation, but this is not used in spanish, let's use:

Suggested change
confirmModalConfirmText: 'Lo tengo',
confirmModalConfirmText: 'OK',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed

@akshayasalvi
Copy link
Contributor Author

akshayasalvi commented Aug 20, 2021

@iwiznia PR updated

@akshayasalvi
Copy link
Contributor Author

@iwiznia thanks for approving. Any other changes pending to be merged?

@iwiznia
Copy link
Contributor

iwiznia commented Aug 23, 2021

No no, was waiting for the tests to finish.

@iwiznia iwiznia merged commit e246d7a into Expensify:main Aug 23, 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.

@akshayasalvi
Copy link
Contributor Author

Thank you

@isagoico
Copy link

@akshayasalvi Hello! Any QA tests needed for this PR?

@iwiznia
Copy link
Contributor

iwiznia commented Aug 26, 2021

I think @akshayasalvi edited the description to include them already.
@akshayasalvi next time please leave a comment.

@akshayasalvi
Copy link
Contributor Author

akshayasalvi commented Aug 26, 2021

Okay. I'll somehow missed putting a comment for this one. Thank you.

@Jag96 Jag96 mentioned this pull request Aug 26, 2021
5 tasks
@OSBotify
Copy link
Contributor

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

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

@botify
Copy link

botify commented Sep 1, 2021

This has been deployed to production and is now subject to a 7-day regression period.
If no regressions arise, payment will be issued on 2021-09-08. 🎊

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.

5 participants