feat: add luxon date adapter - #23167
Conversation
google-cla
Bot
added
the
cla: yes
1765836 to
16e915a
Compare
crisbeto
added
merge safe
P2
|
@crisbeto: as the maintainer of the fork of your original PR, I'd like to notify you that there have been a few fixes over time to it. I don't remember the details for all of them, but take a look at: andreialecu/ngx-material-luxon#4 Also this change is important: See this report: andreialecu/ngx-material-luxon#3 |
|
The feedback has been addressed. Also thanks for the pointers @andreialecu, I've incorporated most of them. I decided to skip the one about the first day of the week, because I think that we should have a consistent way of doing it with the other adapters. It's also fairly easy to work around by extending the existing adapter and overriding the one method. |
|
|
||
| getDateNames(): string[] { | ||
| // At the time of writing, Luxon doesn't offer similar | ||
| // functionality so we have to fall back to the Intl API. |
There was a problem hiding this comment.
Is this the corresponding github issue?
moment/luxon#549
There was a problem hiding this comment.
I don't think so. This isn't really a bug, it's just something that they haven't exposed.
| return (this._useUTC ? DateTime.utc() : DateTime.local()).setLocale(this.locale); | ||
| } | ||
|
|
||
| parse(value: any, parseFormat: string | string[]): DateTime | null { |
There was a problem hiding this comment.
Should we tighten the type of value to what we actually support in the function? Or potentially even unknown since we're checking the type anyway.
There was a problem hiding this comment.
It's an any, because the DateAdapter interface has it as an any too.
There was a problem hiding this comment.
Yeah, I think it makes sense for the abstract adapter to be any, but IIRC subclasses are able to narrow signatures, right? (Non-blocking)
Adds a new package called `@angular/material-luxon-adapter` that provides a date adapter to be used together with Luxon dates.
|
The latest feedback has been addressed. |
|
Caretaker note from Jeremy:
|
crisbeto
added
the
merge: caretaker note
|
I saw that this was merged and that releases happened since, however the I'm only mentioning it in case this isn't a know issue, and something related to build / release setup was overlooked. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
6 participants
Adds a new package called
@angular/material-luxon-adapterthat provides a date adapter to be used together with Luxon dates.