Refactoring Medtrum patch age on pump tab (cleanup code, better l10n)#4326
Refactoring Medtrum patch age on pump tab (cleanup code, better l10n)#4326MilosKozak merged 5 commits intonightscout:devfrom
Conversation
Moved age logic to DateUtil.timeAgoFullString() Refined for better l10n
Update string resources for DateUtil.timeAgoFullString()
Moved ago string logic to DateUtil.timeAgoFullString() for better l10n
|
@MilosKozak Cleaner code + better translatable strings. |
| val daysAgo = T.msecs(milliseconds).days() | ||
| val hoursAgo = T.msecs(milliseconds).hours() % 24 | ||
| val minutesAgo = T.msecs(milliseconds).mins() % 60 | ||
| val agoString = if (daysAgo > 1) |
There was a problem hiding this comment.
Code could be simplified with rh.gq(app.aaps.core.ui.R.plurals.days, daysAgo, daysAgo)
This will manage singular and plural (same for hours and minutes)
dayString and hourString could be merged together with the global string including "ago" ...
| <string name="time_ago">ago</string> | ||
| <string name="days_ago">%1$.1f days ago</string> | ||
| <string name="days_ago_round">%1$.0f days ago</string> | ||
| <string name="day_hours_ago">%1$s day %2$s hours ago</string> |
There was a problem hiding this comment.
Here I think only 2 new strings are enough if you use plurals existing keys in core.ui
%1$s ago and %1$s %2$s ago
See comment below
There was a problem hiding this comment.
Purpose of this PR is:
- Fixing l10n translations
- Generalize and cleanup code (by moving logic outside pump driver)
Changes mentioned are about optimizing use of string resources (optional) in DateUtil.timeAgoFullString().
There was a problem hiding this comment.
Updated PR for use of plurals type resource strings
|
|
@Philoul @jbr7rr: Think this is the final PR now. @MilosKozak Current (updated) PR merges, (AAPS-CI) builds and was tested ok for current dev. |
|
Looks good to me code wise. Like discussed before it would be good to color the patch age (yellow at 3 days, red at 3 days and 8 hours) |
If we color text, then the thresholds defined for StatusLight (Overview preferences) should be used to be consistant with colors in overview 😉 |
Good idea. Think I can do that too. As this is new to the original purpose of this PR (l10n & code cleanup), |
|
@MilosKozak Thanks for merging this PR: @jbr7rr @Philoul Tnx for review input. Will contact you on new PR handling options for coloring the age field. |
Fix typo in new string from #4326
4 participants
Refactoring Medtrum patch age on pump tab: