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

Improper expansion of transition-delay property #7

Open
callmevlad opened this issue Jan 25, 2016 · 3 comments
Open

Improper expansion of transition-delay property #7

callmevlad opened this issue Jan 25, 2016 · 3 comments

Comments

@callmevlad
Copy link

Expanding transition shorthand like this:

shortcss.expand('transition', 'opacity 200ms ease')

Currently returns:

{
  "transition-property": "opacity",
  "transition-duration": "200ms",
  "transition-timing-function": "ease",
  "transition-delay": "200ms"
}

In this case, the transition-delay: 200ms; is invalid since transition delay is not specified in the shorthand and should default to 0s instead.

Expanding opacity 200ms ease 0.5s works correctly as expected 👍

(Will include a PR once I find some time, but just wanted to get this on your radar.)

@callmevlad
Copy link
Author

Actually, this could be a bigger issue that may be outside the scope of this lib. It doesn't seem to support multi-part shorthand properties like transition and background-image. For example:

shortcss.expand('transition', 'margin 3ms ease, padding 100ms ease-in-out')

Should return:

{
  'transition-property': 'margin, padding',
  'transition-duration': '3ms, 100ms',
  'transition-timing-function': 'ease, ease-in-out',
  'transition-duration': ''
}

But currently returns:

{
  'transition-property': 'margin',
  'transition-duration': '3ms', 
  'transition-timing-function': 'ease,', 
  'transition-delay': 'padding'
}

@qfox
Copy link
Member

qfox commented Jan 26, 2016

Thanks for bug report.

I'm not sure about resolving sources to comma separated lists like 'margin, padding'. Need some research. Personally, I would resolve multivalued rules with arrays by default. Like:

shortcss.expand('transition', 'margin 3ms ease, padding 100ms ease-in-out')
// →
[{
  'transition-property': 'margin',
  'transition-duration': '3ms',
  'transition-timing-function': 'ease'
}, {
  'transition-property': 'padding',
  'transition-duration': '100ms',
  'transition-timing-function': 'ease-in-out'
}]

and...

shortcss.expand('transition', 'opacity 200ms ease')
// →
[{
  "transition-property": "opacity",
  "transition-duration": "200ms",
  "transition-timing-function": "ease"
}]

@tyv
Copy link

tyv commented Nov 26, 2016

i can take this issue

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

3 participants