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

Update capitalization of copy to match new guidelines #4854

Merged
merged 4 commits into from
Aug 27, 2021

Conversation

iwiznia
Copy link
Contributor

@iwiznia iwiznia commented Aug 26, 2021

Fixed Issues

$https://github.com/Expensify/Expensify/issues/169061

Tests

Click around on the site both in spanish and english and make sure the capitalization of the copy is correct. We should only capitalize names, the beginning of sentences and acronyms.

QA Steps

Same, but please try to be on the lookout for any capitalization we missed

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

image
image
image

@iwiznia iwiznia requested a review from a team as a code owner August 26, 2021 15:39
@iwiznia iwiznia self-assigned this Aug 26, 2021
@MelvinBot MelvinBot requested review from luacmartins and removed request for a team August 26, 2021 15:39
src/languages/es.js Outdated Show resolved Hide resolved
src/languages/es.js Show resolved Hide resolved
Comment on lines +521 to +523
companyWebsite: 'Página web de la empresa',
taxIDNumber: 'Número de identificación fiscal',
companyType: 'Tipo de empresa',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These translations were wrong, so I also fixed them

@iwiznia iwiznia changed the title Update capitalzization of copy to match new guidelines Update capitlization of copy to match new guidelines Aug 26, 2021
@iwiznia iwiznia changed the title Update capitlization of copy to match new guidelines Update capitalization of copy to match new guidelines Aug 26, 2021
MonilBhavsar
MonilBhavsar previously approved these changes Aug 26, 2021
@MonilBhavsar
Copy link
Contributor

We want to merge with tests ongoing?

@iwiznia
Copy link
Contributor Author

iwiznia commented Aug 26, 2021

We want to merge with tests ongoing?

Nooooo, I can merge when tests are done.

Copy link
Contributor

@luacmartins luacmartins left a comment

Choose a reason for hiding this comment

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

Left a few suggestions. Great work @iwiznia! My eyes are very happy now 😄

src/languages/en.js Outdated Show resolved Hide resolved
src/languages/en.js Outdated Show resolved Hide resolved
src/languages/en.js Outdated Show resolved Hide resolved
src/languages/en.js Outdated Show resolved Hide resolved
src/languages/en.js Outdated Show resolved Hide resolved
src/languages/es.js Outdated Show resolved Hide resolved
src/languages/es.js Outdated Show resolved Hide resolved
src/languages/es.js Outdated Show resolved Hide resolved
src/languages/es.js Outdated Show resolved Hide resolved
src/languages/es.js Outdated Show resolved Hide resolved
Co-authored-by: Carlos Martins <cmartins@expensify.com>
@iwiznia
Copy link
Contributor Author

iwiznia commented Aug 26, 2021

Wow, so many I missed... updated, can you re-review please

luacmartins
luacmartins previously approved these changes Aug 26, 2021
Copy link
Contributor

@luacmartins luacmartins left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@iwiznia
Copy link
Contributor Author

iwiznia commented Aug 26, 2021

@MonilBhavsar merge at will

@luacmartins
Copy link
Contributor

Looks like the conflicts started already! 😭

@MonilBhavsar
Copy link
Contributor

Once conflicts are resolved, we need to merge it asap.

@iwiznia
Copy link
Contributor Author

iwiznia commented Aug 27, 2021

Updated

@iwiznia
Copy link
Contributor Author

iwiznia commented Aug 27, 2021

Going OOO next week, so if this conflicts again before you can merge it, can someone take care of resolving them and getting it merged please?

@MonilBhavsar MonilBhavsar merged commit bc0ca55 into main Aug 27, 2021
@MonilBhavsar MonilBhavsar deleted the ionatan_capitalization branch August 27, 2021 12:30
@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

🚀 Deployed to staging by @MonilBhavsar in version: 1.0.88-3 🚀

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

@OSBotify
Copy link
Contributor

OSBotify commented Sep 1, 2021

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

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

@puneetlath
Copy link
Contributor

FYI: we changed the Terms of Service back to capitalized here because it's a title, rather than a sentence.

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