Add date-time-select and date-range-select to Angular client

[Jane Sandberg]

I've built two components as part of my work on https://bugs.launchpad.net/evergreen/+bug/1816475

I will be posting them here later today on a separate ticket, so that they are easier to review in isolation.

[Jane Sandberg (sandbergja)]

Here is the branch: working/user/sandbergja/lp1834662_add_date_components_to_angular

Here is a link: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/sandbergja/lp1834662_add_date_components_to_angular

[Bill Erickson (berick)]

Hi Jane, starting looking through this today. Initial review and testing looks good.

1. I may have missed it, but would you mind adding an example usage of the egValidDatetime / DatetimeValidatorDirective? (I do see the DatetimeValidator service in use).

2. Should we add the caret specifier to the package.json changes?
    "moment": "^2.24.0",
    "moment-timezone": "^0.5.25",

3. I noticed a number of errors in the console log on the sandbox page and tracked it down to having a null time zone. This was the result of a) not having a timezone configured in the database and b) attempts made by the Angular code to allow for a default (based on OpenSRF.tz) were not successful because of a logic error.

I pushed a fix to working/user/berick/lp1834662-default-tz

Interestingly, when I use this (with no related org unit settings), it results in a dateTimeFormat of "short" which produces a deprecation warning in the console. Removing the meridian string "a" from the mapped format allowed momentizeDateTimeString() to complete without the warning. Not sure what's up with that.

4. More on makeFormatParseable()... I could be missing something, but I'm concerned this mapping would only work for en-US sites, since the Angular date formats vary by locale.

[Jane Sandberg (sandbergja)]

Thanks for taking a look at this, Bill.

I've signed off your commit, and cherry-picked it into my branch. I also added a new commit that fixes 1, 2, and 4. I need to do some more work on 3 -- this code doesn't have the best fall-throughs for when the timezone and/or datetime format ou settings are missing.

[Jane Sandberg (sandbergja)]

I just force-pushed some tests for the locale-aware makeFormatParseable in cs-CZ, ar-JO, and fr-CA.

[Jane Sandberg (sandbergja)]

I force-pushed some ng lint fixes. Interestingly, I had to disable linting for one line of the date-range-select test file because of this issue: https://github.com/angular/angular-cli/issues/11548

[Jane Sandberg (sandbergja)]

Here is a rebased branch: user/sandbergja/lp1834662_add_date_components_to_angular_rebased

[Jane Sandberg (sandbergja)]

Added another commit to fix some deprecation errors and some incomplete locale fallbacks. My testing is going well, both with and without the timezone and datetime format org settings. Adding pullrequest tag to see what y'all think.

[Jane Sandberg (sandbergja)]

I rebased this branch after the flurry of changes to the Angular client: user/sandbergja/lp1834662_add_date_components_to_angular_rebased

This is maybe a bit presumptuous, but I threw these components into the new CommonWidgetsModule. If that's not the best place for them, let me know. I am happy to move them around!

[Bill Erickson (berick)]

Grabbed and got sidetracked. Hope to return soon.

[Bill Erickson (berick)]

Hi Jane, I have a question.

In a world... where we use native date and time selectors (bug #1840782) instead of bootstrap widgets with configurable date input formats, all date/time selectors would produce consistent values, regardless of the locale-specific display. E.g. <input type="date" /> always produces a "YYYY-MM-DD" string value.

In such a scenario, where every date is either a Date, an ISO string, YYYY-MM-DD, or hh:mm:ss, am I right in thinking we would no longer need the MomentJS library and the code to translate between MomentJS and Angular, since we would not be parsing date strings? It's very possible I'm missing something. In any event, I'm hoping to confirm my assumption in case it has any impact on this bug or bug #1840782.

[Jane Sandberg (sandbergja)]

Good question, Bill. We really only need Moment as a dependency of moment-timezone. In the world you described, I think:

1) We would still need to display times in arbitrary timezones (e.g. in the case of booking pickup return times, due times, we need to be able to display them in the timezone of the relevant library, not whatever timezone the user's device happens to be in).
2) We wouldn't need to parse times from arbitrary timezones. So I think we could get rid of all that parsing/translating stuff.

moment-timezone isn't our only option for #1, though. It seems like Moment is unpopular with the Angular crowd because it is not tree-shakeable.

[Bill Erickson (berick)]

Thanks, Jane.

For displaying dates in different time zones, there is some promising new stuff that may allow us to drop moment-timezone.

> var formatter = new Intl.DateTimeFormat('en-us', {
 year: 'numeric',
 month: 'numeric',
 day: 'numeric',
 hour: 'numeric',
 minute: 'numeric',
 timeZone: 'America/Los_Angeles'
})

> console.log(formatter.format(d));
8/20/2019, 2:04 PM

// It's 5:04 PM in my east coast browser.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DateTimeFormat/formatToParts

==

I'm not trying to derail this bug, BTW, just want to get on the same page about where we may be heading. We can always move forward w/ Moment and modify things later if that works best.

[Jane Sandberg (sandbergja)]

COOL! I'd feel pretty good about moving away from moment-timezone in favor of Intl.DateTimeFormat once we start using the browser's date and time inputs.

[Bill Erickson (berick)]

Oh, duh, and the Angular DatePipe accepts timezone and locale arguments.

[Bill Erickson (berick)]

Pause my last comment, since I have not verified if DatePipe accepts strings like 'America/Los_Angeles', etc.

[Bill Erickson (berick)]

Bug #1840782 is on hold for now. Grabbing bug to move forward with Moment-based approach so we can get this / booking into 3.4.

[Bill Erickson (berick)]

Merged to master. Thanks, Jane!