-
Notifications
You must be signed in to change notification settings - Fork 749
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
More control over DateTime#toRelative rounding behavior #1129
Comments
This "rounding down" behavior is especially strange when using With the default behavior, you'll see "1 minute" for a full 60 seconds before you see "59 seconds" and realize now you've actually got a minute left. I was hopeful when I saw #926 because I thought specifying "multiple units" like So maybe an option like |
I ended up implementing my own. If someone wants to fold that into Luxon as a feature, I'll be happy to use it from there. :) function relativeDuration(duration: Duration, opts?: Opts): string {
let sigU = opts?.significantUnits ?? 2
if (sigU < 1) {
throw Error("Signficant units can't be < 1")
}
let units = opts?.units ?? defaultUnits
// Make sure units are ordered in descending significance:
units = allUnits.filter(it => units.includes(it))
let negative = duration.valueOf() < 0
if (negative) { duration = duration.negate() }
duration = duration.shiftTo(...units)
// Remove unnecessary most-significant units:
while (units.length > 1) {
if (duration.get(units[0]) > 0) {
// we've found the most significant unit:
break
}
units = units.slice(1)
duration = duration.shiftTo(...units)
}
units = units.slice(0, sigU)
duration = duration.shiftTo(...units)
// If the last unit has fractional bits, we don't care. We're explicitly limiting significant units:
let lastUnit = units[units.length - 1]
duration = duration.set({
[lastUnit]: Math.floor(duration.get(lastUnit))
})
let relative = duration.toHuman()
if (negative) {
return `${relative} ago`
} else {
return `in ${relative}`
}
}
interface Opts {
// Default: 2
significantUnits?: number
// Default: all but quarters & months
units?: (keyof DurationObjectUnits)[]
}
const allUnits: ReadonlyArray<keyof DurationObjectUnits> = ["years", "quarters", "months", "weeks", "days", "hours", "minutes", "seconds", "milliseconds"]
// No quarters/weeks:
const defaultUnits: ReadonlyArray<keyof DurationObjectUnits> = ["years", "months", "days", "hours", "minutes", "seconds", "milliseconds"] |
I also interpreted it to mean that, and I'm currently lacking a way to format a relative date in that way, so it'd be nice to see this feature added. |
At least make |
I think this or something similar would be a great addition. Especially when moving from moment where the default behavior was different.
Having to add this halfday padding constant doesn't seem very ergonomic.
Would be much more graceful. |
@icambron Sorry for the tag, you're just the first maintainer I encountered. I'd be willing to contribute this change myself, but considering it's a change that as described would break existing functionality (changing the boolean option to a series of strings), I didn't want just to do the work and make a pull request without having an indication of if the project owners had any interest in this change to begin with. I'm tagging you to see if I can get some feedback there, or if maybe with some tweaks such a change would be accepted. |
I am facing the same issue. The difference is 1.99 days and the format displays "in 1 day" :/ One solution is to patch the diffRelative function so that it it doesn't round towards zero (the last parameter to the Something like this should do the trick: @@ -6269,7 +6269,7 @@ function quickDT(obj, opts) {
function diffRelative(start, end, opts) {
var round = isUndefined(opts.round) ? true : opts.round,
format = function format(c, unit) {
- c = roundTo(c, round || opts.calendary ? 0 : 2, true);
+ c = roundTo(c, round || opts.calendary ? 0 : 2, false);
var formatter = end.loc.clone(opts).relFormatter(opts);
return formatter.format(c, unit);
}, That option should probably be added to the |
I would prefer upgrading the |
I created a pull request for this! However, I decided to add only the "nearest" option for now, since it was very small change. And I don't know if anyone would even need the "up" / "ceil" option, but that could be added in a future pull request if someone really needs that. |
I've made a PR (#1685) adding a new parameter Hope you will find it useful. Sorry for the PR mess. 😥 |
Is your feature request related to a problem? Please describe.
For some units of time, rounding down is very unintuitive. It's probably fine if 29.99 minutes rounds down to 29, but it's fairly inaccurate to describe 1.99 years away as "1 year away." Padding in these situations that could be minutes away or years away can lead to a lot of extra code to figure out how much padding there should be in this situation.
Describe the solution you'd like
I think one possible solution would be to change the
round
option you can pass totoRelative
from a on/off style boolean into a setting probably based on a string. Something likeround: 'down' | 'up' | 'closest' | 'off'
where, if we want to keep the current default, it defaults todown
.off
would be the equivalent offalse
currently,up
would invert the behavior and always round upwards, andclosest
would round towards the closest integer, rather than specifically up or down.Describe alternatives you've considered
With the existing options the only alternatives I've come up with are calculating the padding based on the kind of rounding I want based on the difference between the date for comparison and now, or altering the date for comparison itself to generate the term I want when handed to
toRelative
. An alternative to the addition I suggest above might be to allow handing a function to theround
option that would receive a parameter of the unrounded relative value, and the resulting number would be whatever is returned from that function.The text was updated successfully, but these errors were encountered: