[docs][base] Add Tailwind CSS & plain CSS demo on the Slider page #37736
Conversation
mnajdova
added
docs
| ref={ref} | ||
| className={clsx( | ||
| `absolute w-4 h-4 -ml-1.5 -mt-1.5 box-border rounded-full outline-0 border-3 border-solid border-current bg-white hover:shadow-outline-purple ${ | ||
| focusVisible || active ? 'shadow-outline-purple' : '' |
There was a problem hiding this comment.
I didn't want to use utils from our system, like the alpha, so I used the same shadow outline used in the other component. @danilo-leal @zanivan let me know if you think we should maybe add additional one, for distinguishing the styles between focus visible, active and hover.
There was a problem hiding this comment.
I don't think it's necessary to have more, as we don't have it, neither on plain CSS nor system.
Regarding the shadow-outline, on the switch, we're using rgba instead of hex value, so I converted the purple hex to rgb, and added the same 0.25 for the alpha. Would it be a problem? I thought it'd look more consistent with the other styles here.
Netlify deploy previewBundle size reportDetails of bundle changes (Toolpad) |
| @@ -126,6 +126,7 @@ const Slider = React.forwardRef(function Slider<RootComponentType extends React. | |||
| marked: marks.length > 0 && marks.some((mark) => mark.label), | |||
| dragging, | |||
| focusedThumbIndex, | |||
| activeThumbIndex: active, | |||
There was a problem hiding this comment.
@michaldudak similarly to focusedThumbIndex, I added the activeThumbIndex to allow custom component utilizing this information. I know we discussed something similar in the past, but I really feel like this calculated feels should be accessible in the slotProps callback. Maybe we can extend it so that apart from the ownerState, we have two additional states in the callback - active and focused so that we don't need to create custom components just for this calculation. We decided in the past that we should not include them in the ownerState, as they are not really a state of the owner component, but still having access to them feels important.
There was a problem hiding this comment.
From a developer's perspective, yeah, it is totally understandable that someone would expect to have an easy way of applying props based on the active state. Having them as additional parameters of the slotProps callback feels like the best way to achieve this. This way we won't pollute the ownerState with something that does not belong to the owner.
Additionally, for styling, we could make sure that the built-in CSS pseudoclasses (:active and :focus-visible) can be used. For now, I see that only :active works well.
There was a problem hiding this comment.
From a developer's perspective, yeah, it is totally understandable that someone would expect to have an easy way of applying props based on the active state. Having them as additional parameters of the slotProps callback feels like the best way to achieve this. This way we won't pollute the ownerState with something that does not belong to the owner.
Perfect, I will create a pull request for this soon.
Additionally, for styling, we could make sure that the built-in CSS pseudoclasses (:active and :focus-visible) can be used. For now, I see that only :active works well.
Yep
| @@ -102,7 +102,7 @@ async function transpileFile(tsxPath, program) { | |||
|
|
|||
| const propTypesAST = typescriptToProptypes.parseFromProgram(tsxPath, program, { | |||
| shouldResolveObject: ({ name }) => { | |||
| if (name === 'classes') { | |||
| if (name === 'classes' || name === 'ownerState') { | |||
There was a problem hiding this comment.
There were lint issues while generating the ownerState in the JavaScript files. Also, it cluttered a lot the demo, so I am skipping the ownerState here, so that it shows only as object in the propTypes. Check the failing build: https://app.circleci.com/pipelines/github/mui/material-ui/100746/workflows/17aaea13-a224-416a-9d33-c8634deba2d4/jobs/536745
Part of #37222