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

Do not add px to css custom-properties #480

Merged
merged 2 commits into from
Nov 30, 2017
Merged

Do not add px to css custom-properties #480

merged 2 commits into from
Nov 30, 2017

Conversation

TrySound
Copy link
Contributor

Ref #479

What:

Bug

Why:

To support css custom properties which can be used to simplify cascade

How:

Just added special case with checking keys started with --

Checklist:

  • Documentation
  • Tests
  • Code complete

@codecov
Copy link

codecov bot commented Nov 29, 2017

Codecov Report

Merging #480 into master will not change coverage.
The diff coverage is 100%.

Impacted Files Coverage Δ
packages/emotion/src/index.js 100% <100%> (ø) ⬆️

if (unitless[key] !== 1 && !isNaN(value) && value !== 0) {
if (
unitless[key] !== 1 &&
key.indexOf('--') !== 0 &&
Copy link
Member

Choose a reason for hiding this comment

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

Could you change this to key.charCodeAt(1) !== 45(45 is the char code for -) since this is a very hot path and charCodeAt is much faster.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean checking both first two chat codes?

Copy link
Member

Choose a reason for hiding this comment

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

No, there's no need to since there are no regular css properties with a dash as the second character.

Copy link
Member

@emmatown emmatown left a comment

Choose a reason for hiding this comment

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

Thanks!

@emmatown emmatown merged commit 4232dd7 into emotion-js:master Nov 30, 2017
@TrySound TrySound deleted the unitless-custom-properties branch November 30, 2017 05:59
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

Successfully merging this pull request may close these issues.

None yet

2 participants