Repository navigation
Fix: Expensify Card - issued starts with a lowercase i on shipping address message - #51941
nyomanjyotisa wants to merge 1 commit into
Conversation
| issuedCard: ({assignee}: AssigneeParams) => `¡emitió a ${assignee} una Tarjeta Expensify! La tarjeta llegará en 2-3 días laborables.`, | ||
| issuedCardNoShippingDetails: ({assignee}: AssigneeParams) => `¡emitió a ${assignee} una Tarjeta Expensify! La tarjeta se enviará una vez que se agreguen los detalles de envío.`, | ||
| issuedCardVirtual: ({assignee, link}: IssueVirtualCardParams) => `¡emitió a ${assignee} una ${link} virtual! La tarjeta puede utilizarse inmediatamente.`, | ||
| issuedCard: ({assignee}: AssigneeParams) => `¡Emitió a ${assignee} una Tarjeta Expensify! La tarjeta llegará en 2-3 días laborables.`, |
There was a problem hiding this comment.
Also pinging the original feature implementators for their feedback.
Do you think the starting character i in Spanish translation is an oversight?
cc @koko57 @allgandalf
There was a problem hiding this comment.
I think it is intentional. The symbol "¡" is an inverted exclamation mark, used in Spanish to indicate the beginning of an exclamatory sentence or phrase
There was a problem hiding this comment.
I think it is intentional. The symbol "¡" is an inverted exclamation mark, used in Spanish to indicate the beginning of an exclamatory sentence or phrase
Oh ok. That's interesting. Let's proceed then if it's intentional.
There was a problem hiding this comment.
TBH I don't think it's a bug as I see some other cases that we do not start with an uppercased letter, yet we omit the action's actor name
so the message is kind of a logical continuation: [User] [did something]
for the exclamation mark - not a Spanish expert, but AFAIK it can be either at the beginning of the sentence or just a clause, but now I'm not sure if the scheme [User] ¡[did something]! is correct. Looks like it should be before the user name, but it's not possible in this case.
I think it should be consulted with the design team and Spanish translation should be approved
There was a problem hiding this comment.
Typically it has been no capital at the beginning of any system messages. But if the system message is more than one sentence, then you would capitalize the beginning of the second sentence. Does that make sense?
There was a problem hiding this comment.
Typically it has been no capital at the beginning of any system messages.
Right, because system messages are written with the user's name as the subject of the sentence.
James Dean
did something to the expense.
James Dean
did something to the expense. Previously it was different.
There was a problem hiding this comment.
Ah cool, so then I'm misremembering that convo and Shawn's suggestion is correct. 👍
There was a problem hiding this comment.
for system messages like this, we do not capitalize the first letter
Ok. So that means that the changes in this PR i.e. i to I is not required. cc @robertjchen
There was a problem hiding this comment.
Got it, let's close this out then 👍
Reviewer Checklist
Screenshots/VideosMacOS: Chrome / Safari51941-web-safari-001.mp4Android: Native51941-android-native-001.mp4iOS: mWeb Safari51941-mweb-safari-001.mp4Android: mWeb Chrome51941-mweb-chrome-001.mp4iOS: Native51941-ios-native-001.mp4MacOS: Desktop51941-desktop-001.mp4 |
rojiphil
left a comment
rojiphil
left a comment
There was a problem hiding this comment.
@robertjchen Changes LGTM and works well too.
There is a leading i character in Spanish translation as mentioned here but looks like it’s intentional.
Over to you review. Thanks.
robertjchen
left a comment
robertjchen
left a comment
There was a problem hiding this comment.
LGTM, let's see what Design says about the Spanish copy and proper placement of the exclamation mark 🤔
That's an exclamation point and it appears to be intentional. |
Ok. Got it. That brings us to the only pending query from comment here as mentioned below:
As this needs feedback from the Spanish translation team, I have posted it in slack here |
|
I would remove the exclamation marks so it matches the English version |
|
Sorry, even if it doesn't match the English version exactly, for the Spanish copy I would remove But just to be sure, can you confirm how all the copies (English/Spanish) would look and I can do one final review? Thanks! |
|
Since this PR is closed, we can take this up in the issue itself here |
Explanation of Change
Fixed Issues
$ #51386
PROPOSAL: #51386 (comment)
Tests
Same as QA Test
Offline tests
Same as QA Steps
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android-Native.mp4
Android: mWeb Chrome
Android-mWeb.Chrome.mp4
iOS: Native
iOS-Native.mp4
iOS: mWeb Safari
iOS-mWeb.Safari.mp4
MacOS: Chrome / Safari
MacOS-Chrome.mp4
MacOS: Desktop
MacOS-Desktop.mp4