1206318 - Migrate mozL10n.DateTimeFormat to Intl API in Calendar

Status: RESOLVED FIXED

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

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

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 1

[GitHub Autolander Bot]

Attachment: [gaia] zbraniecki:1206318-move-calendar-to-intl-api > mozilla-b2g:master

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

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!

Comment 3

[Gareth Aye [:gaye] (back from PTO)]

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!

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

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?

Comment 5

[Kevin Grandon :kgrandon]

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.

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

:gaye, could you help me with getting my patch better fit Calendar app model?

Comment 7

[Gareth Aye [:gaye] (back from PTO)]

I have some cleanup here https://github.com/gaye/gaia/tree/1206318-move-calendar-to-intl-api

Comment 8

[Zibi Braniecki [:zbraniecki][:gandalf]]

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?

Comment 9

[Gareth Aye [:gaye] (back from PTO)]

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.

Comment 10

[Gareth Aye [:gaye] (back from PTO)]

Treeherder is being weird so I can't confirm that tests are passing right now. Feel free to check in when everything is green.

Comment 11

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/gaia/commit/f7f41903c91bb5de35efa42bdb7e10741c884c08