[base-ui] Improve recommended usage guide #38570
Conversation
oliviertassinari
added
docs
oliviertassinari
changed the title
Netlify deploy previewBundle size report |
| @@ -61,6 +62,8 @@ In this example we're using the [clsx](https://www.npmjs.com/package/clsx) utili | |||
| Use [`slotProps`](#customizing-slot-props) to apply custom styles using [Tailwind CSS](https://tailwindcss.com/), as shown below: | |||
|
|
|||
| ```tsx | |||
| import { Switch as UnstyledSwitch, SwitchOwnerState } from '@mui/base/Switch'; | |||
There was a problem hiding this comment.
Make it easier to start
| const StyledSwitch = styled(Switch)` | ||
| const Switch = styled(UnstyledSwitch)` |
There was a problem hiding this comment.
This makes more sense to me. These components are likely to be used called Switch in the final app pages.
There was a problem hiding this comment.
I usually use BaseSwitch in these contexts. Would it work for you as well?
There was a problem hiding this comment.
I think we can use this instance to come with a standard for all the demos in the documentation. I see two decisions we can make:
- prefix vs. suffit: BaseSwitch vs. SwitchBase
- Name: UnstyledSwich vs. BaseSwitch vs. else
Prefix makes sense for me, it makes it easier to spot what's styled and what's not.
Base references the projet name, it makes sense. It's also faster to type and read.
I think we can bring in @samuelsycamore, @mnajdova, @mj12albert on this discussion, they have also created demos.
There was a problem hiding this comment.
I like BaseSwitch the best 👍
Would it be const Switch = styled(BaseSwitch) then? That looks great to me.
There was a problem hiding this comment.
Alright BaseSwitch, I'm renaming it in this PR.
I think that we should update the rest of the docs/codebase. I have created #38904 to keep track of it.
| const Input = styled('input')` | ||
| const SwitchInput = styled('input')` |
There was a problem hiding this comment.
For consistency with the other demos. But I guess it's also simpler to assume a global namespace. I hope that we have the static CSS extraction, we will be able to use this name to help debug.
| return ( | ||
| <BasicSwitchRoot className={clsx(stateClasses)}> | ||
| <BasicSwitchThumb className={clsx(stateClasses)} /> | ||
| <BasicSwitchInput {...getInputProps()} aria-label="Demo switch" /> | ||
| </BasicSwitchRoot> | ||
| <SwitchRoot className={clsx(stateClasses)}> | ||
| <SwitchThumb className={clsx(stateClasses)} /> | ||
| <SwitchInput {...getInputProps()} aria-label={props['aria-label']} /> | ||
| </SwitchRoot> | ||
| ); | ||
| } | ||
|
|
||
| export default function StylingHooks() { | ||
| return <BasicSwitch />; | ||
| return <Switch aria-label="Demo switch" />; | ||
| } |
There was a problem hiding this comment.
The custom aria-label value inside the Switch component didn't make sense to me. This is can't be part of the design system.
Actually, this makes me wonder why in Base UI switch we aren't forwarding all the props to the hook. I find this strange:
material-ui/packages/mui-base/src/Switch/Switch.tsx
Lines 69 to 80 in 5c96f3c
It could be this, no?
const { getInputProps, checked, disabled, focusVisible, readOnly } = useSwitch(props);| thumb: { className: 'thumb' }, | ||
| input: { className: 'input' }, |
There was a problem hiding this comment.
This seems unnecessary and not the best practice.
There was a problem hiding this comment.
By default, yes, but you can disable generation of default classes.
From looking at #38138 (comment)