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

Compose rules with cx/css #355

Open
davesnx opened this issue Aug 10, 2023 · 1 comment
Open

Compose rules with cx/css #355

davesnx opened this issue Aug 10, 2023 · 1 comment
Labels
feature New feature help wanted Extra attention is needed

Comments

@davesnx
Copy link
Owner

davesnx commented Aug 10, 2023

I keep seeing this pattern, which tries to compose styles:

let headerName = style ([ fontWeight `normal ] @ Font.body)

I was thinking about having a way to do it inside styled-ppx (cx/styled.) like the following:

let font_body_16 = [%css {| font-size: 16px; |}];
let line_height_22 = [%css {|line-height: 22px;|}]

let cellHeader =
  [%cx {|
    $(font_body_16);
    $(line_height_22);
    display: flex;
    justify-content: space-between;
    align-items: center;
    padding-bottom: 12px;
    border-bottom: 1px solid $(Color.Border.primary);
  |}]

Interpolation right now works on values (and selectors), adding this would make interpolation possible on properties. I'm assuming only one property but it can be extended to deal with more than one.

@davesnx davesnx added feature New feature help wanted Extra attention is needed labels Aug 10, 2023
@davesnx
Copy link
Owner Author

davesnx commented Mar 7, 2024

Support multiple declarations on [%css ""]

(* Current implementation *)
css: string -> Css.Rule
[%css "display: block"] -> display(`block) (* Css.Rule *)

cx: array(Css.Rule) -> string (* hashed className *)
[%cx [| [%css "display: block"] |]] -> style(. [| display(`block) |])

(* Problem *)
%css can't contain more than one property.
It woudn't met the interface of `string -> Css.Rule`.

Possible solution I

Wrap every [%css] with a style() call, making css a string.
We would need to change cx as well, to make a "merge" of those styles.

Cons

  • More function calls (didn't benchmark performance on any level, but I belive those changes are the ones that can penalize)
  • We lose composability, Css.Rule isn't longer a thing in our users codebase.

Possible solution II

css: string -> Css.Rule
[%css "display: block"]
(* ----- *)
display(`block)

css: string -> array(Css.Rule)
[%css "display: block; width: 100%"]
(* ----- *)
[|display(`block), width(percent(100.))|]

Cons

  • What a silly polymorphism
  • Not sure If it helps in any way tbh.

Possible solution III

Treat all [%css ...] as array.

css: string -> [| Css.Rule |]
[%css "display: block"]
(* ----- *)
[| display(`block) |]

css: string -> array(Css.Rule)
[%css "display: block; width: 100%"]
(* ----- *)
[|display(`block), width(percent(100.))|]

Cons

  • Not sure how well compose with the rest of the syntaxes:
    • [%cx [%css ""]]
    • [%cx ""]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant