ghivert / sketch

A CSS-in-Gleam package, made to work with frontend, backend, with your favorite framework!
https://sketch.chouquette.dev
MIT License
50 stars 6 forks source link

feat: add initial transform API #23

Closed joseemds closed 2 months ago

joseemds commented 3 months ago

Here is a suggestion of the transform API, it was implemented based on css-transform-1 spec but i noticed that there is a newer version css-transform-2 spec that has some additions/changes.

From the first version of the spec, only the matrix() function is not implemented.

I dispose myself to update the current implementation to css-transform-2 after review of the API status, but would like to know if it is preferable to do it in this PR or create a new one.

joseemds commented 3 months ago

Having None inlined with TransformFunction would permit an API like:

sketch.transform([
  transform.none(),
  transform.rotateX(size.px(2.))
])

IMO an invalid state should not be representable, but it is a tradeoff. Maybe accept it and add documentation can be a workaround.

An alternative suggestion for the API is to expose two functions:

transform: Transform -> Style
transforms: List(Transform) -> Style

But the problem with inlining None would still exists. WDYT?

ghivert commented 3 months ago

Hmm, I did not thought about this… What about the empty list of transform functions? Isn't it the case where we'd like to output none? So we could have transform: fn (List(Transform)) -> Style, and when the list is empty, output none?

joseemds commented 3 months ago

This can be a valid solution, but IMO transform.list([]) is not transparent/explicit but documentation can fix it, i will implement it this way and we can discuss in the future other solutions

joseemds commented 3 months ago

Rebased on main

ghivert commented 3 months ago

To start, sorry for being missing so long & thanks a lot for your changes! I'd like the subject to advance before I make any change to Sketch in future, because I'm excited to get transform with a better API! 😁

I don't understand why there's so much noise in your patch set. It looks like your repo is not up-to-date with main branch. Could you try an other rebase to see if it removes the noise? Apart from this, one quick remark: there's no more transform function in sketch.gleam. I'd be happy to have transform(String) -> Style and transform_(List(Transform)) -> Style. Maybe with a comment (because it breaks Sketch conventions) "transform will be turned into transform_ in 4.0.0, and vice-versa. Meanwhile, it let you enjoy a better typed API through transform_!"?

joseemds commented 2 months ago

Hey, rebased on main and added transform function.

Now i hope everything is okay, please let me know if i should change anything

joseemds commented 2 months ago

Done!

ghivert commented 2 months ago

Thanks a lot!