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.

Sign up for GitHub

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

Tweak a localization to remove any assumption on "+" localization #19473

Merged
merged 4 commits into from
Oct 18, 2022
+70 −14
Merged

Tweak a localization to remove any assumption on "+" localization #19473

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7e7210d
Tweak a localization to remove any assumption on "+" localization
mokagio Oct 17, 2022
daec387
Update strings for localization
mokagio Oct 17, 2022
cd7b131
Refine `differenceLabel` implementation and add tests
mokagio Oct 18, 2022
ede3b8e
Update strings for localization
mokagio Oct 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 26 additions & 11 deletions WordPress/Classes/ViewRelated/Stats/Insights/ViewsVisitors/ViewsVisitorsLineChartCell.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,30 +57,45 @@ struct StatsSegmentedControlData {
// We want to show something like "+1.2K (5%)" if we have a percentage difference and "1.2K" if we don't.
// Because localized strings need to be strings literal, we cannot embed any conditional logic in the `localizedString...` call.
// We therefore need to generate different string literals based on the state.
let differenceSign = difference < 0 ? "" : "+"
if differencePercent != 0 {
let stringFormat = NSLocalizedString(
"insights.visitorsLineChartCell.differenceLabelWithPercentage",
value: "%@%@ (%@%%)",
switch (difference > 0, differencePercent != 0) {
case (true, true): // E.g.: +1.2k (5%)
stringFormat = NSLocalizedString(
"insights.visitorsLineChartCell.differenceLabelPosiviteWithPercentage",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"insights.visitorsLineChartCell.differenceLabelPosiviteWithPercentage",
"insights.visitorsLineChartCell.differenceLabelPositiveWithPercentage",

value: "+%@ (%@%%)",
comment: "Difference label for Insights Overview stat, indicating change from previous period, including percentage value. Example: +99.9K (5%)"
)
return String.localizedStringWithFormat(
stringFormat,
differenceSign,
difference.abbreviatedString(),
differencePercent.abbreviatedString()
)
} else {
let stringFormat = NSLocalizedString(
"insights.visitorsLineChartCell.differenceLabelWithoutPercentage",
value: "%@%@",
case (true, false): // E.g.: +1.2k
// We cannot assume every locale would translate an English string like "+1.2k" in the same way.
// So, even though we have only a "+" prefix, we ought to make this string localized.
stringFormat = NSLocalizedString(
"insights.visitorsLineChartCell.differenceLabelPosiviteWithoutPercentage",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"insights.visitorsLineChartCell.differenceLabelPosiviteWithoutPercentage",
"insights.visitorsLineChartCell.differenceLabelPositiveWithoutPercentage",

value: "+%@",
comment: "Difference label for Insights Overview stat, indicating change from previous period. Example: +99.9K"
)
return String.localizedStringWithFormat(
stringFormat,
differenceSign,
difference.abbreviatedString()
)
case (false, true): // E.g. 1.2k (5%)
Copy link
Contributor

Choose a reason for hiding this comment

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

So yes, if we want to be explicit about the localization negative case, then we should handle cases when difference == 0 and difference < 0 differently:

  1. difference == 0 no sign
  2. difference < 0 negative sign in a localization, pass abs(difference).abbreviatedString()

stringFormat = NSLocalizedString(
"insights.visitorsLineChartCell.differenceLabelNotPosiviteWithPercentage",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"insights.visitorsLineChartCell.differenceLabelNotPosiviteWithPercentage",
"insights.visitorsLineChartCell.differenceLabelNonPositiveWithPercentage",

Or (depending on @staskus 's reply to your answer in the PR description about why we don't use negative sign in the copy):

Suggested change
"insights.visitorsLineChartCell.differenceLabelNotPosiviteWithPercentage",
"insights.visitorsLineChartCell.differenceLabelNegativeWithPercentage",

value: "%@ (%@%%)",
comment: "Difference label for Insights Overview stat, indicating change from previous period, including percentage value, when the change is 0 or less. Example: 99.9K (5%)"
)
return String.localizedStringWithFormat(
stringFormat,
difference.abbreviatedString(),
differencePercent.abbreviatedString()
)
break
case (false, false): // E.g.: 1.2k
// There's no + sign nor percentage value here, we don't need to add any localization treatment.
return difference.abbreviatedString()
}
}

Expand Down
7 changes: 5 additions & 2 deletions WordPress/Resources/en.lproj/Localizable.strings
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3984,11 +3984,14 @@
/* Title displayed on the feature introduction view that announces the updated Stats Insight screen. */
"Insights update" = "Insights update";

/* Difference label for Insights Overview stat, indicating change from previous period, including percentage value, when the change is 0 or less. Example: 99.9K (5%) */
"insights.visitorsLineChartCell.differenceLabelNotPosiviteWithPercentage" = "%1$@ (%2$@%%)";

/* Difference label for Insights Overview stat, indicating change from previous period. Example: +99.9K */
"insights.visitorsLineChartCell.differenceLabelWithoutPercentage" = "%1$@%2$@";
"insights.visitorsLineChartCell.differenceLabelPosiviteWithoutPercentage" = "+%@";

/* Difference label for Insights Overview stat, indicating change from previous period, including percentage value. Example: +99.9K (5%) */
"insights.visitorsLineChartCell.differenceLabelWithPercentage" = "%1$@%2$@ (%3$@%%)";
"insights.visitorsLineChartCell.differenceLabelPosiviteWithPercentage" = "+%1$@ (%2$@%%)";

/* Button label to install a plugin
Confirmation button displayd in alert displayed when user installs their first plugin. */
Expand Down