Improve Insight differenceLabel localization logic (#19473)

wordpress-mobile · Oct 18, 2022 · 611dd09 · 611dd09
2 parents 47c1500 + ede3b8e
commit 611dd09
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 14 deletions.
diff --git a/WordPress/Classes/ViewRelated/Stats/Insights/ViewsVisitors/ViewsVisitorsLineChartCell.swift b/WordPress/Classes/ViewRelated/Stats/Insights/ViewsVisitors/ViewsVisitorsLineChartCell.swift
@@ -54,31 +54,35 @@ struct StatsSegmentedControlData {
     }
 
     var differenceLabel: String {
-        // 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 ? "" : "+"
+        // We want to show something like "+10.2K (+5%)" if we have a percentage difference and "1.2K" if we don't.
+        //
+        // Negative cases automatically appear with a negative sign "-10.2K (-5%)" by using `abbreviatedString()`.
+        // `abbreviatedString()` also handles formatting big numbers, i.e. 10,200 will become 10.2K.
+        let formatter = NumberFormatter()
+        formatter.locale = .current
+        let plusSign = difference <= 0 ? "" : "\(formatter.plusSign ?? "")"
+
         if differencePercent != 0 {
             let stringFormat = NSLocalizedString(
                 "insights.visitorsLineChartCell.differenceLabelWithPercentage",
-                value: "%@%@ (%@%%)",
-                comment: "Difference label for Insights Overview stat, indicating change from previous period, including percentage value. Example: +99.9K (5%)"
+                value: "%1$@%2$@ (%3$@%%)",
+                comment: "Text for the Insights Overview stat difference label. Shows the change from the previous period, including the percentage value. E.g.: +12.3K (5%). %1$@ is the placeholder for the change sign ('-', '+', or none). %2$@ is the placeholder for the change numerical value. %3$@ is the placeholder for the change percentage value, excluding the % sign."
             )
             return String.localizedStringWithFormat(
                 stringFormat,
-                differenceSign,
+                plusSign,
                 difference.abbreviatedString(),
                 differencePercent.abbreviatedString()
             )
         } else {
             let stringFormat = NSLocalizedString(
                 "insights.visitorsLineChartCell.differenceLabelWithoutPercentage",
-                value: "%@%@",
-                comment: "Difference label for Insights Overview stat, indicating change from previous period. Example: +99.9K"
+                value: "%1$@%2$@",
+                comment: "Text for the Insights Overview stat difference label. Shows the change from the previous period. E.g.: +12.3K. %1$@ is the placeholder for the change sign ('-', '+', or none). %2$@ is the placeholder for the change numerical value."
             )
             return String.localizedStringWithFormat(
                 stringFormat,
-                differenceSign,
+                plusSign,
                 difference.abbreviatedString()
             )
         }

diff --git a/WordPress/Resources/en.lproj/Localizable.strings b/WordPress/Resources/en.lproj/Localizable.strings
@@ -3984,10 +3984,10 @@
 /* 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. Example: +99.9K */
+/* Text for the Insights Overview stat difference label. Shows the change from the previous period. E.g.: +12.3K. %1$@ is the placeholder for the change sign ('-', '+', or none). %2$@ is the placeholder for the change numerical value. */
 "insights.visitorsLineChartCell.differenceLabelWithoutPercentage" = "%1$@%2$@";
 
-/* Difference label for Insights Overview stat, indicating change from previous period, including percentage value. Example: +99.9K (5%) */
+/* Text for the Insights Overview stat difference label. Shows the change from the previous period, including the percentage value. E.g.: +12.3K (5%). %1$@ is the placeholder for the change sign ('-', '+', or none). %2$@ is the placeholder for the change numerical value. %3$@ is the placeholder for the change percentage value, excluding the % sign. */
 "insights.visitorsLineChartCell.differenceLabelWithPercentage" = "%1$@%2$@ (%3$@%%)";
 
 /* Button label to install a plugin

diff --git a/WordPress/WordPress.xcodeproj/project.pbxproj b/WordPress/WordPress.xcodeproj/project.pbxproj
@@ -748,6 +748,7 @@
 		3FEC241525D73E8B007AFE63 /* ConfettiView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3FEC241425D73E8B007AFE63 /* ConfettiView.swift */; };
 		3FF1A853242D5FCB00373F5D /* WPTabBarController+ReaderTabNavigation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3FF1A852242D5FCB00373F5D /* WPTabBarController+ReaderTabNavigation.swift */; };
 		3FFA5ED22876152E00830E28 /* JetpackButton.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3FFA5ED12876152E00830E28 /* JetpackButton.swift */; };
+		3FFE3C0828FE00D10021BB96 /* StatsSegmentedControlDataTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3FFE3C0728FE00D10021BB96 /* StatsSegmentedControlDataTests.swift */; };
 		400199AB222590E100EB0906 /* AllTimeStatsRecordValueTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 400199AA222590E100EB0906 /* AllTimeStatsRecordValueTests.swift */; };
 		400199AD22259FF300EB0906 /* AnnualAndMostPopularTimeStatsRecordValueTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 400199AC22259FF300EB0906 /* AnnualAndMostPopularTimeStatsRecordValueTests.swift */; };
 		400A2C772217A8A0000A8A59 /* VisitsSummaryStatsRecordValue+CoreDataClass.swift in Sources */ = {isa = PBXBuildFile; fileRef = 400A2C752217A8A0000A8A59 /* VisitsSummaryStatsRecordValue+CoreDataClass.swift */; };
@@ -6091,6 +6092,7 @@
 		3FEC241425D73E8B007AFE63 /* ConfettiView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ConfettiView.swift; sourceTree = "<group>"; };
 		3FF1A852242D5FCB00373F5D /* WPTabBarController+ReaderTabNavigation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "WPTabBarController+ReaderTabNavigation.swift"; sourceTree = "<group>"; };
 		3FFA5ED12876152E00830E28 /* JetpackButton.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = JetpackButton.swift; sourceTree = "<group>"; };
+		3FFE3C0728FE00D10021BB96 /* StatsSegmentedControlDataTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StatsSegmentedControlDataTests.swift; sourceTree = "<group>"; };
 		400199AA222590E100EB0906 /* AllTimeStatsRecordValueTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AllTimeStatsRecordValueTests.swift; sourceTree = "<group>"; };
 		400199AC22259FF300EB0906 /* AnnualAndMostPopularTimeStatsRecordValueTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AnnualAndMostPopularTimeStatsRecordValueTests.swift; sourceTree = "<group>"; };
 		400A2C752217A8A0000A8A59 /* VisitsSummaryStatsRecordValue+CoreDataClass.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "VisitsSummaryStatsRecordValue+CoreDataClass.swift"; sourceTree = "<group>"; };
@@ -10957,9 +10959,12 @@
 				40F50B7F221310D400CBBB73 /* FollowersStatsRecordValueTests.swift */,
 				40E7FEC72211EEC00032834E /* LastPostStatsRecordValueTests.swift */,
 				40EE948122132F5800CD264F /* PublicizeConectionStatsRecordValueTests.swift */,
+				938466B82683CA0E00A538DC /* ReferrerDetailsViewModelTests.swift */,
 				400A2C922217B463000A8A59 /* ReferrerStatsRecordValueTests.swift */,
 				40C403ED2215CE9500E8C894 /* SearchResultsStatsRecordValueTests.swift */,
+				3FDDFE9527C8178C00606933 /* SiteStatsInformationTests.swift */,
 				40E7FEC42211DF790032834E /* StatsRecordTests.swift */,
+				3FFE3C0728FE00D10021BB96 /* StatsSegmentedControlDataTests.swift */,
 				40F50B81221310F000CBBB73 /* StatsTestCase.swift */,
 				931215E0267DE1C0008C3B69 /* StatsTotalRowDataTests.swift */,
 				4054F4632214F94D00D261AB /* StreakStatsRecordValueTests.swift */,
@@ -10969,8 +10974,6 @@
 				40C403F72215D88100E8C894 /* TopViewedStatsTests.swift */,
 				400A2C942217B68D000A8A59 /* TopViewedVideoStatsRecordValueTests.swift */,
 				400A2C962217B883000A8A59 /* VisitsSummaryStatsRecordValueTests.swift */,
-				938466B82683CA0E00A538DC /* ReferrerDetailsViewModelTests.swift */,
-				3FDDFE9527C8178C00606933 /* SiteStatsInformationTests.swift */,
 			);
 			name = Stats;
 			sourceTree = "<group>";
@@ -22204,6 +22207,7 @@
 				C738CB0F28626466001BE107 /* QRLoginScanningCoordinatorTests.swift in Sources */,
 				8B6214E627B1B446001DF7B6 /* BlogDashboardServiceTests.swift in Sources */,
 				C856749A243F4292001A995E /* TenorMockDataHelper.swift in Sources */,
+				3FFE3C0828FE00D10021BB96 /* StatsSegmentedControlDataTests.swift in Sources */,
 				D81C2F5820F86CEA002AE1F1 /* NetworkStatus.swift in Sources */,
 				E1C545801C6C79BB001CEB0E /* MediaSettingsTests.swift in Sources */,
 				C3439B5F27FE3A3C0058DA55 /* SiteCreationWizardLauncherTests.swift in Sources */,

diff --git a/WordPress/WordPressTest/StatsSegmentedControlDataTests.swift b/WordPress/WordPressTest/StatsSegmentedControlDataTests.swift
@@ -0,0 +1,48 @@
+import Nimble
+import XCTest
+@testable import WordPress
+
+class StatsSegmentedControlDataTests: XCTestCase {
+
+    func testDifferenceLabel() {
+        expect(StatsSegmentedControlData.fixture(difference: -12_345, differencePercent: -1).differenceLabel)
+            == "-12.3K (-1%)"
+        expect(StatsSegmentedControlData.fixture(difference: -12_345, differencePercent: 0).differenceLabel)
+            == "-12.3K"
+        expect(StatsSegmentedControlData.fixture(difference: -12_345, differencePercent: 1).differenceLabel)
+            == "-12.3K (1%)"
+        expect(StatsSegmentedControlData.fixture(difference: 0, differencePercent: -1).differenceLabel)
+            == "0 (-1%)"
+        expect(StatsSegmentedControlData.fixture(difference: 0, differencePercent: 0).differenceLabel)
+            == "0"
+        expect(StatsSegmentedControlData.fixture(difference: 0, differencePercent: 1).differenceLabel)
+            == "0 (1%)"
+        expect(StatsSegmentedControlData.fixture(difference: 12_345, differencePercent: -1).differenceLabel)
+            == "+12.3K (-1%)"
+        expect(StatsSegmentedControlData.fixture(difference: 12_345, differencePercent: 0).differenceLabel)
+            == "+12.3K"
+        expect(StatsSegmentedControlData.fixture(difference: 12_345, differencePercent: 1).differenceLabel)
+            == "+12.3K (1%)"
+    }
+}
+
+extension StatsSegmentedControlData {
+
+    static func fixture(
+        segmentTitle: String = "title",
+        segmentData: Int = 0,
+        segmentPrevData: Int = 1,
+        difference: Int = 2,
+        differenceText: String = "text",
+        differencePercent: Int = 3
+    ) -> StatsSegmentedControlData {
+        StatsSegmentedControlData(
+            segmentTitle: segmentTitle,
+            segmentData: segmentData,
+            segmentPrevData: segmentPrevData,
+            difference: difference,
+            differenceText: differenceText,
+            differencePercent: differencePercent
+        )
+    }
+}