Update more in-app Stats texts to always use western arabic numerals #17217

ovitrif · 2022-09-27T14:15:54Z

Fixes 20.8 beta-testing issues discovered on #17160 (which fixes partially #17002) [internal ref: p5T066-3Ak-p2#comment-13583]

Included fixes (see previews):

"Total"-type cards: (I.e. Total Likes, Total Comments): Stats → Insights → Total Likes
Dates in Line Chart: Stats → Insights → Views & Visitors
"Value change" text above bar chart: Stats → Days / Weeks / Months ...

To test:

1️⃣ Test Case 1 : Jetpack / Wordpress App Stats

Open the JP/WP App, Go to Me → App Settings → Interface Language and select Arabic.
Go back to My Site → tap Stats & explore the first tab (Insights)
- Expect all updated texts to use western arabic numbers (see preview 1 & 2).
Go to any other Stats tab (Days/Weeks/...)
- Expect The "value change" text to use western arabic numbers (see preview 3).

Previews

1. Text in "Total"-type cards

Before	After

2. Views & Visitors chart dates

Before	After

3. Value "change" text

Before	After

Regression Notes

Potential unintended areas of impact
Texts using text view component that forces western arabic numerals.
Updated texts for non-arabic languages.
What I did to test those areas of impact (or what existing automated tests I relied on)
Manual testing — the changes are rather trivial, just exposing a former private method public and reusing it.
In one instance (for the "value change" text) we're forcing LTR text direction — this text is expected to include only numbers and symbols.
What automated tests I added (or what prevented me from doing so)
N/a — PR consists of small UI changes.

PR submission checklist:

I have completed the Regression Notes.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

…date lines

AliSoftware · 2022-09-27T14:30:27Z

cc @ParaskP7 who will cover for me as Release Manager for WPAndroid while I'll be AFK the next 3 days. We might want to try to include that fix in the beta you planned on doing tomorrow (the one we usually do for L10n smoke-testing)

wpmobilebot · 2022-09-27T14:30:46Z

📲 You can test these changes on WordPress by downloading wordpress-installable-build-pr17217-32cf991.apk

	💡 Scan this QR code with your Android phone to download and install the APK directly on it.
	App	`WordPress`
	Build Flavor	`Jalapeno`
	Build Type	`Debug`
	Commit	`32cf991`

Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

wpmobilebot · 2022-09-27T14:33:39Z

📲 You can test these changes on Jetpack by downloading jetpack-installable-build-pr17217-32cf991.apk

	💡 Scan this QR code with your Android phone to download and install the APK directly on it.
	App	`Jetpack`
	Build Flavor	`Jalapeno`
	Build Type	`Debug`
	Commit	`32cf991`

Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

…' into fix/20.8-western-arabic-numerals # Conflicts: # WordPress/src/main/res/layout/stats_block_value_item.xml

ovitrif · 2022-09-27T14:46:48Z

cc @ParaskP7 who will cover for me as Release Manager for WPAndroid while I'll be AFK the next 3 days. We might want to try to include that fix in the beta you planned on doing tomorrow (the one we usually do for L10n smoke-testing)

As mentioned internally (p5T066-3Ak-p2#comment-13594) imho there's really no reason to push this to 20.8 unless we're sure this would not complicate the normal release flow 😄

WordPress/src/main/java/org/wordpress/android/util/extensions/StringExtensions.kt

ovitrif · 2022-09-30T11:30:44Z

Moved this PR to the next milestone: 21.0

WordPress/src/main/java/org/wordpress/android/util/extensions/StringExtensions.kt

antonis · 2022-10-11T13:32:13Z

WordPress/src/main/res/layout/stats_block_value_item.xml

-        android:visibility="gone" />
+        android:visibility="gone"
+        tools:text="-7 (-100%)"
+        android:layoutDirection="ltr"


This is not a request for a change. Just curious why we set the layout direction here 🤔

Thank you for raising the question @antonis 🙇

I noticed this view was placing the number sign inverted when using a RTL language, see screenshot:

This isn't the case in other places because we always have a prefix / suffix or the number is part of a longer string.

Upon further investigation it appears this happens only when the text of the view contains a number and any other characters except letters.

This seems to be the behavior of text views in RTL languages, though not consistent after we've introduced the changes to use western arabic numerals.

A few other explorations without setting android:layoutDirection="ltr":

Prefix word Suffix word

Conclusion: In order to maintain a consistent position of the number sign with the other places, this change was indeed needed 🙂.

At the time I was also using Wikipedia to check the topic and my understanding was that the negative sign is positioned to the right only for eastern arabic numerals.

antonis

Great work @ovitrif 👍
I've tested the app on a Pixel 5 (Android 13) and everything worked ass expected. The code also LGTM 🎉
A couple of notes but nothing blocking for this PR to be merged:

We could add a release note (though this would seem duplicate to this)
Do we need to set the layout direction here?

ovitrif · 2022-10-11T13:45:13Z

Great work @ovitrif 👍
I've tested the app on a Pixel 5 (Android 13) and everything worked ass expected. The code also LGTM 🎉
A couple of notes but nothing blocking for this PR to be merged:

We could add a release note (though this would seem duplicate to this)

Do we need to set the layout direction here?

Thank you for reviewing & testing @antonis 🙇

I'll add a new release note, this way we're more sure during beta testing we're focusing on the changes 🙇
This view was behaving differently and it would only show the positivity sign (+/-) similar to the other views in RTL when setting the layout direction. I'll explain more in the code-related comment 👍

ovitrif added 4 commits September 27, 2022 13:30

Extract CharSequence.enforceWesternArabicNumerals

0c5dfc9

Enforce Western Arabic numerals in Views and Visitors Stats chart on …

b65822f

…date lines

Enforce Western Arabic numerals in Stats text blocks

af6b25c

Enforce LTR layout direction for Stats change text view

6f6a27e

ovitrif added [Type] Enhancement i18n General labels Sep 27, 2022

ovitrif added this to the 20.8 ❄️ milestone Sep 27, 2022

ovitrif requested a review from antonis September 27, 2022 14:15

ovitrif self-assigned this Sep 27, 2022

Enforce LTR layout direction for Stats change text view

3a004f8

ovitrif changed the base branch from trunk to release/20.8 September 27, 2022 14:19

ovitrif added 2 commits September 27, 2022 16:37

Merge remote-tracking branch 'origin/fix/20.8-western-arabic-numerals…

8bf25e3

…' into fix/20.8-western-arabic-numerals # Conflicts: # WordPress/src/main/res/layout/stats_block_value_item.xml

Remove unused imports from MaterialTextViewWithNumerals

fc743ff

ovitrif changed the base branch from release/20.8 to trunk September 27, 2022 14:45

ovitrif modified the milestones: 20.8 ❄️, 20.9 Sep 27, 2022

AliSoftware reviewed Sep 27, 2022

View reviewed changes

WordPress/src/main/java/org/wordpress/android/util/extensions/StringExtensions.kt Show resolved Hide resolved

ovitrif modified the milestones: 20.9, 21.0 Sep 30, 2022

ovitrif requested a review from a team October 10, 2022 16:38

antonis reviewed Oct 11, 2022

View reviewed changes

WordPress/src/main/java/org/wordpress/android/util/extensions/StringExtensions.kt Show resolved Hide resolved

antonis reviewed Oct 11, 2022

View reviewed changes

antonis approved these changes Oct 11, 2022

View reviewed changes

ovitrif added 2 commits October 11, 2022 16:15

Merge branch 'trunk' into fix/20.8-western-arabic-numerals

b5589f1

Update release notes for 17217

32cf991

ovitrif merged commit 2885159 into trunk Oct 12, 2022

ovitrif deleted the fix/20.8-western-arabic-numerals branch October 12, 2022 15:53

ovitrif added the Stats label Nov 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update more in-app Stats texts to always use western arabic numerals #17217

Update more in-app Stats texts to always use western arabic numerals #17217

ovitrif commented Sep 27, 2022 •

edited

Loading

AliSoftware commented Sep 27, 2022

wpmobilebot commented Sep 27, 2022 •

edited

Loading

wpmobilebot commented Sep 27, 2022 •

edited

Loading

ovitrif commented Sep 27, 2022

ovitrif commented Sep 30, 2022 •

edited

Loading

antonis Oct 11, 2022

ovitrif Oct 11, 2022 •

edited

Loading

antonis left a comment

ovitrif commented Oct 11, 2022 •

edited

Loading

Update more in-app Stats texts to always use western arabic numerals #17217

Update more in-app Stats texts to always use western arabic numerals #17217

Conversation

ovitrif commented Sep 27, 2022 • edited Loading

1️⃣ Test Case 1 : Jetpack / Wordpress App Stats

Previews

1. Text in "Total"-type cards

2. Views & Visitors chart dates

3. Value "change" text

Regression Notes

AliSoftware commented Sep 27, 2022

wpmobilebot commented Sep 27, 2022 • edited Loading

wpmobilebot commented Sep 27, 2022 • edited Loading

ovitrif commented Sep 27, 2022

ovitrif commented Sep 30, 2022 • edited Loading

antonis Oct 11, 2022

Choose a reason for hiding this comment

ovitrif Oct 11, 2022 • edited Loading

Choose a reason for hiding this comment

antonis left a comment

Choose a reason for hiding this comment

ovitrif commented Oct 11, 2022 • edited Loading

ovitrif commented Sep 27, 2022 •

edited

Loading

wpmobilebot commented Sep 27, 2022 •

edited

Loading

wpmobilebot commented Sep 27, 2022 •

edited

Loading

ovitrif commented Sep 30, 2022 •

edited

Loading

ovitrif Oct 11, 2022 •

edited

Loading

ovitrif commented Oct 11, 2022 •

edited

Loading