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

Review: Classes DSL #6

Closed
cmeeren opened this issue Sep 1, 2019 · 2 comments
Closed

Review: Classes DSL #6

cmeeren opened this issue Sep 1, 2019 · 2 comments
Labels
help wanted Extra attention is needed

Comments

@cmeeren
Copy link
Collaborator

cmeeren commented Sep 1, 2019

Most MUI components have a classes prop that is used to set class names of MUI's internal sub-components (or for specific pseudo-selectors or component states). The allowed rule names vary per component.

Here's the JS native API:

<Button
  classes={{
    root: "foo"
    label: "bar"
  }}
>

Here's an example of the view DSL with the classes prop:

Mui.button [
  button.classes [
    classes.button.root "foo"
    classes.button.label "bar"
  ]
]

The button.classes prop is defined like this:

https://github.com/cmeeren/Feliz.MaterialUI/blob/adb43cba1d6b9aea2153da32a9987d608159ed92/src/Feliz.MaterialUI/Props.fs#L338

The actual classes are defined like this:

https://github.com/cmeeren/Feliz.MaterialUI/blob/adb43cba1d6b9aea2153da32a9987d608159ed92/src/Feliz.MaterialUI/Classes.fs#L116-L123

With a design like this, it's impossible to use incorrect rule names.

Is this design and implementation OK?


Ideally, I'd like to flatten it:

Mui.button [
  button.classes.root "foo"
  button.classes.label "bar"
]

But this of course causes problems in the prop generation, since classes is a sub-object.

I had an early hack going where I used prop names like "classes.root", and when creating the prop object from the IReactProperty list I extracted props with . in the name, split them on . and created a sub-object. The API was nice, but I was concerned about performance since this was done for every single prop list in the whole app. (I didn't measure anything, though.) Inputs on this approach are welcome.

@cmeeren cmeeren added the help wanted Extra attention is needed label Sep 1, 2019
@irium
Copy link

irium commented Oct 21, 2019

I'm ok with current solution. It's the same as prop.style usage.

@cmeeren
Copy link
Collaborator Author

cmeeren commented Jan 25, 2020

Continued in #39.

@cmeeren cmeeren closed this as completed Jan 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants