[base-ui][Button] Make demos more readable, modern, and robust #40742

colmtuite · 2024-01-22T17:41:27Z

[WIP]

Rename the root class to “Button”, so it’s more copy/pasteable for the target audience.
Replace MUI Stack with a simple div, and remove the import.
Apply cursor: not-allowed to disabled controls.
Apply more modern, robust, and functional CSS (reset, flex, box-sizing, vertical-align, flex-shrink, user-select etc.)
Remove cursor: pointer (that value is intended to indicate a link)
Convert longhand CSS properties to shorthand for improved editing and readability.
Convert all px values to rem.
Remove unnecessary 0 from decimal values.
Remove transform: scale effect (browsers rasterise text before scaling it, so it looks weird/blurry).
Alphabetise CSS declarations.
Set component size based on an 8px baseline for to enable and centering, and for consistency across all UI controls.
Replace transition-property: all with explicit declarations for improved performance and manipulation.
Remove <React.Fragment />.
Use outline for focus styles to enable offset focus rings, and clean up the box-shadow values.
Improve shadow rendering by declaring multiple blended values, increasing saturation, and setting background-clip: padding-box, so shadows can bleed through borders.
Wrap hover styles in a media query.
Wrap transitions in a media query.

Currently looking for feedback on these changes before I copy the changes to all button demos.

https://deploy-preview-40742--material-ui.netlify.app/base-ui/react-button/#introduction

danilo-leal

Looking good — first PR! 🎉

Left just a couple of small comments. I think it'd be great to consider documenting/discussing the alphabetical CSS sorting — I don't have any preference from the get-go, but whatever path we take, we should probably try to standardize as much as we can throughout the docs!

docs/data/base/components/button/UnstyledButtonIntroduction/css/index.js

michaldudak · 2024-01-23T09:48:57Z

docs/data/base/components/button/UnstyledButtonIntroduction/css/index.js

+        transition-property: background-color, box-shadow;
+        transition-timing-function: ease;
+      }
+      @media (any-hover: hover) {


Im curious what's the point of @media (any-hover: hover)? If no input device supports hover, these styles won't be applied anyway, right?

There are lots of problems on mobile devices and other devices that support touch but not hover. The effect can often "stick" to a component when you touch a UI control, or touch close to a control, then slide over it.

mnajdova

The Tailwind focus indicator is not consistent with the other two styling engines (I like the system's one better):

mnajdova · 2024-01-24T11:47:36Z

docs/data/base/components/button/UnstyledButtonIntroduction/css/index.js

-    box-shadow: 0 2px 1px ${
-      isDarkMode ? 'rgba(0, 0, 0, 0.5)' : 'rgba(45, 45, 60, 0.2)'
+      .Button {
+        all: unset;


I am curious what's the motivation for adding this to the demos. It would break the default presets people may have in their projects.

This is just a quick way to reset the UA styles, rather than going through and declaring each reset style one-by-one.

It would break the default presets people may have in their projects.

Any styles they need would be declared below this all: unset style. imo, in a component-based architecture, people shouldn't rely on leaky/global CSS to style elements. All styles they need should be self-contained and encapsulated in the component, so you can drop it into any screen/environment and it just works.

Makes sense to me. I mentioned this because it conflicts with other parts of the docs where we promote using the CssBaseline component, for e.g. in Material UI. We should be consistent with this across the products in my opinion.

mnajdova · 2024-01-24T11:49:23Z

docs/data/base/components/button/UnstyledButtonIntroduction/css/index.js

-    border-radius: 8px;
-    color: white;
-    transition: all 150ms ease;
-    cursor: pointer;


Should we start the cursor: pointer discussion? 😄 I am just joking, I would love to follow the default browser behavior, however just noting that this is opposite to what we use in Material UI, which kind of feels better 😄

Should we start the cursor: pointer discussion?

I think we just did lol

however just noting that this is opposite to what we use in Material UI

imo buttons should not have cursor: pointer, but it's not a hill I want to die on haha. So you choose, just say the word and I will add it?

This and the not-allowed cursor have been discussed several times in the past (I could find #17871, #27719 (comment), #32371), so there's no way we could make everyone happy :)

Having our solution use the patterns found in the native browser or OS controls feels the safest, though.

mnajdova · 2024-01-24T11:53:05Z

docs/data/base/components/button/UnstyledButtonIntroduction/css/index.js

+        user-select: none;
+        vertical-align: middle;
+      }
+      @media (prefers-reduced-motion: no-preference) {


💯 I remember a time when I needed to create an animation context with a switch on/off that would remove all transitions automatically. Life's much easier now 😄

Btw, we would add the @media on the other demos (tailwind and system) - this is the pain point that would be solved if we have an automatic translation to System & Tailwind based on the plain CSS demo.

danilo-leal

Looks great! Great improvements all around!

Not sure if you were meaning to use this PR just for the introduction demos, but we should make sure every other demo on the page is using the same adjusted styles (tedious, I know 😕 but the docs-infra will tackle this soon).

Additionally, lots of CSS-specific quirks/strategies were discussed in here, pointing to potential "universal" guidelines for styling the Base UI demos. I think we should document them somewhere with explanations so we can reference them later on — I'm particularly thinking of a page in the Base UI Notion space to start (edit: started to do this here). Not sure whether it'd be valuable to expose this as a new page in the docs.

colmtuite added 4 commits January 22, 2024 16:03

Plain css ts example

6dc2898

plain css js demo

6f2aff7

mui system demos

3bc023d

TW example

17e15bb

colmtuite requested review from michaldudak, mnajdova and danilo-leal January 22, 2024 17:57

danilo-leal changed the title ~~[BaseUI][Button] Make docs demos more readable, modern, and robust.~~ [base-ui][Button] Make demos more readable, modern, and robust Jan 22, 2024

danilo-leal added docs Improvements or additions to the documentation component: button This is the name of the generic UI component, not the React module! package: base-ui Specific to @mui/base labels Jan 22, 2024

danilo-leal reviewed Jan 22, 2024

View reviewed changes

docs/data/base/components/button/UnstyledButtonIntroduction/css/index.js Show resolved Hide resolved

docs/data/base/components/button/UnstyledButtonIntroduction/css/index.js Show resolved Hide resolved

michaldudak reviewed Jan 23, 2024

View reviewed changes

mnajdova reviewed Jan 24, 2024

View reviewed changes

danilo-leal approved these changes Jan 24, 2024

View reviewed changes

colmtuite mentioned this pull request Feb 1, 2024

[base-ui][docs] Remove redundant "Styled" prefix #40478

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[base-ui][Button] Make demos more readable, modern, and robust #40742

[base-ui][Button] Make demos more readable, modern, and robust #40742

colmtuite commented Jan 22, 2024 •

edited by danilo-leal

danilo-leal left a comment

michaldudak Jan 23, 2024

colmtuite Jan 23, 2024

mnajdova left a comment

mnajdova Jan 24, 2024

colmtuite Jan 24, 2024 •

edited

mnajdova Feb 5, 2024

mnajdova Jan 24, 2024

colmtuite Jan 24, 2024

michaldudak Jan 24, 2024

mnajdova Jan 24, 2024

mnajdova Jan 24, 2024 •

edited

danilo-leal left a comment •

edited

[base-ui][Button] Make demos more readable, modern, and robust #40742

Are you sure you want to change the base?

[base-ui][Button] Make demos more readable, modern, and robust #40742

Conversation

colmtuite commented Jan 22, 2024 • edited by danilo-leal

danilo-leal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnajdova left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colmtuite Jan 24, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnajdova Jan 24, 2024 • edited

Choose a reason for hiding this comment

danilo-leal left a comment • edited

Choose a reason for hiding this comment

colmtuite commented Jan 22, 2024 •

edited by danilo-leal

colmtuite Jan 24, 2024 •

edited

mnajdova Jan 24, 2024 •

edited

danilo-leal left a comment •

edited