search

Shmew / Feliz.MaterialUI

Feliz-style Fable bindings for Material-UI
https://shmew.github.io/Feliz.MaterialUI/
MIT License
70 stars 19 forks source link

Review: Children prop overloads #9

Closed cmeeren closed 4 years ago

cmeeren commented 5 years ago

All components that support children have their own children props (partly because the documentation may contain relevant component-specific information). The children props accept node acording to the MUI docs, and currently have these overloads:

https://github.com/cmeeren/Feliz.MaterialUI/blob/adb43cba1d6b9aea2153da32a9987d608159ed92/src/Feliz.MaterialUI/Props.fs#L53-L64

Does that make sense?

I have also used the same overloads for other children-like props that accept node, such as the cardHeader props action/avatar/subheader/title.

Also, I'm wondering if my use of prop.children for two of the overloads is good.

Zaid-Ajaj commented 4 years ago

with 

static member inline children(elements: ReactElement seq) = prop.children elements

wouldn't React complain about the children not having a key? I would need to try it out but in Feliz we are using the helper function React.Children.toArray to add custom keys automatically for these nodes

cmeeren commented 4 years ago

Look closer - it's delegating to prop.children defined in Feliz. :)

Zaid-Ajaj commented 4 years ago

Hahah indeed, never mind then

cmeeren commented 4 years ago

Do you find the rest of the questions/API to make sense? Should I close this?

cmeeren commented 4 years ago

Closing this since I haven't heard anything. The only thing I'm unsure about is the string seq overload, which might not be useful. AFAIK in some cases it'll be more efficient than string concatenation, but I'm not sure how common and significant this is.