-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Comments
Triggered auto assignment to @trjExpensify ( |
Bug0 Triage Checklist (Main S/O)
|
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 |
Okay, so I got confirmation that there's no reason Additionally, the month in the picker is in lowercase. I.e There's also another in |
Job added to Upwork: https://www.upwork.com/jobs/~01e1c094a06078c01b |
Current assignee @trjExpensify is eligible for the External assigner, not assigning anyone new. |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @thesahindia ( |
Triggered auto assignment to @luacmartins ( |
Proposal
What is the root cause of that problem?
What changes do you think we should make in order to solve the problem?
or
|
ProposalPlease 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 |
ProposalPlease 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. What changes do you think we should make in order to solve the problem?We need to change the Line 58 in 449fa75
Line 146 in 449fa75
Line 908 in ab3af32
Line 72 in ab3af32
Next, we need to make changes to capitalise the first letter for all month names here: App/src/components/CalendarPicker/index.js Line 113 in 63b96d3
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
The same needs to be done for days of the week.
|
ProposalPlease 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 What changes do you think we should make in order to solve the problem?We should change 4 texts inside
and 1 text inside
For the month/days of week text, it comes from Some more reference: |
ProposalPlease re-state the problem that we are trying to solve in this issue. What is the root cause of that problem?The issue is caused by wrong capitalisation of words in What changes do you think we should make in order to solve the problem?We need to change the wrongfully capitalised letters in |
📣 @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.
Format:
|
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
@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. |
Proposal updated as per latest discussions. |
@thesahindia Sorry for the proposal design actually I'm not good at it. |
Proposal updated. |
@ayazhussain79's proposal looks good to me! C+ reviewed 🎀👀🎀 cc: @luacmartins |
Hm, strange. Why didn't it add @luacmartins and @thesahindia to the PR automatically? Done that. 👍 |
|
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.
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
|
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 would you mind completing the checklist above? |
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.
|
I think that's a good idea, @thesahindia. Do we have a link to somewhere that outlines the capitalisation guidelines to follow? |
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 |
No we don't have a link. But I just found this article. I think it's good. |
Ah, I meant in like our style guide here or something we can point to. Perhaps it's worth adding it to there? |
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?
|
Yeah, let's just use this. It looks good to me! |
Cool, want to submit a PR?
On Thu, 13 Apr 2023 at 12:46, Sahil ***@***.***> wrote:
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!
—
Reply to this email directly, view it on GitHub
<#16403 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AD3246IIJNXTSYZUFJCTJMLXA7RQNANCNFSM6AAAAAAWEGAUWM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
*Tom Rhys Jones *
*Expensify*
|
@trjExpensify, I have raised the PR |
Awesome. Everyone has been settled up with, closing! |
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:
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?
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
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
The text was updated successfully, but these errors were encountered: