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

[HOLD for payment 2023-04-10] [$1000] The new date of birth field header has the capital ‘N’ in the third word unlike other areas of the application where every letter are small in Spanish #16403

Closed
6 tasks done
kavimuru opened this issue Mar 22, 2023 · 38 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review

Comments

@kavimuru
Copy link

kavimuru commented Mar 22, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Open staging dot on web chrome and change language to spanish
  2. First Go to profile. Security. Close account and see that the placeholder in spanish has the first letter of every word in small letters. Even in the workspaces area, it has the same.
  3. But now click on profile and personal details page and see that only the ‘Fecha de Nacimiento’ has the capital N letter. To maintain consistency with all the other areas of the application , the letter should be made small.

Expected Result:

In ‘Fecha de Nacimiento’, the N letter should be made small to maintain consistency with the other areas of the application

Actual Result:

In ‘Fecha de Nacimiento’, the first letter of the third word is in capital letter

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.2.88-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

spanisherror-2023-03-22_09.17.25.mp4

Untitled

Expensify/Expensify Issue URL:
Issue reported by: @priya-zha
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1679456578035209

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01e1c094a06078c01b
  • Upwork Job ID: 1638916762779299840
  • Last Price Increase: 2023-03-23
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Mar 22, 2023
@MelvinBot
Copy link

Triggered auto assignment to @trjExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@MelvinBot
Copy link

MelvinBot commented Mar 22, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@trjExpensify
Copy link
Contributor

I agree this would be lower case in English, but I'm checking in thread on whether there would be a reason the Spanish should word would be capitalised: https://expensify.slack.com/archives/C049HHMV9SM/p1679511712503149?thread_ts=1679456578.035209&cid=C049HHMV9SM

@trjExpensify
Copy link
Contributor

Okay, so I got confirmation that there's no reason Fecha de Nacimiento should be in upper case in Spanish, so we need to change that to: Fecha de nacimiento.

Additionally, the month in the picker is in lowercase. I.e febrero whereas it should be uppercase Febrero. Let's make sure to change both of these while we're here.

There's also another in Editar Foto which should be Editar foto.

@trjExpensify trjExpensify added the External Added to denote the issue can be worked on by a contributor label Mar 23, 2023
@melvin-bot melvin-bot bot changed the title The new date of birth field header has the capital ‘N’ in the third word unlike other areas of the application where every letter are small in Spanish [$1000] The new date of birth field header has the capital ‘N’ in the third word unlike other areas of the application where every letter are small in Spanish Mar 23, 2023
@MelvinBot
Copy link

Job added to Upwork: https://www.upwork.com/jobs/~01e1c094a06078c01b

@MelvinBot
Copy link

Current assignee @trjExpensify is eligible for the External assigner, not assigning anyone new.

@MelvinBot
Copy link

Triggered auto assignment to Contributor-plus team member for initial proposal review - @thesahindia (External)

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 23, 2023
@MelvinBot
Copy link

Triggered auto assignment to @luacmartins (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@ayazhussain79
Copy link
Contributor

ayazhussain79 commented Mar 23, 2023

Proposal
Please re-state the problem that we are trying to solve in this issue.

  1. The header for the "date of birth" field has the letter 'N' capitalized in the third word, while all other letters are lowercase.
  2. The letter of the days in Spanish is not capitalized.
  3. The first letter of the month name in Spanish is not capitalized.
  4. The header for the "Edit photo" feature, which is translated to "Editar foto" in Spanish, has the letter 'F' capitalized.
  5. The header "One Moment" is capitalized in both English and Spanish translations, which is inconsistent with the rest of the application where only the first letter of the first word is capitalized.
  6. The header for the "postal code" field, which is translated to "Código Postal" in Spanish, has the letter 'P' capitalized, while it is lowercase in all other areas of the application.

What is the root cause of that problem?

  1. The letter 'N' is capitalized in the Spanish translation for the "date of birth" field header.
  2. The moment.js library returns month names in lowercase, causing the first letter of the month name to be lowercase in the Spanish translation.
  3. The moment.js library all returns days in lowercase, causing the letter of the days to be lowercase in the Spanish translation.
  4. The letter 'F' is capitalized in the Spanish translation for the "Edit photo" header.
  5. The word "Moment" is capitalized in both English and Spanish translations.
  6. The letter 'P' is capitalized in the Spanish translation for the "postal code" field header, while it is lowercase elsewhere in the application.

What changes do you think we should make in order to solve the problem?

  1. Update the "es.js" file to correct the capitalization of the letter 'N' to lowercase in the "date of birth" field header.
    Specifically, change line 58 from "dob: 'Fecha de Nacimiento'" to "dob: 'Fecha de nacimiento'".
    This change will ensure consistency in the application and resolve the issue.

  2. The code for the moment.js returns month names in lowercase so we need to modified this
    code this.monthNames = moment.localeData(props.preferredLocale).months(); in Index.js
    to this:

this.monthNames = moment.localeData(props.preferredLocale).months().map(month => month.replace(/^\w/, c => c.toUpperCase()));

or

this.monthNames = moment.localeData(props.preferredLocale).months().map(month => month.charAt(0).toUpperCase() + month.slice(1));

  1. After the month line for days use this line:
    this.daysOfWeek = moment.localeData(props.preferredLocale).weekdays().map(day => day.toUpperCase());

  2. The letter 'F' in the "Editar foto" header in the Spanish translation should be changed to lowercase.
    Code:

avatarCropModal: {
142 title: 'Editar Foto', //change this line and add small f
description: 'Arrastra, haz zoom y rota tu imagen para que quede como te gusta.',

  1. The capitalization of the Header "One Moment" should be made consistent throughout the application,
    by either capitalizing both letters or keeping both letters lowercase.

Line 914: oneMoment: 'One moment',
and in other file
oneMoment: 'Un momento',

  1. The letter 'P' in the "Código Postal" header in the Spanish translation should be changed to lowercase,
    On en.js file in line 72: zipPostCode: 'Código Postal',

@daraksha-dk
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

The new date of birth field header has the capital ‘N’ in the third word unlike other areas of the application where every letter are small in Spanish

What is the root cause of that problem?

This is an improvement or correction.

What changes do you think we should make in order to solve the problem?

We just need to update our spanish translation to have the correct version as per expected behaviour

@Prince-Mendiratta
Copy link
Contributor

Prince-Mendiratta commented Mar 23, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

There are instances of incorrect casing of text in spanish.

What is the root cause of that problem?

The keys were probably not verified correctly.
EDIT Although month and days name are lowercase in spanish, @iwiznia says here since they are names, they should be in Sentence case.

What changes do you think we should make in order to solve the problem?

We need to change the es.js here -

dob: 'Fecha de Nacimiento',

title: 'Editar Foto',

oneMoment: 'One Moment',

zipPostCode: 'Código Postal',

Next, we need to make changes to capitalise the first letter for all month names here:

{this.monthNames[currentMonthView]}

This can be done using {this.monthNames[currentMonthView].replace(/^./, str => str.toUpperCase())}. If we want, we can have conditions to only run the capitalization when the locale is es.

OR

We could iterate the array containing the months and capitalize them there altogether, removing the need to capitalize at each render

this.monthNames = moment.localeData(props.preferredLocale).months();

The same needs to be done for days of the week.
this.daysOfWeek = moment.localeData(props.preferredLocale).weekdays();

@bernhardoj
Copy link
Contributor

bernhardoj commented Mar 23, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

There are some texts in Spanish/English with inconsistent capitalization.

What is the root cause of that problem?

The text inside the es/en.js language file has some character in capital which should be in lowercase and specifically for the month text, the localization comes from moment library.

What changes do you think we should make in order to solve the problem?

We should change 4 texts inside es.js.

  1. common.dob from 'Fecha de Nacimiento' to 'Fecha de nacimiento'
  2. avatarCropModal.title from 'Editar Foto' to 'Editar foto'
  3. reimbursementAccountLoadingAnimation.oneMoment from 'Un Momento' to 'Un momento'
  4. common.zipPostCode from 'Código Postal' to 'Código postal'

and 1 text inside en.js.

  1. reimbursementAccountLoadingAnimation.oneMoment from 'One Moment' to 'One moment'

For the month/days of week text, it comes from moment. I read some and found that months and days of week in Spanish is in lowercase (ref). I suggest ignoring it.

Some more reference:
https://meta.wikimedia.org/wiki/Capitalization_of_Wiktionary_pages#Capitalization_of_month_names
https://stackoverflow.com/a/35927745/11488790

@jaybarnes33
Copy link

Proposal

Please re-state the problem that we are trying to solve in this issue.
Some words are wrongfully capitalised when the language is set to spanish. eg. (Fecha de Nacimiento instead of Fecha de nacimiento, Editar Foto instead of Editar foto, febrero instead of Febrero)

What is the root cause of that problem?

The issue is caused by wrong capitalisation of words in
App/src/languages/es.js, App/src/languages/es.js and also wrong casing of months in the App/src/components/CalendarPicker/index.js

What changes do you think we should make in order to solve the problem?

We need to change the wrongfully capitalised letters in App/src/languages/es.js and for the month field, we can add a style to convert to capitalise it. This will ensure that this doesn't happen again.

@MelvinBot
Copy link

📣 @jaybarnes33! 📣

Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.

Screen Shot 2022-11-16 at 4 42 54 PM

Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@jaybarnes33
Copy link

Contributor details
Your Expensify account email: ohenesetwumasi@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~0124a5d95e19ad6610

@MelvinBot
Copy link

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@trjExpensify
Copy link
Contributor

trjExpensify commented Mar 23, 2023

@thesahindia, @ayazhussain79 has a few more places this is going to need updating that we've talked about in thread. He's going to edit his comment above so they are documented in this issue as well.

@Prince-Mendiratta
Copy link
Contributor

Proposal updated as per latest discussions.

@ayazhussain79
Copy link
Contributor

@thesahindia Sorry for the proposal design actually I'm not good at it.

@bernhardoj
Copy link
Contributor

Proposal updated.

@thesahindia
Copy link
Member

@ayazhussain79's proposal looks good to me!

C+ reviewed 🎀👀🎀

cc: @luacmartins

@melvin-bot melvin-bot bot added the Reviewing Has a PR in review label Mar 25, 2023
@trjExpensify
Copy link
Contributor

Hm, strange. Why didn't it add @luacmartins and @thesahindia to the PR automatically? Done that. 👍

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels Apr 3, 2023
@melvin-bot melvin-bot bot changed the title [$1000] The new date of birth field header has the capital ‘N’ in the third word unlike other areas of the application where every letter are small in Spanish [HOLD for payment 2023-04-10] [$1000] The new date of birth field header has the capital ‘N’ in the third word unlike other areas of the application where every letter are small in Spanish Apr 3, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Apr 3, 2023
@MelvinBot
Copy link

Reviewing label has been removed, please complete the "BugZero Checklist".

@MelvinBot
Copy link

MelvinBot commented Apr 3, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.93-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-04-10. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@MelvinBot
Copy link

MelvinBot commented Apr 3, 2023

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@thesahindia / @luacmartins] The PR that introduced the bug has been identified. Link to the PR:
  • [@thesahindia / @luacmartins] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@thesahindia / @luacmartins] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to improvement: Update checklist item #17407
  • [@trjExpensify] Determine if we should create a regression test for this bug.
  • [@thesahindia] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
  • [@trjExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@luacmartins
Copy link
Contributor

@thesahindia would you mind completing the checklist above?

@thesahindia
Copy link
Member

This was caused by multiple PRs during the implementation. I think we should edit our current checklist item to add more focus on proper capitalization -

- I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for - Copy label for a copy review on the original GH to get the correct copy.
+ I verified any copy / text that was added to the app is grammatically correct in English, and it adheres to proper 
+ capitalization guidelines, and is approved by marketing by adding the Waiting for Copy label for a copy review on the 
+ original GH to get the correct copy.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 Daily KSv2 labels Apr 10, 2023
@trjExpensify
Copy link
Contributor

trjExpensify commented Apr 11, 2023

I think that's a good idea, @thesahindia. Do we have a link to somewhere that outlines the capitalisation guidelines to follow?

@trjExpensify
Copy link
Contributor

trjExpensify commented Apr 11, 2023

This was caused by multiple PRs during the implementation.

I also agree with this btw, there are a bunch and we can just catch it better in the PR checklist. I also don't think we need to add a regression test specific to capitalisation, so I'm checking them off.

As for payment then, that's due now. I've sent the following offers on the Upwork job:

$1,000 to @ayazhussain79 for the fix
$1,000 to @thesahindia for C+
$250 to @priya-zha for the bug report

@thesahindia
Copy link
Member

I think that's a good idea, @thesahindia. Do we have a link to somewhere that outlines the capitalisation guidelines to follow?

No we don't have a link. But I just found this article. I think it's good.

@trjExpensify
Copy link
Contributor

Ah, I meant in like our style guide here or something we can point to. Perhaps it's worth adding it to there?

@trjExpensify
Copy link
Contributor

I think someone might be working on a broader design/style guide actually, so for now @thesahindia what about just pointing out where we're seeing the problem the most?

I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.

@thesahindia
Copy link
Member

I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.

Yeah, let's just use this. It looks good to me!

@trjExpensify
Copy link
Contributor

trjExpensify commented Apr 13, 2023 via email

@thesahindia thesahindia mentioned this issue Apr 13, 2023
56 tasks
@melvin-bot melvin-bot bot added the Reviewing Has a PR in review label Apr 13, 2023
@thesahindia
Copy link
Member

@trjExpensify, I have raised the PR

@trjExpensify
Copy link
Contributor

Awesome. Everyone has been settled up with, closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

10 participants