mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
94.09k stars 32.33k forks source link

[RFC] Material-UI v5 πŸš€ #20012

Closed oliviertassinari closed 3 years ago

oliviertassinari commented 4 years ago

Introduction

Material-UI v4 was released 10 months ago, per our release schedule. It's time to plan for v5. Our GitHub milestone has a due date for October 1st 2020, and while I doubt we will release in time, planning 6 months ahead seems relevant.

Developers made it clear in the past that they don't enjoy breaking changes. This is feedback we took into consideration when designing our versioning strategy, and for each minor/patch releases. The result was to commit to a minimum of 1 year between each major release.

At the same time, the v0 to v1 upgrade was a major breaking change (a rewrite with a brand new API) and yet it was successful. I think that we should be willing (taking the risk) to make bold changes, as long as they fit in the direction we see the community going in the long term (with Material-UI empowering it).

Structural changes

1. Unstyled components

Similar to Angular Material CDK components (Component Development Kit) #6218.

In practice, it could be a new hook version of all the components, something we have started to experiment with the useAutocomplete and usePagination in the lab.

The problem:

Why this solution?:

React components for faster and easier web development. Build your own design system, or start with Material Design.

2. Full strict mode support

18018, #13394

The problem:

Why this solution?

3. Migrate to styled-components

6115, #16947

The problem:

Why this solution?

It could be interesting to provide a choice between different style engines. v5's users would be able to replace @material-ui/styles with react-jss.

4. Enable the use of @material-ui/system's props in the core components

15561

The problem

Why this solution?

This is mainly driven by the positive feedback Tailwind and styled-system have had recently in the community. I enjoy the pattern a lot for one-off layout problems. It's frustrating that only the Box component supports it. I think that it should also cover #6140.

<Typography textAlign={{ xs: 'left', md: 'center' }} />
<Button mt={3}>

Regarding the implementation and final API, we could still reconsider the tradeoff (relying on global class names rather than style functions).

5. Allow the use of dynamic theme variants and colors

15573 & #13875. Allow breakpoints customization #11649.


I was wondering about revamping the theme structure to match System-UI Theme Specification but we don't see a clear win so far.

Material-UI's theme structure, on its own, is a specification, the structure is set. Assuming that Material-UI aims to support a wide range of components (unstyled or not), matching this spec, not just Material Design, I fail to see a strong advantage a different constraint could yield.

On the cost side, using theme-specification would require to drop some of the features of our theme and require breaking changes. The benefit isn't obvious, but it's something to consider.

Regarding styled-system. I think that we should re-evaluate if we still need @material-ui/system.

Material Design

βœ… Checklist - [x] [Checkbox] Make color="primary" the default (#26002) - [x] [Paper] Dark mode brighening based on elevation #18309 (#25522) - [x] [Snackbar] Dark mode color #24438 - [x] [Tabs] Improve color management #24286 - [x] [Switch] Make color="primary" the default (BC: #26182) - [x] [Radio] Make color="primary" the default (BC #26180) - [x] [Tabs] Remove min-width media query #15824 (BC: #26458) - [x] [Select] Update to match the specification #18493 (BC: #26200) - [x] [Dialog] Flatten DialogTitle DOM structure #19696 (BC: #26323) - [x] ~[Button] Reduce the duration of the transition from 250ms to 200ms https://github.com/mui-org/material-ui/pull/24521#discussion_r562095317~ I don't think this is worth doing now - edited by @siriwatknp - [x] ~[Autocomplete] Consistency with select #18136~ [move to v5.1] - [x] [AppBar] Improve the design #18308 (BC: #26545) - [x] [Checkbox][Switch] Remove dependency on IconButton #21503 (BC: #26460) - [x] [docs] Use the default theme #22112, https://github.com/mui-org/material-ui/issues/21040#issue-618679269 - [x] [theme] Dark mode colors #18378 - [x] ~Material Design states #10870~ deferred @siriwatknp - [x] ~[Tabs] Update to match the specification #15324~ deferred (can be workaround by css) @siriwatknp - [x] [theme] Improve the breakpoints values #21902 - [x] [Menu] Remove MenuItem dependency on ListItem #21587 - [x] [Slider] Update to match the spec #20153 - [x] [IconButton] Update default size and add `large` #24045 - [x] ~[Button] Update to match the spec #19664~ deferred @siriwatknp

Lab to core components

I think that we can aim to move the following components from the lab to the core:

βœ… Components - [x] Alert (#22651) - [x] Autocomplete (#22715) - [x] AvatarGroup #18869 (#23121) - [x] Pagination (#22848) - [x] Rating (#22725) - [x] Skeleton (#22740) - [x] SpeedDial (finally!) (#22743) - [x] Toggle Button (#22784)

The only component I have would propose to handle later on is the TreeView. We still have a couple of important features to implement, and might need to change the API to ship these features. Once we do, we'll start to get more adoption, so the more likely it is that developers will uncover root issues.

Supported browsers

While we plan to keep IE 11 support until v6, We will look into proposing different proposing entry points #18447. and if we can drop older browsers' versions #15496.

Deprecations

βœ… List of breaking changes we can introduce with a deprecation during v4. - [x] ~Drop createStyles (see https://github.com/mui-org/material-ui/issues/20012#issuecomment-752125358)~ - [x] ~[Portal] Remove Portal `disablePortal` prop. Maybe we can implement the logic at the Portal component consumer level. https://github.com/mui-org/material-ui/pull/18692#issuecomment-562197612~ - [x] [theme] Remove `theme.mixins.gutters`. I don't believe we document them, nor are they used. (BC: #22109, Dep: #22245) - [x] [theme] Rename theme `type` -> `mode`. While the spec use "schema", saying "dark mode" seems to be more common, e.g https://css-tricks.com/dark-modes-with-css/. (BC: #22687, "Dep": #22702 – `adaptV4Theme()`) - [x] [theme] Rename color helpers https://github.com/mui-org/material-ui/issues/13039#issuecomment-476020214. (BC: #22834, Dep: #22837) - [x] [test] Remove enzyme test helpers. They are legacy. It's also a good opportunity to upgrade the documentation #17070. (BC: #21855, #21863, Dep: #24074) - [x] Remove the onX* transition props from Dialog, Snackbar, Menu & Popover, https://github.com/mui-org/material-ui/issues/17047#issuecomment-523549825. (Snackbar BC: #22107, Dep: #22206; Popover BC: #22184, Dep: #22202; Dialog BC: #22113, Dep: #22114; Menu BC: #22212, Dep: #22213) - [x] Remove RootRef, it relies on `ReactDOM.findDOMNode(component)` that is already deprecated. (BC: #21974, Dep: #24075) - [x] [icons] Change default `fontSize` prop's value: default -> medium, for consistency with the other components #14993. (BC: #23950, Dep: #23951) - [x] [Accordion] Normalize focusVisible logic (BC: #22567, Dep: #24083) - [x] [Avatar] Rename variant circle -> circular for consistency #21964. (BC: #22015, Dep: #22090) - [x] [Badge] Rename overlap circle -> circular and rectangle -> rectangular for consistency #21964. (BC: #22050, Dep: #22076) - [x] [CircularProgress] Remove one variant. Kill the current `determinate` variant, rename `static` => `determinate`. https://github.com/mui-org/material-ui/issues/7223#issuecomment-566536553. (BC: #22060, Dep: #22094) - [x] [Collapse] Add orientation and horizontal support (BC: #20619, Dep: #24079) - [x] [Dialog] Remove withMobileDialog. #14992. We have already removed it from the documentation. (BC: #23202, Dep: #23570) - [x] [Dialog][Modal] Remove disableBackdropClick (BC: #23607, Dep: #24081) - [x] [Fab] Rename Fab round -> circular for consistency #21964. (BC: #21903, Dep: #24080) - [x] [Grid] Rename prop justify -> justifyContent to match system API. (BC: #21845, Dep: #24078) - [x] [GridList] Rename component to ImageList as more intuitive and better match the wording of the Material Design spec #14994. (BC: #22311, Dep: #22363) - [x] [GridList] Rename Tile to Item (BC: #22385, Dep: #22363) - [x] [GridList] Refactor using CSS grid & React context (BC: #22395, Dep: #22363) - [x] [GridList] Rename `spacing` prop to `gap` to align with the CSS attribute. (BC: #22395, Dep: #22363) - [x] [GridList] Rename the GridList `cellHeight` prop to `rowHeight`. (BC: #22395, Dep: #22363) - [x] [GridList] Rename the GridListItemBar `titlePosition` prop to `position`. (BC: #22395, Dep: #22363) - [x] [Modal] Add support for onKeyDown and remove onEscapeKeyDown (BC: #23571, Dep: #24081) - [x] [Portal] Remove onRendered -> ref. context: https://github.com/mui-org/material-ui/pull/16262#discussion_r294750748. (BC: #22464, Dep: #24082) - [x] [TextareaAutosize] Rename rowsMax -> maxRows for consistency, (BC: #21873, Dep: #23530) - [x] [Table] Rename onChangeRowsPerPage -> onRowsPerPageChange, onChangePage -> onPageChange for consistency. It's a good opportunity to mention this API design choice [in the documentation](https://material-ui.com/guides/api/#controlled-components): https://jaketrent.com/post/naming-event-handlers-react/#more-complicated-naming; (BC: #22900, Dep: #23789) - [x] [theme] Remove the `fade` color helper #13039. (BC: #25895) - [x] [Button] Remove the `buttonRef` prop. (BC: #25896, Dep: #25897) - [x] [Popover] Remove `getContentAnchorEl` prop. This method was present to implement a macOS native select like experience. It complicates the implementation of the Popover and could prevent us from moving #17353 forward. - [x] [TextField] The `position` prop should be required in `InputAdornment`. (BC: #25891, Dep: #25912) - [x] [Button] Change LoadingButton prop pending to loading #21593 (BC: #25874) - [x] [SwitchBase] Remove second onChange argument: event.target.checked is enough. (BC: #25871) - [x] [theme] Remove the exported theme.typography.round helper. (BC: #25914, Dep: #25916) - [x] ~[theme] Remove legacy v4 deprecation `Object.defineProperty(spacing, 'unit', {`~ - [x] ?[theme] Replace `theme.direction === 'rtl'` -> `theme.isRtl`. - [x] [Table] Rename `padding="default"` -> `padding="normal"` (BC: #25924, Dep: #25990) - [x] [theme] Remove `function width(key) { return values[key]; }`, it's useless. (BC: #25918, Dep: #25993) - [x] [theme] Rename createMuiTheme -> createTheme to match ThemeProvider's theme prop. (BC: #25992, Dep: #26004) - [x] ~?[theme] Flatten colors to support CSS variables as theme option #12827~ later

For a list of committed breaking changes and associated deprecations, see: https://github.com/mui-org/material-ui/issues/22074

Breaking changes

Unlike the previous section, these are breaking changes we can't gracefully warn against.


Timing

I think that we can try the following:

  1. Deprecation preparation. (starts now)
  2. Create the next branch, publish the next versions under https://next.material-ui.com/ and the next npm tag. (April?). Stop all efforts on the v4.x version. Put the master branch (v4.x) in a low maintenance mode, only handle security fixes.
  3. Work on the above items, in whatever order is simpler, keep the same release schedule, once a week, with alpha first.
  4. Once we have handled all the breaking changes and confidence with the architectural choices move to a beta stage. Once we are confident with the implemented features, start release candidates.
Current progress 109/109 ```jsx Array.from(document.querySelectorAll('.contains-task-list')[2].children) .concat(Array.from(document.querySelectorAll('.contains-task-list')[3].children)) .concat(Array.from(document.querySelectorAll('.contains-task-list')[1].children)) .concat(Array.from(document.querySelectorAll('.contains-task-list')[0].children)) .reduce((acc, item) => { if (item.querySelector('input[type="checkbox"]:checked')) { acc.done += 1; } acc.total += 1; return acc; }, { total: 0, done: 0 }); ```
ValentinH commented 4 years ago

Really detailed plan and it looks good to me.

The only thing that scares me about is the styled component part.

v5's users would be able to replace @material-ui/styles with react-jss.

Does react-jss includes all the features of @material-ui/styles? Specifically the awesome TS support for the return type of useStyles()?

dmtrKovalenko commented 4 years ago

Unstyled components

I think that providing hooks is not a really best solution for making custom design systems. I know that some companies using material-ui like

// packages/design-system
export * from '@material-ui/core'

export const ThemeProvider = ({ ...props }) => 
  <ThemeProvider theme={ourCustomStyledMuiTheme()} /> 

// then in app code
import { Button } from '@my-project/design-system' 

And this is a really powerful way to build custom design, based on best practices and documentation of material-ui. The main problem here is only bundlesize bloating. You have no way to remove from the bundle overridden css. Unstyled components can solve this problem just fine.

But the proposed solution with hooks will may them to write custom components, custom dom structure – that’s why I don't think it will not become really popular.

Should it work something like this? This will make building design system overcomplicated and require everybody to rebuild html structure, which in its turn can break accessibility 😒

import { useButton } from '@material-ui/core'

const buttonProps = useButton()
export const Button = () => <button {...buttonProps} /> 

Proposal

I propose to provide an additional exported components but without any css. So they will be just empty html components with the logic. Thus any company will be able to handcraft custom design for provided components and use all the power of material-ui accessibility, types, documentation and so on.

export * from '@material-ui/core/unstyled'

How? For example, we can make a custom babel plugin that will, for example, remove all the css code from components and will leave only class names.

yordis commented 4 years ago

Personally, before tackling the unstyled components, I would make sure to divide it into two categories: Behavioral and UI. Or at least have clear boundaries about this in the source code, now with hooks, probably most things would become as part of a hook rather than as part of the component body.

Behavioral components expose functionality either by hooks or by elements that don't affect the UIs as much, for example, Slider, Click Away Listener, Popovers, Portals.

While UI is purely how things look.

oliviertassinari commented 4 years ago

Does react-jss includes all the features of @material-ui/styles? Specifically the awesome TS support for the return type of useStyles()?

@ValentinH I don't have the details on this question. If it doesn't, it will be a great opportunity to port these advantages back to react-jss.

I think that providing hooks is not a really best solution for making custom design systems.

@dmtrKovalenko I like the proposal. I have been wondering about going into this direction. I think that solving this problem will be very interesting, it's a complex problem with many use cases to consider and strong tradeoffs to take. In any case, I doubt that hooks can be 100% of the answer.

saulshanabrook commented 4 years ago

We are considering using Material UI in JuyterLab (https://github.com/jupyterlab/jupyterlab/issues/7574) and was wondering about this point:

It could be interesting to provide a choice between different style engines. v5's users would be able to replace @material-ui/styles with react-jss.

This sounds like an interesting and useful idea and I am curious if other libraries provide compatibility with different style engines. Ideally, it would be nice to be able to switch dynamically between these frameworks and just generating plain CSS, by just changing some config or build value and not having to rewrite all our components. πŸ€·β€β™‚

However, I haven't though very deeply about the topic. Please let me know if this has already been discussed somewhere. Also curious if other libraries have experimented with this technique.

oliviertassinari commented 4 years ago

Also curious if other libraries have experimented with this technique.

@saulshanabrook I'm not aware of any deployed prior work on this matter.

We are considering using Material UI in JuyterLab (jupyterlab/jupyterlab#7574) and was wondering about this point:

This is awesome, I'm going to following this thread to see if I can bring in any facts.

balazsorban44 commented 4 years ago

Really well-written!

I have a proposal(?) for:

[theme] Rename theme type -> mode. While the spec use "schema", saying "dark mode" seems to be more common, e.g https://css-tricks.com/dark-modes-with-css/.

I recently started implementing dark mode for our project at our company, and more than once (actually pretty much all the time) I do either of these:

  1. When accessing theme.palette.type is to check for theme.palette.type === "dark", and write it to a isDark (doing this inside makeStyles(), when I create classes for components),
  2. I use a custom hook useTheForce().isDark (πŸ€“) (which is just useTheme().palette.type === "dark")

Would it be easier to just expose theme.palette.isDark (name can be anything) as a boolean, either next to the proposed theme.palette.mode or dropping mode entirely. Is it possible to implement more than light and dark at all? If yes, I can understand the reason behind using a string, otherwise one have to write some boilerplate to access this simple true or false value.

Do tell if there is a reasoning behind it, or I could do better than the above-mentioned options.

Looking forward to Material UI v5! πŸŽ‰

oliviertassinari commented 4 years ago

@balazsorban44 Interesting, we could extend the discussion to theme.direction === "rtl" vs theme.isRtl.

balazsorban44 commented 4 years ago

I don't have a case for that (yet?), but that looks to be the same situation, yes.

Except maybe that there are only right-to-left and left-to-right. I guess theoretically it is allowed to add more than "light" and "dark" options to mode though, or is there any underlying restriction (Can MUI handle more than two modes?) Still, I would argue that accessing dark mode is so common that it should be exposed as a boolean.

Are there any reasons not to include both (the proposed mode as a string and an isDarkMode boolean) variants?

Also, is it possible to expose an accompanying function (eg.: changeMode) to be able to quickly change between modes (and RTL)? Right now, one has to use createMuiTheme in some kind of a provider above ThemeProvider that has a simple mode state (which is used to toggle the passed theme to ThemeProvider) and a setMode setter, that can be used to switch between modes from anywhere in the app). If a setter would be exposed on useTheme(), an extra context could be spared here, as this only "syncs" the mode's state in ThemeProvider.

oliviertassinari commented 4 years ago

@balazsorban44 thank you for bringing your perspective on the matter. It's also interesting to look at the strategy we had in v0: https://v0.material-ui.com/#/customization/themes. Considering the idea that we want to provide multiple themes (Material Design, Gmail, Firebase, Material Studies, Fluent, Ant Design, Chakra, Bootstrap, High contrast, etc), we could consider handle the dark mode as a standalone theme πŸ€”.

is it possible to expose an accompanying function (eg.: changeMode) to be able to quickly change between modes (and RTL)?

See the discussion in #20495.

popuguytheparrot commented 4 years ago

I'm Concerned for DX in Developer Tools. I would like to avoid such nesting without to the use of filters. For example, if this possible to change hoc withStyles to hook, and forward ref to hook. Fix Anonymous names.

image

eps1lon commented 4 years ago

I'm Concerned for DX in Developer Tools.

@popuguytheparrot You can hide higher-order components in the devtools. hide-hoc

popuguytheparrot commented 4 years ago

I wrote about it. The solution through the filter does not solve the problem. it is important to refuse in favor of hooks

nahtnam commented 4 years ago

With the move to styled components, is react-native support something that can be done? Mobile apps would be significantly easier to make

coder2000 commented 4 years ago

This may seem out of the blue but is there any plan for upgrading to Popper 2.0 for 5.0? or is it in the works for a version in the 4.0 release stream?

ryancogswell commented 4 years ago

This may seem out of the blue but is there any plan for upgrading to Popper 2.0 for 5.0? or is it in the works for a version in the 4.0 release stream?

@coder2000 At the end of the Breaking Changes section:

[Popper] Figure out what we do with popper.js, we have 4 options upgrade, remove it, rewrite it, fork it. #19358

oliviertassinari commented 4 years ago

The concerns with Popper to take into account:

  1. IE 11 support
  2. Minimizing external dependencies, internalizing the solution of hard UI problems (Material-UI doesn't aim to be a "skin" on top of react-select, react-table, react-dropzone, etc. the opposite).
  3. Including more features that we absolutely need, reduce bundle size
coder2000 commented 4 years ago

Sorry, I did read it but must have missed that last bit. I can look into helping with options, I have a bit of OCD when it comes to my code in that I don't like warnings so Node is painful for me as there are always warnings of some deprecation 5 levels deep in a dependency.

oliviertassinari commented 4 years ago

@coder2000 Upgrade to the latest stable version, we solved the warning

jedwards1211 commented 4 years ago

@oliviertassinari have y'all figured out yet what aspects of the theme and style overrides APIs will get broken by the switch to styled-components?

oliviertassinari commented 4 years ago

will get broken by the switch to styled-components?

@jedwards1211 No, I didn't spend much time on it since the beginning of the year. To be specific to your question, I don't think that we should weight much the BC part of theming in v5. The objective is to find a new and better local maximum. If it requires changing many parts to get something much better, why not? We might even only start our enterprise offering from this new infra.

Right now, I think that the priority should be to make the deprecation warnings on v4, release them, so we can actually move to v5 development.

Looking at how things are going, I might not be able to get involved in the coding tasks that require long strength of focused time much anymore, this could impact this task. It has become increasingly difficult, as we are ramping up $ funding and new hiring. I think that one plan that could be great is to reproduce what we did for the DataGrid and DatePicker: find somebody fully dedicated to this theming/styling/unstyled problem. We might not be too far away from such a solution. The main challenge is to find the right person, it's a tough problem with high risk and high return profile type of task. There are many constraints to take into account. It seems that any solution needs to take them all into account, finding a balance.

The engine itself

I could find 2 downsides of a styled-components only approach: 1. the hashed class names that it generates, it makes it harder for the developers that want to override the components using the Dev Tools (it's personally my go-to approach any time I need to customize the component). The use cases in impacted ranges from theme.overrides to CSS modules & SASS users. 2. The generates styles can't be edited in production, this makes it harder to quickly try different values. 2. is OK-ish compared to what we gain, 1. is more concerning. I'm very keen on allowing people to choose between different styles engines. Say, styled-components, JSS, SASS.

Theming

Looking at the direction we are taking, we would like, in the coming years (meaning we will try a couple and see if it find a market before testing more) to support the following themes by default:

  1. unstyled (no CSS) #6218
  2. bear (foundations to build a brand new design, e.g. Reach-UI default styles)
  3. iOS theme for parity with Ionic
  4. A theme for China (similar to element, antd, rsuite)
  5. A marketing/landing page theme (find someone similar to bootstrap, tailwind, chakra, sancho).
  6. Android + dashboard: Material Design (as we already do, keep pushing updates to match the spec)

CSS utils (#15561)

Dynamic variants (#15573)

eps1lon commented 4 years ago

new breaking change (should not break TypeScript projects, unlikely that it breaks untyped projects since it only adds a runtime warning): Make position required in InputAdornment at runtime. Until now only TypeScript marked this prop as required. It was optional at runtime.

vizardkill commented 4 years ago

This project is extremely phenomenal, I love it, I work in a software testing company in Colombia, and the whole area loves this project, we are around 600 people.

Could we also see the notistack (iamhosseindhv) package included, in the future? and on the packages of the laboratory they will include them within the nucleus?

b-zurg commented 4 years ago

@oliviertassinari with regards to the classnames, I believe if you specify a classname on a component then that is retained and combined with the styled-component autogenerated classname.

i.e. styled-components classnames will be what they are but the core components can each have a static classname that is used. Wouldn't that solve the problem?

oliviertassinari commented 4 years ago

@b-zurg It's not ideal. When you open your dev tool and inspect the applied CSS, you won't be able to match which style (from styled-components) matches which class name (from the static ones). I'm personally relying a lot on this property to work with customizations, the best way I could find so far. I think that we need, at a minimum, to be able to identify the name of the component with its styled. Maybe source map could solve this problem.

ItsWendell commented 4 years ago

Is the sx prop for system style properties going to stick, and when will this be applied to all core components?

oliviertassinari commented 4 years ago

Is the sx prop for system style properties going to stick

@ItsWendell Yes.

When will this be applied to all core components?

@mnajdova has just completed the support for the first core component of sx: the Slider in #23308. The next steps are:

ItsWendell commented 4 years ago

What is the reason for the movement of system props to the SX property, instead of actually keeping them in the props? Is it to prevent prop collision? Performance? Etc. I kind of liked them being actual properties in your components, wonder why this decision was made, couldn't find anything about it.

In one of my projects, we've already migrated to V5.0 alpha and getting warnings now for the Box component, actually considering composing our own Box component with these props but would love to hear more about the reasoning behind the XS property.

mnajdova commented 4 years ago

What is the reason for the movement of system props to the SX property, instead of actually keeping them in the props? Is it to prevent prop collision? Performance? Etc. I kind of liked them being actual properties in your components, wonder why this decision was made, couldn't find anything about it.

There are several reasons for this. The first one is props collision - as we want to support the system on all core components, there will likely be collisions, for properties like color for example. The second one is, we are implementing the sx as a superset of CSS, which means that it can support pseudo and nested selectors, so we cannot possibly generate all props for this. The third one, collecting then under one prop, helps with structuring which props are effecting strictly the styling, vs the component's own props.

In one of my projects, we've already migrated to V5.0 alpha and getting warnings now for the Box component, actually considering composing our own Box component with these props but would love to hear more about the reasoning behind the XS property.

Nothing wrong with it if you really want to support them, but you won't be able to benefit the pseudo-selectors, like ":hover" or any other nested selectors. Realy depends on your use-case. I would recommend doing that only if you want the component to whitelist only specific props, if that's not the intention, you are probably locking yourself out of some functionality. We also have a codemod for this, hopefully, it can help migrate easier.

jedwards1211 commented 3 years ago

@oliviertassinari you mentioned wanting to refactor the Popover to be able to work like a non modal here, should we add that to the list here? Should we make an issue for it other than #17353? I'm willing to help discuss that and help implement the changes

oliviertassinari commented 3 years ago

@jedwards1211 I believe it's already part of the list in https://github.com/mui-org/material-ui/milestone/35.

eps1lon commented 3 years ago

Considering v5 will be ready after January we'll probably drop support for TypeScript 3.3. Right now this only means we'll drop createStyles. The change will be codemod-able and introduce as const assertions.

TypeScript version support in this case follows that of DefinitelyTyped (https://github.com/DefinitelyTyped/DefinitelyTyped#older-versions-of-typescript-31-and-earlier) i.e. every @types/* package. We don't plan on increasing that requirement above that of DefinitelyTyped.

vdjurdjevic commented 3 years ago

@eps1lon What do you mean by "will be ready after January"? Stable release? I am about to start a new greenfield project in February, so I think I might use v5 with emotion and give sx prop approach a try.

mbrookes commented 3 years ago

Stable release?

Definitely after January. (How long after January is another question altogether! πŸ˜€)

vdjurdjevic commented 3 years ago

Hehe :) Is there at the least a close answer to that question? :D What would you do, stick with v4 or go with v5 for new project?

o-alexandrov commented 3 years ago

I believe the question here is:

All other API changes are going to be easy-to-adjust for us, users-developers.

P.S. you could safely stay on the following versions as those are the versions that are still pretty much v4 with some fixes, but without emotion:

    "@material-ui/utils": "5.0.0-alpha.8",
    "@material-ui/styles": "5.0.0-alpha.11",
    "@material-ui/system": "5.0.0-alpha.11",
vdjurdjevic commented 3 years ago

I can tolerate both emotion and jss bundled for a while. I am using emotion with v4 anyway. Hopefully, v5 will be stable before I go to production :)

mbrookes commented 3 years ago

Trying to predict that will be a bit of a lose-lose. Too optimistic, and we disappoint those who commit to v5 early, and are impacted, when it slips; too pessimistic, and those who really want v5, but opt for v4 for stability in production are disappointed with having to migrate if v5 arrives sooner.

All I suggest is, if you want v5 to land sooner, get stuck in and help.

jstiers commented 3 years ago

Is there any possibility of adding Tailwind-CSS props to the System-Props or is this technically impossible?

oliviertassinari commented 3 years ago

@jstiers Could you be more specific? So far in the plan for v5-stable, all the core components come with the sx prop, detailed in https://next.material-ui.com/system/basics/, e.g. <Button sx={{ mt: 1 }}>. The CSS utility components: Grid, Typography, Box, and Stack have the subset of the system available as flattened props, e.g. <Typography mt={1}>.

Jack-Works commented 3 years ago

I have a new concern with the new styling solution.

Background:

  1. Our react app is multiple roots. It has separate emotion&JSS instance for each root to avoid collisions. Further, each instance has its own random prefix for MUI styles.
  2. Today we're using makeStyles to create CSS

Today we can use classes = useStyles() to get class names then pass it into e,g, <Slider classes={{marked: classes.marked}}>. This is good because it is strongly typed.

We don't like to have both JSS and emotion in our product so we will follow the @material-ui to switch to the emotion-based solution gradually. But it seems like we cannot keep this deep element modifying strongly typed.

The most simple example:

const CustomizedSlider = styled(Slider)`
  color: #20b2aa;
  & .MuiSlider-thumbs { border-radius: 1px; }
`

In the above code, I misspelled .MuiSlider-thumb to .MuiSlider-thumbs but I won't get a type error.

This problem becomes more serious in our application, as I said before in our application we have multiple roots, and to avoid the collision, each root has its own random prefix. So global name of the Material UI components will not work in our application because it's become something like .emotion-instance-random-id-qwerty-MuiSlider-thumb.

It seems to be possible to style the deeper elements in the new framework this in way:

const CustomizedSlider = styled(Slider)`
  color: #20b2aa;
  & .marked1 { border-radius: 1px; }
`
<CustomizedSlider classes={{marked: 'marked1'}} />

But I think this way is very not elegant and error-prone.

I'd like to get suggestions from the Material UI team about how can we strongly type all classes in our use case with emotion-based styled API?

oliviertassinari commented 3 years ago

I'd like to get suggestions from the Material UI team about how can we strongly type all classes

@Jack-Works You have two options to have stronger types.

  1. You import the xxxClasses object and use its keys. For instance with the unstyled slider (same pattern everywhere):
import SliderUnstyled, { sliderUnstyledClasses } from "@material-ui/unstyled/SliderUnstyled";

const StyledSlider = styled(SliderUnstyled)`
  color: black;

  & .${sliderUnstyledClasses.rail} {

https://codesandbox.io/s/unstyledslider-material-demo-forked-pt3wk?file=/demo.tsx:112-529

  1. You apply the same strategy as with JSS. The difference between v4 and v5 is that you now inject components with customized style in each slot, instead of class names with customized style.

Instead of doing:

<CustomizedSlider classes={{ marked: classes.marked }} />

you can do:

<CustomizedSlider components={{ Marked: StyledMarked }} />
Jack-Works commented 3 years ago

@oliviertassinari Hi your suggestion works well in most situations, thanks! But there're still some cases not covered.

I created the styled version of Drawer component, and I want to change the style of it's inner component Paper. In v4 solution I can do <MyDrawer PaperProps={{ classes: { root: classes.paper } }} />.

There are no components on the Drawer component, no PaperComponent either, so method 2 doesn't work in this case.

There is no drawerClasses.paper so I use paperClasses.root and write this code.

const NavigationDrawer = styled(Drawer)(({ theme }) => ({
    top: `${theme.mixins.toolbar.minHeight}px !important`,
    [`& .${paperClasses.root}`]: {
        width: 232,
        top: theme.mixins.toolbar.minHeight,
        paddingTop: theme.spacing(7.5),
        background: new Color(theme.palette.background.paper).alpha(0.8).toString(),
        backdropFilter: 'blur(4px)',
    },
}))

But it will affect all children instead of the drawer one. I changed it to the following selector but it is still risky.

[`& > .${paperClasses.root}`]: {

It depends on the underneath knowledge of the library (that paper is a direct child of drawer which is not a contract of the API and might change in the future).

Method 2 you mentioned is very helpful but I found not every component can be customized like that. Is that an on-going feature or it only appears on the unstyled components?

<CustomizedSlider components={{ Marked: StyledMarked }} />
Jack-Works commented 3 years ago

image

Another question is that, as you can see, our JSS and emotion instances are randomized (cause we have multiple instances). Is it true for the following list (otherwise it is not safe to use things like listClasses.root)?

As you can see in my screenshot, DialogTitle does not have an unprefixed version so dialogTitleClasses.root will not work for us.

Jack-Works commented 3 years ago

Well, I think based on what API @emotion exposed, it is possible to build an emotion version of makeStyles. I will investigate this and I think upgrading should not let people choose between running 2 CSS-in-JS engines or rewrite all CSS to migrate. It would be better to have an official version of this.

oliviertassinari commented 3 years ago

Method 2 you mentioned is very helpful but I found not every component can be customized like that. Is that an on-going feature or it only appears on the unstyled components?

@Jack-Works We want to introduce the components prop independently from the unstyled components. For instance, it's required to customize the Tooltip that portals it's title.

As you can see in my screenshot, DialogTitle does not have an unprefixed version so dialogTitleClasses.root will not work for us.

DialogTitle was migrated in #24623 but isn't released yet. It will be solved once we cut v5.0.0-alpha.25.

Well, I think based on what API emotion exposed, it is possible to build an emotion version of makeStyles

This has potential, let us know :)

Jack-Works commented 3 years ago

Cool thanks, does this mean in future all components will have that's components props and we can modify anything just like the classes props today?

I will try to make a POC of emotion based version of make styles when I have free time. Hope to see that supported as an official way so we don't have to migrate every CSS we wrote and only have one CSS in JS engine in the runtime.

dimitropoulos commented 3 years ago

Just wanted to chime in and say that I wrote a new (small) app today with the v5.0.0-alpha.24 and give some feedback (almost all positive).

1. Stylishly system

The number one thing I was worried about was losing the interop between the css-classes and the property names you use to style with JSS. In my opinion, this feature of material-ui is the single best thing about the library. To be clear, what I'm talking about is the part where, in the chrome dev tools, I can see styles for a class like "MuiOutlinedInput-root" and I immediately know where to look to style it. I personally love JSS and cannot stand having template strings with CSS syntax in any of my projects (in other words, I don't have a high opinion of the approach of styled-components, although I think it's a great project for lots of other reasons).

2. changes to createMuiTheme

The second thing I noticed was some changes to the organization of createMuiTheme. I had no real objection to the prior way of doing things, but TypeScript intellisense was more than enough of a guide to help me figure out what had changed without ever having to consult the docs. Now that I see this new system (where the style overrides and default props are grouped together per component) it makes a lot of sense and I really like it!

here's what my `theme.ts` looks like, for example ```ts import { createMuiTheme } from '@material-ui/core'; import { purple } from '@material-ui/core/colors'; export const theme = createMuiTheme({ components: { MuiContainer: { defaultProps: { maxWidth: 'sm', }, }, MuiOutlinedInput: { styleOverrides: { root: { backgroundColor: 'white', }, }, }, MuiTableCell: { defaultProps: { align: 'center', }, }, MuiTextField: { defaultProps: { type: 'number', }, styleOverrides: { root: { maxWidth: 100, }, }, }, }, palette: { primary: { main: purple[500], }, }, }); ```

3. sx props

I have never used the sx props before today, but they were super easy to understand for the most part. I'd been a big fan of using withStyles for literally all one-off styling in the past and I won't miss doing that if I have the sx props.

There was one thing, though, that really surprised me:

<Typography sx={{ marginBottom: 2, marginTop: 4 }} variant="h5">text</Typography>

Results in a margin bottom of 16px and a margin top of 32px, whereas I was expecting it to be 2px and 4px respectively. I did read https://next.material-ui.com/system/basics/#the-sx-prop and didn't see anything about this, in particular. It makes sense to me now, and I think it's cool, but it could be a bit confusing to the uninitiated.

oliviertassinari commented 3 years ago
  1. Stylishly system

@dimitropoulos If this aspect wasn't there, we would have probably tried to migrate sooner. We have been documenting this aspect in #22342 under "Deterministic classnames on the components that can be targeted for custom styles". Hopefully, it will be worth the cost. So far, it seems to be largely the case :).

mbrookes commented 3 years ago

I was expecting it to be 2px and 4px respectively

For that you can provide a string value: sx={{ mb: '2px', mt: '4px' }}.