Update TimeFormatter to include localized Strings #1106
Conversation
There was a problem hiding this comment.
Some minor comments. Other than that this looks 💯
Thanks @danesfeder 🙏
| @@ -492,6 +491,11 @@ private void sendFeedbackEvent(FeedbackEvent feedbackEvent) { | |||
| feedbackEvent.getFeedbackType(), feedbackEvent.getScreenshot(), feedbackEvent.getFeedbackSource()); | |||
| } | |||
|
|
|||
| private long dateDiff(Date date1, Date date2, TimeUnit timeUnit) { | |||
There was a problem hiding this comment.
Could we rename date1 and date2 into something more semantic?
| return dateDiff(new Date(lastCheckedLocation.getTime()), new Date(location.getTime()), TimeUnit.SECONDS); | ||
| } | ||
|
|
||
| private long dateDiff(Date date1, Date date2, TimeUnit timeUnit) { |
| @@ -8,4 +8,12 @@ | |||
| <string name="miles">mi</string> | |||
| <string name="feet">ft</string> | |||
| <string name="eta_format">%s ETA</string> | |||
|
|
|||
| <!-- Time formatting --> | |||
| <string name="day">day</string> | |||
There was a problem hiding this comment.
Should we use quantity string (plurals) instead of having multiple "day" strings?
danesfeder
force-pushed
the
dan-localize-bottomsheet
branch
from
745bbd5
to
4e47c24
Compare
|
@Guardiola31337 this is updated and ready for another round of 👀 - do you mind checking my usage of |
| if (days != 0) { | ||
| String dayFormat = days > 1 ? DAYS : DAY; | ||
| String dayString = String.format(" %s ", resources.getQuantityString(R.plurals.numberOfDays, (int) days)); |
There was a problem hiding this comment.
Could we extract " %s " magic number?
danesfeder
force-pushed
the
dan-localize-bottomsheet
branch
from
4e47c24
to
fb20342
Compare
|
@danesfeder do we need to update/do something in Transifex for supporting latest changes (especially |
|
@Guardiola31337 yeah we should push the update strings.xml to Transifex - I'm wondering if the plurals usage will throw off Transifex, but we will have to see |
Closes #1097
We are currently using hard-coded constants for arrival time formatting. This PR replaces them with
Stringresources, which have also already been pushed to Transifex.Noting that this PR does not actually add the necessary translations but it does set up the code to be ready for them once we pull them from Transifex.