Closed Bug 1206318 Opened 9 years ago Closed 9 years ago

Migrate mozL10n.DateTimeFormat to Intl API in Calendar

Categories

(Firefox OS Graveyard :: Gaia::Calendar, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: zbraniecki, Assigned: zbraniecki)

References

Details

Attachments

(1 file)

We're moving away from l10n_date.js:

 - all localeFormat should switch to Intl.DateTimeFormat
 - all prettyNow should switch to async relativeDate (and in the future will be moved to mozIntl and then to Intl once the API adds relative dates).
Comment on attachment 8663238 [details] [review]
[gaia] zbraniecki:1206318-move-calendar-to-intl-api > mozilla-b2g:master

Gaye, I took the first look at this. It's been a lot of work but I think it's very rewarding. I think that Intl API and IntlHelper make the code look cleaner and open up a way to unify a lot of datetimeformatters.

I was trying to minimize the changes in this patch so I left some overlapping formatters, and didn't try to move to use IntlHelper.observe, but I did try to remove all mozL10n.DateTimeFormats.

Two are left because they use relativeParts and relativeDate - both API usages are fine.

What's also exciting is that a lot of work that is going on around Intl API rev. 3 will help with Calendar - things like firstDayOfTheWeek, relativeDates etc. should end up in the API and we'll be able to move to that and further simplify our code.

At this point I didn't touch any tests and would just like to get your overall feedback on the approach. Thanks!
Attachment #8663238 - Flags: feedback?(gaye)
Comment on attachment 8663238 [details] [review]
[gaia] zbraniecki:1206318-move-calendar-to-intl-api > mozilla-b2g:master

> At this point I didn't touch any tests and would just like to get your overall feedback on the approach. Thanks!

Approach looks great. Nice work!
Attachment #8663238 - Flags: feedback?(gaye) → feedback+
Depends on: 1208698
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Depends on: 1209821
Comment on attachment 8663238 [details] [review]
[gaia] zbraniecki:1206318-move-calendar-to-intl-api > mozilla-b2g:master

This patch moves Calendar from using mozL10n.DateTimeFormat to mozIntl.

Kevin, can you review this?
Attachment #8663238 - Flags: review?(kevingrandon)
Comment on attachment 8663238 [details] [review]
[gaia] zbraniecki:1206318-move-calendar-to-intl-api > mozilla-b2g:master

I haven't been a calendar peer in a while now, so I'm not sure I should review this. Forwarding to Gareth who's the owner.
Attachment #8663238 - Flags: review?(kevingrandon) → review?(gaye)
:gaye, could you help me with getting my patch better fit Calendar app model?
Flags: needinfo?(gaye)
Awesome! I applied your cleanups and fixed some tests and the tree is almost green.

I left one question for you in the github, and I have one left orange test due to:

 - https://github.com/mozilla-b2g/gaia/blob/master/apps/calendar/test/unit/views/time_header_test.js#L139

In this line, the test is trying to set the scale to 'year', but as you can see here: 
 - https://github.com/mozilla-b2g/gaia/blob/master/apps/calendar/js/views/time_header.js#L36-L43

no such scale exists.

Previously that probably failed silently. Now it fails with a bang.

How do you want me to fix it?
Flags: needinfo?(gaye)
Comment on attachment 8663238 [details] [review]
[gaia] zbraniecki:1206318-move-calendar-to-intl-api > mozilla-b2g:master

In response to rjs question -- we still need to load it in the main html file. We used the rjs shim config which just grabs the thing you're requiring from global ns.
Flags: needinfo?(gaye)
Attachment #8663238 - Flags: review?(gaye) → review+
Treeherder is being weird so I can't confirm that tests are passing right now. Feel free to check in when everything is green.
https://github.com/mozilla-b2g/gaia/commit/f7f41903c91bb5de35efa42bdb7e10741c884c08
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: