-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[docs] Add section about overriding slots to base concepts #10421
[docs] Add section about overriding slots to base concepts #10421
Conversation
I am opening this one up for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for taking the time to improve this misleading and mysterious concept. 🙈
@@ -1,10 +1,10 @@ | |||
--- | |||
productId: x-date-pickers | |||
title: Date and Time Pickers - Custom subcomponents | |||
title: Date and Time Pickers - Custom slots and subcomponents |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about not renaming the menu item and it's heading? 🤔
Technically, almost all the Visual customization
pages are about custom slots. 🙈
Maybe it's already time to rename all or most of the components
word mentions on this page into slots
? 🤔
Or at least only rename the section below to Overriding slot components
or Overidding slots
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am somewhat in favor of leaving at least some mention of the word 'subcomponent' 🤔
I am not sure how easy it is for someone who is starting out on this doc to make the connection between slot
and subcomponent
🤔
I am open to suggestions on this
Technically, almost all the Visual customization pages are about custom slots
Yeah, that is true. I feel like the other pages are more focused on specific use-cases. This one includes more scenarios and is somewhat of a gateway into understanding slots better. It would still make sense to have the slots mentioned in the menu as well 🤔
rename the section below to Overriding slot components or Overidding slots
totally agree 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you agree that eventually, this page will become an home page for slot customization but all the actual examples will be moved to standalone pages (like we started doing)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, it probably will. And I would be in favor of using the term 'components' less and less, but maybe removing 'subcomponents' entirely from the headings is not the way to go 🤔 I feel like it's clear to us that slots are actually building blocks that you can override, but is it clear to users at a first look? So if somebody is on the docs, trying to figure out how to replace the openPickerIcon
, which menu item is more self-explanatory 'Custom slots' or 'Custom slots and subcomponents'?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that "slot" is not understandable by people reaching to the doc for the 1st time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there are no other objections, I'm fine with the suggested renaming. 👌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about using a verb to describe what the user would actually do with this information? For example, the Base and Joy docs on this topic use the title "Overriding component structure", which IMO gives the reader a much better idea of how to apply this information, and doesn't require any knowledge of MUI-specific terminology.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overriding component structure
is definitely a super valid option 💯 But in our case, I feel like it would add a bit of inconsistency, especially since the Visual customization
section includes a number of other pages, most of them named like this:
I like the idea of introducing the word 'slots' to suggest the link between slots and subcomponents (also helps the search of either of these terms), and revisiting this topic in a broader refactor of the docs structure. WDYT about this?
Co-authored-by: Lukas <[email protected]> Signed-off-by: Nora <[email protected]>
…noraleonte/mui-x into add-slots-customization-example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Big improvement! I left a couple style suggestions here.
Also: have you seen this page on Overriding component structure from the Base UI docs? I think it's helpful to present the mental model for slots, particularly this line:
"A component's structure is determined by the elements that fill that component's slots. Slots are most commonly filled by HTML tags, but may also be filled by React components."
Maybe it would be worth adding this line and/or a link back to this Base UI doc here?
Also: please feel free to ping me for a review on any and all docs PRs over here on the X side of things! 💙 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice improvement! 🙌
@@ -1,10 +1,10 @@ | |||
--- | |||
productId: x-date-pickers | |||
title: Date and Time Pickers - Custom subcomponents | |||
title: Date and Time Pickers - Custom slots and subcomponents |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there are no other objections, I'm fine with the suggested renaming. 👌
Co-authored-by: Lukas <[email protected]> Signed-off-by: Nora <[email protected]>
…-slots-customization-example
Co-authored-by: Sam Sycamore <[email protected]> Signed-off-by: Nora <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small nitpick on style here but otherwise I'd say it's good to go!
closes #9285