Skip to content

Add Time duration support#87

Merged
shellscape merged 1 commit intoshellscape:masterfrom
radeno:timeValues
Jun 26, 2019
Merged

Add Time duration support#87
shellscape merged 1 commit intoshellscape:masterfrom
radeno:timeValues

Conversation

@radeno
Copy link
Copy Markdown
Contributor

@radeno radeno commented Jun 24, 2019

This PR contains:

  • bugfix
  • feature
  • refactor
  • tests
  • documentation
  • metadata

Breaking Changes?

  • yes
  • no

Please Describe Your Changes

This PR adds support for time duration values. Nothing more.

jonathantneal
jonathantneal approved these changes Jun 24, 2019
View reviewed changes
Comment thread lib/nodes/Numeric.js
const Node = require('./Node');

const unitRegex = /%|ch|cm|em|ex|in|mm|pc|pt|px|q|rem|vh|vmax|vmin|vw$/i;
const unitRegex = /%|ch|cm|em|ex|in|mm|ms|pc|pt|px|s|q|rem|vh|vmax|vmin|vw$/i;
Copy link
Copy Markdown
Collaborator

@jonathantneal jonathantneal Jun 24, 2019

Choose a reason for hiding this comment

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

As defined in:

CSS Level 2: https://www.w3.org/TR/CSS2/aural.html#times
CSS Values Level 3: https://drafts.csswg.org/css-values-3/#time
CSS Values Level 4: https://drafts.csswg.org/css-values-4/#time

@jonathantneal
Copy link
Copy Markdown
Collaborator

jonathantneal commented Jun 24, 2019

@shellscape, this seems pretty clean. I‘m not sure why the unitRegex has a $ expression at the end of vw, but this change is consistent with how the tool already works.

@radeno
Copy link
Copy Markdown
Contributor Author

radeno commented Jun 24, 2019

I tried to add also angle units. But it doesn't take 90deg as value, test fails. Another value, like 10deg was ok. Don't have time to dig deeper why.
So support time values is small step forward.

@shellscape shellscape merged commit 926df59 into shellscape:master Jun 26, 2019
@shellscape
Copy link
Copy Markdown
Owner

shellscape commented Jun 26, 2019

Thanks for putting this together 🍺

@glen-84 glen-84 mentioned this pull request Aug 6, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jonathantneal jonathantneal jonathantneal approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@radeno @jonathantneal @shellscape