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

seconds unit dropped for :animation when pretty print turned off when magnitude is X.0 #119

Closed
thedavidmeister opened this issue Oct 22, 2016 · 9 comments · Fixed by #181
Milestone

Comments

@thedavidmeister
Copy link

I tried this:

{:animation [[:my-keyframes (u/s 3.0) :infinite :ease-in-out]]}

Works with pretty print turned off, but when it is turned on, the s for 3.0s gets dropped.

Changing the magnitude from 3.0 to 3 or 3.5 works. Seems like anything that ends in .0 drops the s when compiled without pretty print.

@thedavidmeister
Copy link
Author

:transition seems to have the same problem, so i don't think it has anything to do with the rule, just the way units are handled.

@noprompt
Copy link
Owner

@thedavidmeister Thanks for opening up all of these issues. It would really help if you gave me some steps to reproduce. I need more than a snippet. I need the exact code that failed and what you expected the result to be.

noprompt added a commit that referenced this issue Nov 29, 2016
This patch rethinks the architecture of Garden in terms of well
defined boundaries between parsing and compiling. Instead of
directly compiling objects into CSS, Garden will first parse the objects
into a CSS AST which can then be consumed by the compiler. This
architecture is closer to what one might expect to find in a
programming language implementation setting. This new architecture
yields greater flexibility to introduce new features and semantics
while vastly simplifying the work of the compiler.

As a result many requested features and solutions to bugs have now
been either implemented, solved, or are now trivial to implement or
solve.

Changelog updates forthcoming.

Closes #31
Closes #109
Closes #115
Closes #116
Closes #119
Closes #120
Closes #121
@noprompt noprompt added this to the v2.0.0 milestone Nov 29, 2016
@RadicalZephyr
Copy link

Not sure if you still need more to reproduce this issue, but I just noticed it as well.

(defstyles screen
    [:.test
   {:thing "10.0s"
    :two "10s"}])

produced:

.test {
    thing: 10.0;
    two: 10s
}

where it should have been:

.test {
    thing: 10.0s;
    two: 10s
}

(note the additional "s" in the value of the thing property)

@rafd
Copy link

rafd commented Jun 24, 2019

Seems to also be an issue with non-animations:

(garden/css {:pretty-print? false} [:body {:font-size "1.0em"}])

Gives:

body{font-size:1.0}

Which is not valid.

It seems this is a YUI compressor issue: yui/yuicompressor#322

My workaround has been to do: 1em

@opqdonut
Copy link

bumped into this same issue with px:

(g/css {:pretty-print? false} (list [:foo {:bar (u/px 1200.0)}]))

generates

foo{bar:1200.0}

@noprompt
Copy link
Owner

YUI has been a continual source of pain and suffering. I think we should just cut it loose and continue without it. For 1.X.X releases we can continue with the simple compression provided with the goal of just getting Garden out of the compression business long-term.

Currently, I have a time obligation to Meander for the next couple months and, honestly, I'm not going to be able to devote any time to Garden until after Q3.

I would really appreciate someone taking the wheel on this one and eliminating YUI from the project.

@pavel-klavik
Copy link
Contributor

This bug is very annoying because it is not something one suspects. While the long term solution of cutting YUI can be great, it would be great to add a quick hack which would fix it. It should be enought to add a check when converting maps describing units into strings whether the value is a float which is actually an integer, and if so, to automatically convert it into int. Something like this:

(if (and (float? value) (== value (int value))
  (int value)
  value)

@noprompt
Copy link
Owner

noprompt commented Jan 1, 2021

@pavel-klavik Are you able to submit a patch for this? If your proposal works to solve the issue, I'm happy to merge it.

pavel-klavik added a commit to pavel-klavik/garden that referenced this issue Jan 1, 2021
A work around which detects floats ending with .0 and converts them to int. Without this, when :pretty-print? is set to true, YUI removes the CSS unit making the output invalid. This fixes noprompt#119.
@pavel-klavik
Copy link
Contributor

This should work, tested in REPL. I am not sure how to actually build Garden for development, so it would be great if someone can test it fully or maybe adding automatic tests??

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants