-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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][base] Add Tailwind CSS & plain CSS demos on the Input page #37685
[docs][base] Add Tailwind CSS & plain CSS demos on the Input page #37685
Conversation
Netlify deploy previewBundle size report |
Submitted a PR for this on #37688
![]() |
One more thing, as I was reviewing it on mobile, I noticed that the demo container and its toolbar are not displayed, and therefore, the style selector is missing as well. Overall, I think it makes sense, but perhaps for Base UI we should have a way to switch demo styles, considering it's one of the major advantages of using it. What are your thoughts, @danilo-leal @mnajdova? |
Uhm, good point about the styling menu not being shown on mobile. The demo toolbar was already not displayed but it might make sense to at least have the option to toggle the styles 🤔 We can think about that in parallel (not within the scope of this PR). |
theme: { | ||
extend: { | ||
boxShadow: { | ||
'outline-purple': '0 0 0 3px #c084fc', |
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.
@danilo-leal @zanivan I noticed that we use different strategy for the outlined shadow, for e.g. here we use boxShadow, while in the select we use outline. Should we normalize this?
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.
For sure, yes! It seems box-shadow is a safer approach, right (due to outlines not following border radii on Safari)?
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.
Cool, I followed this in the select PR.
@zanivan @danilo-leal is there something else worth addressing here? We'll need to merge this one before the select, as it uses the same extended theme for the focus visible outline :) |
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.
It looks good to me!
I added a custom box-shadow value in the theme for the outline - it's used in more components, so I believe it makes sense.
Preview: https://deploy-preview-37685--material-ui.netlify.app/base-ui/react-input/#basics
Part of #37222