Skip to content
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

Minutes are not rounded properly #14

Closed
manufont opened this issue Oct 15, 2018 · 6 comments
Closed

Minutes are not rounded properly #14

manufont opened this issue Oct 15, 2018 · 6 comments

Comments

@manufont
Copy link

First, let me thank you for providing such a useful component.
I stumbled upon a case where the minutes value is not rounded properly: It renders '1 hour 60 minutes' when it should be '2 hours 0 minutes'. Is that the expected behavior ?

image

<FormattedDuration
  seconds={7179}
  textComponent="span"
  format="{hours} {minutes}"
/>
@piuccio
Copy link
Owner

piuccio commented Oct 16, 2018

oh interesting. Surely it's not expected, I'll try to have a look these days

@manufont
Copy link
Author

For anyone wondering, I managed to make it work by using the format {hours} {minutes} {seconds} with a value rounded to the closest minute.

const stripSeconds = value => Math.round(value / 60) * 60;

[...]

<FormattedDuration
  seconds={stripSeconds(7179)}
  textComponent="span"
  format="{hours} {minutes} {seconds}"
/>

@piuccio
Copy link
Owner

piuccio commented Oct 18, 2018

Hey, I'm now working on this and I'm wondering what should be the expected result, 7179 is 1h 59m 39s. Should the library approximate automatically? I'm tempted to go for 1h 59m with a configurable option to display 2h

@manufont
Copy link
Author

Thank you for taking care of this issue.

According to the documentation:

Seconds is also optional and if missing, minutes will be rounded to the closed value

<FormattedDuration seconds={10} textComponent={Text} format="{minutes}" />
// will render `0 minutes`

<FormattedDuration seconds={30} textComponent={Text} format="{minutes}" />
// will render `1 minute`

<FormattedDuration seconds={70} textComponent={Text} format="{minutes}" />
// will render `1 minute`

So I guess the default option should be to round it to 2h, for consistency with other formats

@piuccio
Copy link
Owner

piuccio commented Oct 22, 2018

I've been working on better organising the code, now the logic to format seconds is on this repo https://github.com/en-japan-air/intl-unofficial-duration-unit-format
What's left is to update this project to use it, I have it in a separate branch but I need a bit more testing. I'm quite busy this week, so I'm not sure when I'll work on it.

@piuccio
Copy link
Owner

piuccio commented Oct 31, 2018

I've published a fix for this in version 3.0.0 (#15)

@piuccio piuccio closed this as completed Oct 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants