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

Fix negative duration #1317

Merged
merged 6 commits into from Jan 21, 2021
Merged

Fix negative duration #1317

merged 6 commits into from Jan 21, 2021

Conversation

kamontat
Copy link
Contributor

@kamontat kamontat commented Jan 6, 2021

Problem

When I create Duration with negative number, the default value of each unit is -1 since you round everything down (Math.floor).

Solution

I add if case when number is negative it should run Math.ceil() instead.

Example

For positive number

const duration = dayjs.duration(1, "milliseconds")
console.log(duration) // { year: 0, month: 0, day: 0, hour: 0, minute: 0, second: 0, millisecond: 1 }

For negative number (current)

const duration = dayjs.duration(-1, "milliseconds")
console.log(duration) // { year: -1, month: -1, day: -1, hour: -1, minute: -1, second: -1, millisecond: -1 }

For negative number (expected)

const duration = dayjs.duration(-1, "milliseconds")
console.log(duration) // { year: 0, month: 0, day: 0, hour: 0, minute: 0, second: 0, millisecond: -1 }

@codecov
Copy link

codecov bot commented Jan 6, 2021

Codecov Report

Merging #1317 (3ae2d4b) into dev (cc993f0) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##               dev     #1317   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          176       176           
  Lines         1709      1737   +28     
  Branches       391       394    +3     
=========================================
+ Hits          1709      1737   +28     
Impacted Files Coverage Δ
src/plugin/duration/index.js 100.00% <100.00%> (ø)
src/locale/bn.js 100.00% <0.00%> (ø)
src/locale/de.js 100.00% <0.00%> (ø)
src/locale/pt.js 100.00% <0.00%> (ø)
src/locale/sl.js 100.00% <0.00%> (ø)
src/plugin/weekYear/index.js 100.00% <0.00%> (ø)
src/plugin/devHelper/index.js 100.00% <0.00%> (ø)
src/plugin/advancedFormat/index.js 100.00% <0.00%> (ø)
src/plugin/customParseFormat/index.js 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc993f0...3ae2d4b. Read the comment docs.

kamontat added a commit to kamontat/countdown that referenced this pull request Jan 6, 2021
test/plugin/duration.test.js Outdated Show resolved Hide resolved
@iamkun
Copy link
Owner

iamkun commented Jan 21, 2021

Cool, thanks

@iamkun iamkun merged commit 3f5c085 into iamkun:dev Jan 21, 2021
iamkun pushed a commit that referenced this pull request Jan 22, 2021
## [1.10.4](v1.10.3...v1.10.4) (2021-01-22)

### Bug Fixes

* Correct handling negative duration ([#1317](#1317)) ([3f5c085](3f5c085))
* Improve `Duration` types ([#1338](#1338)) ([4aca4b1](4aca4b1))
* parse a string for MMM month format with underscore delimiter ([#1349](#1349)) ([82ef9a3](82ef9a3))
* Update Bengali [bn] locale ([#1329](#1329)) ([02d96ec](02d96ec))
* update locale Portuguese [pt] yearStart ([#1345](#1345)) ([5c785d5](5c785d5))
* update Polish [pl] locale yearStart ([#1348](#1348)) ([e93e6b8](e93e6b8))
* Update Slovenian [sl] relativeTime locale ([#1333](#1333)) ([fe5f1d0](fe5f1d0))
@iamkun
Copy link
Owner

iamkun commented Jan 22, 2021

🎉 This PR is included in version 1.10.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@@ -25,6 +25,7 @@ const wrapper = (input, instance, unit) =>
new Duration(input, unit, instance.$l) // eslint-disable-line no-use-before-define

const prettyUnit = unit => `${$u.p(unit)}s`
const roundNumber = number => number < 0 ? Math.ceil(number) : Math.floor(number)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be replaced with Math.trunc(number), which is shorter.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems not supported in IE :-(

andrewhood125ruhuc added a commit to andrewhood125ruhuc/SidRH2 that referenced this pull request May 10, 2022
## [1.10.4](iamkun/dayjs@v1.10.3...v1.10.4) (2021-01-22)

### Bug Fixes

* Correct handling negative duration ([#1317](iamkun/dayjs#1317)) ([3f5c085](iamkun/dayjs@3f5c085))
* Improve `Duration` types ([#1338](iamkun/dayjs#1338)) ([4aca4b1](iamkun/dayjs@4aca4b1))
* parse a string for MMM month format with underscore delimiter ([#1349](iamkun/dayjs#1349)) ([82ef9a3](iamkun/dayjs@82ef9a3))
* Update Bengali [bn] locale ([#1329](iamkun/dayjs#1329)) ([02d96ec](iamkun/dayjs@02d96ec))
* update locale Portuguese [pt] yearStart ([#1345](iamkun/dayjs#1345)) ([5c785d5](iamkun/dayjs@5c785d5))
* update Polish [pl] locale yearStart ([#1348](iamkun/dayjs#1348)) ([e93e6b8](iamkun/dayjs@e93e6b8))
* Update Slovenian [sl] relativeTime locale ([#1333](iamkun/dayjs#1333)) ([fe5f1d0](iamkun/dayjs@fe5f1d0))
andrewhood125ruhuc added a commit to andrewhood125ruhuc/SidRH2 that referenced this pull request May 10, 2022
## [1.10.4](iamkun/dayjs@v1.10.3...v1.10.4) (2021-01-22)

### Bug Fixes

* Correct handling negative duration ([#1317](iamkun/dayjs#1317)) ([3f5c085](iamkun/dayjs@3f5c085))
* Improve `Duration` types ([#1338](iamkun/dayjs#1338)) ([4aca4b1](iamkun/dayjs@4aca4b1))
* parse a string for MMM month format with underscore delimiter ([#1349](iamkun/dayjs#1349)) ([82ef9a3](iamkun/dayjs@82ef9a3))
* Update Bengali [bn] locale ([#1329](iamkun/dayjs#1329)) ([02d96ec](iamkun/dayjs@02d96ec))
* update locale Portuguese [pt] yearStart ([#1345](iamkun/dayjs#1345)) ([5c785d5](iamkun/dayjs@5c785d5))
* update Polish [pl] locale yearStart ([#1348](iamkun/dayjs#1348)) ([e93e6b8](iamkun/dayjs@e93e6b8))
* Update Slovenian [sl] relativeTime locale ([#1333](iamkun/dayjs#1333)) ([fe5f1d0](iamkun/dayjs@fe5f1d0))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants