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: add support for svh, lvh, dvh, svw, lvw, dvw, svmin, lvmin, dvmin, svmax, lvmax, dvmax units #218

Merged
merged 1 commit into from
Aug 14, 2024

Conversation

geecko86
Copy link
Contributor

@geecko86 geecko86 commented Aug 11, 2024

... as well as vb, svb, lvb, dvb, vi, svi, lvi, dvi.

fix #217

I didn't add tests since you don't have any that's specific to vh and vw.

Copy link
Collaborator

@ludofischer ludofischer left a comment

Choose a reason for hiding this comment

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

Thank you! I think we need to still regenerate the parser.

@ludofischer ludofischer merged commit f5eaea1 into postcss:master Aug 14, 2024
4 checks passed
@geecko86
Copy link
Contributor Author

geecko86 commented Aug 16, 2024

Thank you! I think we need to still regenerate the parser.

Sorry I'm not familiar enough with the project - not sure I understand. Anything I can assist you with?

Also, would you kindly provide an ETA for a push of this commit to production? Thank you

@ludofischer
Copy link
Collaborator

Could you maybe post some sample CSS that you tried to parse, so I can add some tests, just to make sure we don't forget about it if we ever change the parser?
I realized after that nothing needs to be done with the parser is because the parser JavaScript code that runs when the plugin is used is generated from the grammar in the .jison file, but the generated parser code is not committed to the repository, instead the generator runs during the prepare which should run when someone publishes.

@geecko86
Copy link
Contributor Author

Sure. Here are the lines the plugin is outputting as errors/warning for my project:

Code:
  max-height:calc(98% - 1.5rem - (85svh/8.2 + 1.9rem + 1.65svh))!important
Code:
  padding:calc(1.9rem + 1.6svh) 0 0 0
Code:
  max-width:calc(100% - 2.75svh)
Code:
  bottom:calc(var(--table_height) + (50svmax - 42.5svmax) - (75svmax * .8))!important
Code:
  gap:calc(153.5svmax - var(--lamp-width))
Code:
  padding-left:calc(100svmax - 44.5svh)
Code:
  padding-left:calc(100svmax - 44.5svmin)
Code:
  max-height:calc(98% - 1.5rem - (85svh/8.2 + 1.9rem + 1.65svh))!important
Code:
  padding:calc(1.9rem + 1.6svh) 0 0 0
Code:
  max-width:calc(100% - 2.75svh)
Code:
  bottom:calc(var(--table_height) + (50svmax - 42.5svmax) - (75svmax * .8))!important
Code:
  gap:calc(153.5svmax - var(--lamp-width))
Code:
  padding-left:calc(100svmax - 44.5svh)

the generator runs during the prepare which should run when someone publishes.

Sorry to insist, but do you have a broad estimation of when that might happen? Thank you.

@ludofischer
Copy link
Collaborator

ludofischer commented Aug 16, 2024

Sorry to insist, but do you have a broad estimation of when that might happen? Thank you.

If you really need immediately I could do it this evening or tomorrow.

Is this issue preventing the minification to complete? I think it's supposed to skip minifying the code it does not understand and output a warning, but it should not crash the program. Or is the warning itself a a problem?

@geecko86
Copy link
Contributor Author

You're correct. The warning is not a problem in itself. I do however need to minify as much code as possible and was under the assumption that the plugin skipping lines was probably hurting its effectiveness.

If you really need immediately I could do it this evening or tomorrow.

That's very kind, thank you. I certainly don't mean to be pushy - but yeah when you have a moment for this, it would be much appreciated.

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.

[Bug]: Parse error when using dynamic viewport units (e.g. svh, lvw)
3 participants