[system] Add styled primitives #27207

siriwatknp · 2021-07-10T03:08:02Z

related to #23220 (will close in the future).

Quick summary
This api gives a better DX than Box with the same functionality

<Box component="span" display="flex" sx={{ ... }}>
  ...long children
</Box> // what is the Box

// This looks closer to html and more declarative when the children is long.
<mui.span display="flex" sx={{ ... }}>
  ...long chilren
</mui.span>

I would like this API to be used internally for rebranding first and see the result, so no documentation is needed in this PR (might be official in v5.x). The purpose of this draft to discuss on the API interface. Here are some options that I can think of

Option 1

We should be careful about this name because it will be used in second design system. But given the scope might be changed to @mui so this name could work.

import { mui } from '@material-ui/core/primitives';
// import { mui } from '@material-ui/core'; // is also possible

<mui.div display="flex" sx={{ alignItems: 'center' }} />

Option 2

similar to xstyled. It is very convenient to write but could be double-edged.

import { x } from '@material-ui/core/primitives';
// import { x } from '@material-ui/core'; // is also possible

<x.div display="flex" sx={{ alignItems: 'center' }} />

Option 3

use core/styles path

import { mui } from '@material-ui/core/styles';

<mui.div display="flex" sx={{ alignItems: 'center' }} />

@mui-org/core any thought?

I have followed (at least) the PR section of the contributing guide.

mui-pr-bot · 2021-07-10T03:09:55Z

Details of bundle changes (experimental)

@material-ui/system: parsed: +0.73% , gzip: +0.65%

Generated by 🚫 dangerJS against 9ea1f11

siriwatknp · 2021-07-10T03:13:36Z

test/utils/initMatchers.ts

-            | number
-          >,
-          string
+        expectedStyle: Partial<


It should have Partial, otherwise it expects all of the css properties.

mnajdova · 2021-07-10T03:42:23Z

packages/material-ui-styled-engine/src/index.js

@@ -25,6 +25,10 @@ export default function styled(tag, options) {
  return stylesFactory;
 }

+Object.keys(emStyled).forEach((tag) => {


Not sure why I didn’t add this before. 👍

mnajdova · 2021-07-10T03:43:10Z

packages/material-ui/src/primitives/primitives.ts

+
+  styled.htmlTags.forEach((tag) => {
+    // @ts-ignore
+    mui[tag] = styled(tag)();


I am wondering what is the bundle size of this when we import it

eps1lon · 2021-07-12T16:12:36Z

I don't see any value in

-Box component="span"
+mui.div

Especially because the first one is an existing pattern in Material-UI. The new API is yet another completely new API that doesn't solve any problems that aren't solved with Box. The breaking change seems not worth it. Adding this API while keeping Box is even worse.

oliviertassinari · 2021-07-12T21:01:44Z

The main chunk of value that I can find is regarding:

the closing tag: [system] Support sx on all primitive elements #23220 (comment)
makes it easier to switch between a naked primitive and a styled one.

At least, it's the two pain points it solves for me when I was working on implementing the branding pages. Is this enough to overweight the possible cons it has? I don't know. What are the cons?

It seems that Box is useless if we introduce this API. We would need to remove it.

This demo: https://next.material-ui.com/system/basics/#demo turns into:

import * as React from 'react';
import Box from '@material-ui/core/Box';
import { alpha } from '@material-ui/core/styles';
import ErrorOutlineIcon from '@material-ui/icons/ErrorOutline';

export default function Demo() {
  return (
    <mui.div
      sx={{
        display: 'flex',
        flexDirection: { xs: 'column', md: 'row' },
        alignItems: 'center',
        bgcolor: 'background.paper',
        overflow: 'hidden',
        borderRadius: '12px',
        boxShadow: 1,
        fontWeight: 'bold',
      }}
    >
      <mui.img
        sx={{
          height: 233,
          width: 350,
          maxHeight: { xs: 233, md: 167 },
          maxWidth: { xs: 350, md: 250 },
        }}
        alt="The house from the offer."
        src="https://images.unsplash.com/photo-1512917774080-9991f1c4c750?auto=format&w=350&dpr=2"
      />
      <mui.div
        sx={{
          display: 'flex',
          flexDirection: 'column',
          alignItems: { xs: 'center', md: 'flex-start' },
          m: 3,
          minWidth: { md: 350 },
        }}
      >
        <mui.span sx={{ fontSize: 16, mt: 1 }}>
          123 Main St, Phoenix AZ
        </mui.span>
        <mui.span sx={{ color: 'primary.main', fontSize: 22 }}>
          $280,000 — $310,000
        </mui.span>
        <mui.div
          sx={{
            mt: 1.5,
            p: 0.5,
            backgroundColor: (theme) => alpha(theme.palette.primary.main, 0.1),
            borderRadius: '5px',
            color: 'primary.main',
            fontWeight: 'medium',
            display: 'flex',
            fontSize: 12,
            alignItems: 'center',
            '& svg': {
              fontSize: 21,
              mr: 0.5,
            },
          }}
        >
          <ErrorOutlineIcon />
          CONFIDENCE SCORE 85%
        </mui.div>
      </mui.div>
    </mui.div>
  );
}

It feels a bit weird at first when reading, but we can get used to it.

I think that we could ask the community on Twitter about this move.

eps1lon · 2021-07-13T06:17:40Z

the closing tag: [system] Support sx on all primitive elements [system] Support sx on all primitive elements #23220 (comment)

makes it easier to switch between a naked primitive and a styled one.

Could you elaborate on either one. These statements lack context.

oliviertassinari · 2021-07-15T14:15:48Z

Could you elaborate on either one. These statements lack context.

@eps1lon Yes, sure:

the closing tag. This is most valuable when you can't see the opening tag of the Box. For instance, when using the <footer> primitive. Would you rather see:

a.

              license: (
                <Link
                  color="text.primary"
                  underline="hover"
                  href={`https://github.com/mui-org/material-ui/blob/v${process.env.LIB_VERSION}/LICENSE`}
                >
                  {t('license')}
                </Link>
              ),
            }}
          >
            {t('homeFooterRelease')}
          </Interpolate>
          {' Copyright © '}
          {new Date().getFullYear()}
          {' Material-UI. '}
        </Typography>
      </Box>
    </Box>
  );
}

or b?

              license: (
                <Link
                  color="text.primary"
                  underline="hover"
                  href={`https://github.com/mui-org/material-ui/blob/v${process.env.LIB_VERSION}/LICENSE`}
                >
                  {t('license')}
                </Link>
              ),
            }}
          >
            {t('homeFooterRelease')}
          </Interpolate>
          {' Copyright © '}
          {new Date().getFullYear()}
          {' Material-UI. '}
        </Typography>
      </mui.div>
    </mui.footer>
  );
}

I would personally rather see b.

switch between a naked primitive and a styled one. On this one, it's about the DX when you need to add styles on a primitive. Would you rather have this UX:

a.

box.mp4

or b?

mui.mp4

I would personally rather do b.

Actually, I think that we should also involve the perspective of @danilo-leal at it touches the UX on the product.

eps1lon · 2021-07-17T09:59:59Z

I get that. But it's nowhere near enough reason for a breaking change. Especially because this applies to all components implementing component

oliviertassinari · 2021-07-19T10:52:40Z

enough reason for a breaking change

Is it enough for a breaking change? I think is this is the main product question we still need to resolve.

I think that the following would better help navigate the space:

Can we write a codemod to automatically migrate all the codebases? It seems that the answer is yes. We could only make Box deprecated in v5.
Does it relates to <Stack component="span" /> or the other components that implement the component prop? I don't see a strong link. The two issues we have with Box are no impacting these components because they are specialized. We already have the value of the closing tag, and the switch between a primitive is a lot less likely to happen.
Would the community support the change? I think that we should submit the idea to them, and gather feedback. It might be what matters the most ultimately. They might be against the simple notion of a change, not wanting to change habits, I think that we should discount it as not long-term focused.

@siriwatknp do you want to keep exploring and own the final product decision?

siriwatknp · 2021-07-19T13:05:22Z

@siriwatknp do you want to keep exploring and own the final product decision?

@oliviertassinari @eps1lon I don't plan to introduce breaking change or public API in this PR. My plan is to make it internal so that I can try it out for rebranding purpose. However, if it is a hard NO from the team, I can close the this PR.

danilo-leal · 2021-07-19T14:30:27Z

Well, if I could add something is that it is definitely easier for reading. Especially when using the HTML tags that already exist, like mui.article, mui.section, etc. It's easier for me. The only worry I'd have is regarding its rollout, Box looks very established?! Not that sure but it's also something only to worry about in the future. Definitely agree with checking in with the community before making hard decisions. I'd think that this would impact somewhat significantly DX, so that care is much appropriated.

eps1lon · 2021-07-19T16:29:55Z

I just think that this isn't the right time to introduce an API just adds another choice. Maybe a couple of weeks after v5 release but right now is not the time to continue to go in the same direction (just add more API surface) without having any feedback if the current direction works.

siriwatknp · 2021-07-20T09:45:51Z

Maybe a couple of weeks after v5 release

closed this PR and we can revisit again after v5.

oliviertassinari · 2021-09-29T10:27:40Z

With v5 being out, I would propose that we wait a couple of months before exploring this again. If we do it too soon, we might trigger this response:

Developers might be confused that we start promoting a new API, right after we released a new one. They might think that we don't know what we are doing. If this was the API they truly wanted to push, why didn't they make the change before the stable release? At least, I think that it's how I would interpret the change, if say, React was doing something similar. (remember the new classes API just before they introduced the hooks 😅)

So I propose we wait. Plus, we make it more likely for developers to experience the pain, and to see value in the change.

In the meantime, we could, for instance look into improving the performance of the existing API 😇. Explore ways to improve the Lighthouse score on the homepage.

siriwatknp added 4 commits July 9, 2021 19:02

wip

608c4f8

attach primitives to styled-engine

18a73e6

add styled primitive

78af401

remove dev page

579e1ca

siriwatknp commented Jul 10, 2021

View reviewed changes

siriwatknp added 2 commits July 10, 2021 10:20

fix types build error

c308226

export Mui type

9ea1f11

mnajdova reviewed Jul 10, 2021

View reviewed changes

github-actions bot mentioned this pull request Jul 10, 2021

Update snapshots for pr/27207 eps1lon/mui-scripts-incubator#2764

Closed

oliviertassinari changed the title ~~[core] add styled primitives~~ [system] add styled primitives Jul 10, 2021

oliviertassinari changed the title ~~[system] add styled primitives~~ [system] Add styled primitives Jul 10, 2021

oliviertassinari added design This is about UI or UX design, please involve a designer package: system Specific to @mui/system and removed design This is about UI or UX design, please involve a designer labels Jul 10, 2021

siriwatknp requested review from oliviertassinari, eps1lon and michaldudak July 12, 2021 14:09

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 15, 2021

siriwatknp closed this Jul 20, 2021

oliviertassinari mentioned this pull request Sep 29, 2021

[system] Support sx on all primitive elements #23220

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[system] Add styled primitives #27207

[system] Add styled primitives #27207

siriwatknp commented Jul 10, 2021 •

edited

Loading

mui-pr-bot commented Jul 10, 2021 •

edited

Loading

siriwatknp Jul 10, 2021

mnajdova Jul 10, 2021 •

edited

Loading

mnajdova Jul 10, 2021

eps1lon commented Jul 12, 2021 •

edited

Loading

oliviertassinari commented Jul 12, 2021 •

edited

Loading

eps1lon commented Jul 13, 2021

oliviertassinari commented Jul 15, 2021 •

edited

Loading

eps1lon commented Jul 17, 2021 •

edited

Loading

oliviertassinari commented Jul 19, 2021

siriwatknp commented Jul 19, 2021

danilo-leal commented Jul 19, 2021

eps1lon commented Jul 19, 2021

siriwatknp commented Jul 20, 2021

oliviertassinari commented Sep 29, 2021 •

edited

Loading

[system] Add styled primitives #27207

[system] Add styled primitives #27207

Conversation

siriwatknp commented Jul 10, 2021 • edited Loading

Option 1

Option 2

Option 3

mui-pr-bot commented Jul 10, 2021 • edited Loading

siriwatknp Jul 10, 2021

Choose a reason for hiding this comment

mnajdova Jul 10, 2021 • edited Loading

Choose a reason for hiding this comment

mnajdova Jul 10, 2021

Choose a reason for hiding this comment

eps1lon commented Jul 12, 2021 • edited Loading

oliviertassinari commented Jul 12, 2021 • edited Loading

eps1lon commented Jul 13, 2021

oliviertassinari commented Jul 15, 2021 • edited Loading

eps1lon commented Jul 17, 2021 • edited Loading

oliviertassinari commented Jul 19, 2021

siriwatknp commented Jul 19, 2021

danilo-leal commented Jul 19, 2021

eps1lon commented Jul 19, 2021

siriwatknp commented Jul 20, 2021

oliviertassinari commented Sep 29, 2021 • edited Loading

siriwatknp commented Jul 10, 2021 •

edited

Loading

mui-pr-bot commented Jul 10, 2021 •

edited

Loading

mnajdova Jul 10, 2021 •

edited

Loading

eps1lon commented Jul 12, 2021 •

edited

Loading

oliviertassinari commented Jul 12, 2021 •

edited

Loading

oliviertassinari commented Jul 15, 2021 •

edited

Loading

eps1lon commented Jul 17, 2021 •

edited

Loading

oliviertassinari commented Sep 29, 2021 •

edited

Loading