`largest` option API unclear or bug

[pandaiolo]

My app is displaying a counter down.

I set largest option to 3:

• when time left is above 1 day, seconds are not shown
• when time left is less than 1 day, show seconds

It works well except one case that broke recently:

• time left is more than 1 day
• minutes === 0

For example with duration of 1 day, 2 hours, 1 minute and 5 seconds, and then 10sec and 20sec later

Expected: 1d 2h 1min 1d 2h 1d 2h

Displayed: 1d 2h 1min 1d 2h 55s 1d 2h 45s

I also use the trim: 'both' option. Edit: I see from the doc this has no effect

Is this the expected behaviour? I think it is weird that the seconds are displayed during 1 minute at every hour of the countdown. I would expect the seconds not to show until there is actually less than 1 day left. ie I would expect largest option to be largest 3 units before trim and not largest visible units.

Not sure of the other use cases and side effects though.

[jsmreese]

You've got a great point. What you're observing is the expected behaviour as it's implemented... but I think you're right that the output is not intuitive.

I'd call this a bug in the largest implementation. largest, as you say, should take the n largest-magnitude tokens from the template, starting from the first one that has a value.

Setting largest should default trim to "all" if trim is not already set, but largest and trim should be able to be used together as well.

I ran into a very similar issue trying to use the new maxValue and minValue options.

[jsmreese]

@pandaiolo The updated version is published. Please let me know if you see anything that isn't formatting as you expect!

[pandaiolo]

Sorry for the delay!

It works but I think there is a regression bug. The following test addition shows the new bug:

equal(moment.duration(1, 'minutes').format('h:mm:ss', { trim: false }), '0:01:00')

It shows: 0:1:00 (padding of minutes is not respected)

[jsmreese]

Totally surprised that wasn't caught by any of my tests! Thank you!

[jsmreese]

@pandaiolo Thank you again! I've just published a fix for that issue in version 2.1.1.

[pandaiolo]

It works, thanks! 👍