-
Notifications
You must be signed in to change notification settings - Fork 49
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
feat: add capitalize and titleCase method for i18n #168
base: main
Are you sure you want to change the base?
Conversation
@@ -21,7 +21,9 @@ const Overview = () => { | |||
}, | |||
{ | |||
key: 'karmada-status', | |||
label: i18nInstance.t('3fea7ca76cdece641436d7ab0d02ab1b', '状态'), | |||
label: capitalize( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we always use title case
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, for this case capitalize
and titleCase
method share the same effect, but titleCase
is more suitable.
Signed-off-by: warjiang <[email protected]>
…tleCase Signed-off-by: warjiang <[email protected]>
… switcher Signed-off-by: warjiang <[email protected]>
2075e7a
to
a645653
Compare
/assign @RainbowMango @samzong |
Signed-off-by: warjiang <[email protected]>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems unnecessary to handle it like this. recommended to correct the i18n text, otherwise, this part of the code logic will confuse users; Additionally, will there be any issues when concatenating prompt words?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes~. Only for some corner-case, there is need to use capitalize、titleCase、lowerCase kinds of method. For example, assume that we need to translate 概览
into overview
, for some cases, like the sidebard menu or card title, we need to use Overview
, for some other cases, like tag view, we need to use overview
in lowercase, but these kind of cases if not very common. So just change the translation dictionary firstly, for corner case, use format method. cc @RainbowMango
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel it is kind of redundant to add titleCase
to each term. I prefer to correct it at i18n text.
for some cases, like the sidebard menu or card title, we need to use Overview, for some other cases, like tag view, we need to use overview in lowercase, but these kind of cases if not very common.
Then define two terms to differentiate them.
What type of PR is this?
/kind feature
What this PR does / why we need it:
1. capitalize or titleCase for some special usages.
2. set english as default language.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: