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.

Sign up for GitHub

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

Jump to bottom

Broken sort with min-width: 0 #57

Closed
ethanresnick opened this issue Jan 24, 2018 · 3 comments · Fixed by #58
Closed

Broken sort with min-width: 0 #57

ethanresnick opened this issue Jan 24, 2018 · 3 comments · Fixed by #58
Labels
bug

Comments

@ethanresnick
Copy link

I realize this media query will always match, so it's a bit weird to even have it, but I do (for irrelevant reasons), and it doesn't seem to be sorted correctly.

I believe the issue is that this line checks !minWidth, so min-width: 0 is treated as though the min-width isn't specified at all.
ethanresnick added a commit to ethanresnick/column-setter that referenced this issue Jan 24, 2018
@ethanresnick
Don’t output needless (min-width: 0) in media queries
0871bb7 
Don’t output needless (min-width: 0) in media queries. This doesn’t
play well with some tools (e.g.
hail2u/node-css-mqpacker#57).
@hail2u hail2u added the bug label Jan 25, 2018
hail2u added a commit that referenced this issue Jan 25, 2018
@hail2u
Treat 0 as correct CSS length
9a243ae 
`(min-width: 0)` is parsed and picked up correctly, but was inspected
as a invalid CSS length whereas 0 is a valid CSS length. MQPacker should
special-case `0` (only `0`).

This fixed #57.
hail2u added a commit that referenced this issue Jan 25, 2018
@hail2u
Treat 0 as correct CSS length
3b2cbf7 
`(min-width: 0)` is parsed and picked up correctly, but was inspected
as a invalid CSS length whereas 0 is a valid CSS length. MQPacker should
special-case `0` (only `0`).

This fixed #57.
@hail2u hail2u mentioned this issue Jan 25, 2018
Merged
@hail2u
Copy link
Owner

hail2u commented Jan 25, 2018

Thanks, 0 should be special-cased. This will ship as soon as possible.

@ethanresnick
Copy link
Author

ethanresnick commented Jan 25, 2018 •
edited
Loading

Thanks for responding to this so promptly! Looking at your fix, though, am I right to conclude that it'll only handle the case of min-width: 0, and not, e.g., min-width: 0px or min-width: 0em? If so, shouldn't it handle those other cases (i.e., 0 regardless of unit) as well?

@hail2u
Copy link
Owner

hail2u commented Jan 25, 2018

This bug only occurs with min-width: 0, not with min-width: 0px. And I think min-width: 0px etc. have been handled correctly in older releases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Treat 0 as correct CSS length hail2u/node-css-mqpacker
2 participants
@hail2u @ethanresnick