- Notifications
You must be signed in to change notification settings - Fork184
Add new implementation of relative time#300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
3cdcb9d to361ef2dCompareThe failing test in the first check: is caused by the original test using |
| @@ -453,7 +454,7 @@ export class RelativeTimeElement extends HTMLElement implements Intl.DateTimeFor | |||
| if (format === 'duration') { | |||
| newText = this.#getDurationFormat(duration) | |||
| } else if (format === 'relative') { | |||
| newText = this.#getRelativeFormat(duration) | |||
| newText = this.#getRelativeFormat(date) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The intent of using durations here is to anticipateTemporal becoming part of the web platform.
We calculate the duration of the two dates and return a proxy object forTemporal.Duration, so that we can in turn hope to callround on this duration, to get a single unit duration object, which can then be more easily fed intoIntl.RelativeTimeFormat.
TheTemporal.Duration#round function is very complex, and so for now we haveroundToSingleUnit as a "simplified" version of this function, which provides the minimum viable requirements for this code.
The hope is that, onceTemporal is a part of the web platform, we can delete a lot of the delicate date code in this library.
The change in this PR, while reasonable in isolation, steps us further away from this goal of steering toward Temporal.
Having said that I'm sure this PR can be factored such that it maintains this goal. It might be worth looking at theround API to figure out how to change the inputs torelativeTime. Perhaps it is worth taking the functionality ofrelativeTime and factoring it intoroundToSingleUnit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That makes me question then why an already existing implementation of theround function could not be adopted.
Perhaps it is worth taking the functionality of
relativeTimeand factoring it intoroundToSingleUnit?
Sure, it can be quickly done. Without knowing theTemporal vision I made a new function with a different signature because calculating a duration then calculating back the original date would be a very roundabout thing to do. Perhaps then I will raise this concern totc39/proposal-temporal.
I have applied this implementation to |
(This is part of a set of multiple pull requests looking to overhaul the calculation functions.)
This pull request adds a new
relativeTimefunction that directly takes two date values and outputs a single-unit relative time point, with more robust logic compared to the current implementation.Accordingly, the logic of the relative time element for the
relativeformat is changed to use the new function.