sebastiaanvisser / clay

A CSS preprocessor as embedded Haskell.
Other
357 stars 72 forks source link

Fix rendering of the Number type #256

Closed ddssff closed 9 months ago

ddssff commented 10 months ago

Do not use the the Show instance of Clay.Property.Number for rendering. Instead add an instance of Pretty that uses the showFixed function of the underlying Fixed value. Adds a test case to spec/Clay/MediaSpec.hs that demonstrates the resulting output of the keyframes function.

turion commented 10 months ago

Ah, I wasn't aware the rendering of Number is broken. Can you give an example of what went wrong?

ddssff commented 9 months ago

Without fix:

Number {unNumber = 0.00000}% {visibility:visible;}

With fix:

0% {visibility:visible;}

It is assuming the automatically derived Show instance is a renderer, the way the show instance for Double is. In my opinion, it is better to use the Pretty printer for this.

ddssff commented 9 months ago

But you could just replace the call to show with showFixed True . unNumber

turion commented 9 months ago

I think even better would be to use fromText $ cssNumberText p. I definitely wouldn't want to incur an extra dependency for this.

ddssff commented 9 months ago

Whatever works!

ddssff commented 9 months ago

Ok, pretty dependency removed.

ddssff commented 9 months ago

Warnings fixed.

turion commented 9 months ago

Thanks a lot!