[DataGrid] Replace style-components with @material-ui/styles by dtassone · Pull Request #168 · mui/mui-x

dtassone · 2020-08-13T16:56:30Z

https://deploy-preview-168--material-ui-x.netlify.app/components/data-grid/

Drop dependency on styled-components
Closes Color Accessibility threshold #2
Closes Cells typography #3
Closes Improve CSS architecture #99
Closes [data grid][docs] Opening the data grid demos in codesandbox doesn't work #157
Closes [DataGrid] Fix dark mode support #231
Help with Discrepancy with main repository #6

oliviertassinari

Please, only use @material-ui/core/styles until we refactor the styling approach, in v5. This disqualifies CSS modules as a viable option for v4 and until we figure that out.

…tStyles.ts Co-authored-by: Olivier Tassinari <[email protected]>

oliviertassinari

A first pass at the review

oliviertassinari

A summary of the propose changes:

Flatten CSS specificity as much as possible: make overrides easier.
Use unitless when possible: won't make a large difference, only to match current coding style.
Remove blank lines: won't make a large difference, only to match the current coding style.
Use MuiDataGrid- to prefix all applied class names.
Use Mui- to prefix artificial pseudo-classes.
Hardcoded color prevent customization, e.g. dark theme.
Use theme.typography variants over cherry-picking values

oliviertassinari · 2020-09-08T16:02:17Z

I have updated the pull request's description to link all the related issues to the pull request.

…-table But I won't have time to push it further today. For tomorrow 😴

oliviertassinari

Great progress :)! I have started to continue the pull request to match the style of https://material-ui.com/components/tables/#simple-table. I will finish tomorrow.

oliviertassinari · 2020-09-10T12:13:24Z

@dtassone Could you double-check my changes? A couple of things we can still do to improve the UI:

Update the docs configuration to use @material-ui/core v4, not v5. I haven't looked where this happens with the documentation. It would fix:

I think that we will need to push the reflection further on what's the correct behavior for the focus-visible/outline.

dtassone · 2020-09-10T12:43:05Z

@dtassone Could you double-check my changes? A couple of things we can still do to improve the UI:

Update the docs configuration to use @material-ui/core v4, not v5. I haven't looked where this happens with the documentation. It would fix:

I think that we will need to push the reflection further on what's the correct behavior for the focus-visible/outline.

I don't think it's ideal to work on the same PR. It's not easy to see what has changed and understand why it has. It would have been better to create a feature branch on upstream and iterate on this one.

AFAIK we were using material ui core 4.

Not sure what happened with the number and the sort icon

oliviertassinari · 2020-09-10T12:51:19Z

It would have been better to create a feature branch on upstream and iterate on this one.

I can still do that if you would like.

It's not easy to see what has changed and understand why it has

You can see the full diff along with detailed commits at 883c789...8640691.

Not sure what happened with the number and the sort icon

I have tried to explain it in the previous comment. It's an issue with the version imported of @material-ui/core (v4 on npm vs. v4 on git). It doesn't impact the end-user (outside of the documentation). We need to use the right version. I can work on it after.

dtassone · 2020-09-10T13:03:20Z

It would have been better to create a feature branch on upstream and iterate on this one.

I can still do that if you would like.

Let's merge and create another PR for future changes

It's not easy to see what has changed and understand why it has

You can see the full diff along with detailed commits at 883c789...8640691.

I didn't know the trick! But not ideal to comment...

Not sure what happened with the number and the sort icon

I have tried to explain it in the previous comment. It's an issue with the version imported of @material-ui/core (v4 on npm vs. v4 on git). It doesn't impact the end-user (outside of the documentation). We need to use the right version. I can work on it after.

Hmm I see. I thought the component would use the npm modules, but if I understand well it uses the code in the repo, due to babel conf?

oliviertassinari · 2020-09-10T13:28:55Z

Let's merge and create another PR for future changes

Ok!

I didn't know the trick! But not ideal to comment...

True, only work for small changes, I think that after +-100 it starts to be questionable.

Hmm I see. I thought the component would use the npm modules, but if I understand well it uses the code in the repo, due to babel conf?

It has something to do with using the docs components from the mono-repo. I can't tell without investigating.

dtassone added 3 commits August 13, 2020 18:34

remove style-components

d1ae9ff

fix rollup build

41199da

fix import

691ed84

dtassone requested a review from oliviertassinari August 13, 2020 16:56

dtassone marked this pull request as draft August 13, 2020 16:56

dtassone commented Aug 13, 2020

View reviewed changes

Comment thread babel.config.js Outdated

dtassone commented Aug 13, 2020

View reviewed changes

Comment thread tsconfig.json Outdated

dtassone commented Aug 13, 2020

View reviewed changes

Comment thread packages/storybook/package.json Outdated

oliviertassinari reviewed Aug 13, 2020

View reviewed changes

oliviertassinari added the scope: data grid Changes related to the data grid. label Aug 13, 2020

oliviertassinari changed the title ~~[CSS-Modules] Remove style-components, add css modules~~ [DataGrid] Remove style-components, add css modules Aug 13, 2020

dtassone added 3 commits August 14, 2020 16:08

convert styles to jss

b44a69e

fix style

81b3444

more cleanup

2b248d1

dtassone requested a review from oliviertassinari August 14, 2020 14:48

dtassone added 2 commits August 14, 2020 16:58

prettier

072a716

lint

dacbaf0

oliviertassinari self-assigned this Aug 16, 2020

oliviertassinari mentioned this pull request Aug 29, 2020

[docs] Forward x data-grid mui/material-ui#22400

Merged

dtassone added 2 commits September 7, 2020 15:50

Merge branch 'master' into cssModules

0dcf77d

fix font styles and fix hover header error

32511cd

dtassone marked this pull request as ready for review September 7, 2020 15:18

dtassone added 2 commits September 7, 2020 17:25

remove some specificity on css

7e7a644

fix prettier

54f30f9

oliviertassinari reviewed Sep 8, 2020

View reviewed changes

Comment thread packages/grid/x-grid-modules/src/components/styled-wrappers/getStyles.ts Outdated

Comment thread packages/grid/x-grid-modules/src/components/styled-wrappers/getStyles.ts Outdated

Update packages/grid/x-grid-modules/src/components/styled-wrappers/ge…

16032d1

…tStyles.ts Co-authored-by: Olivier Tassinari <[email protected]>

dtassone requested a review from oliviertassinari September 8, 2020 09:55

oliviertassinari reviewed Sep 8, 2020

View reviewed changes

cleanup styling

dc2e18b

dtassone added 2 commits September 8, 2020 16:20

fix dependency no cycle

548ccae

prettier

3bfd638

oliviertassinari requested changes Sep 8, 2020

View reviewed changes

oliviertassinari added the type: new feature Expand the scope of the product to solve a new problem. label Sep 8, 2020

oliviertassinari changed the title ~~[DataGrid] Remove style-components, add css modules~~ [DataGrid] Replace style-components with @material-ui/styles Sep 8, 2020

oliviertassinari removed their assignment Sep 8, 2020

dtassone added 4 commits September 8, 2020 19:12

small cleanup

a924dd7

prettier

b107942

refactor styles and selector

b244586

fix test

883c789

dtassone requested a review from oliviertassinari September 9, 2020 15:10

oliviertassinari added 8 commits September 10, 2020 00:09

fix DataGrid resizing

fac1d37

replace class name that doesn't exist

2cf4421

remove all double spaces

eff34f2

generate a global class names

332a7d8

remove dead code

0c9e629

all in with MuiDataGrid for the CSS

b7abae5

polish CSS

2895038

start to reproduce https://material-ui.com/components/tables/\#simple…

d0baee1

…-table But I won't have time to push it further today. For tomorrow 😴

oliviertassinari reviewed Sep 9, 2020

View reviewed changes

oliviertassinari added 2 commits September 10, 2020 13:55

Material Design spec

d6730b4

improve footer display

8640691

oliviertassinari approved these changes Sep 10, 2020

View reviewed changes

dtassone merged commit 13b4a58 into mui:master Sep 10, 2020

Uh oh!

Uh oh!

Conversation

dtassone commented Aug 13, 2020 • edited by oliviertassinari Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oliviertassinari left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oliviertassinari left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oliviertassinari left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oliviertassinari commented Sep 8, 2020

Uh oh!

oliviertassinari left a comment

Choose a reason for hiding this comment

Uh oh!

oliviertassinari commented Sep 10, 2020

Uh oh!

dtassone commented Sep 10, 2020

Uh oh!

oliviertassinari commented Sep 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dtassone commented Sep 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oliviertassinari commented Sep 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dtassone commented Aug 13, 2020 •

edited by oliviertassinari

Loading

oliviertassinari left a comment •

edited

Loading

oliviertassinari commented Sep 10, 2020 •

edited

Loading

dtassone commented Sep 10, 2020 •

edited

Loading