Skip to content
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

Update material-ui to version 4 #1394

Merged
merged 4 commits into from
Jun 6, 2019
Merged

Update material-ui to version 4 #1394

merged 4 commits into from
Jun 6, 2019

Conversation

Lily418
Copy link
Contributor

@Lily418 Lily418 commented Jun 5, 2019

I'd like to use material-ui version 4 so I can use the new version of ThemeProvider and ServerStyleSheets with next

The type of GridSpacing has been updated
https://github.com/mui-org/material-ui/blob/89687f38cae750650555772ba4d821c9084d8dfc/packages/material-ui/src/Grid/Grid.d.ts

and ExpansionPanelSummary is now wrapped in a higher order styling component so I updated the test to find an the ancestor ExpandPanelRenderer

Lily Hoskin added 2 commits June 5, 2019 19:28
@Lily418 Lily418 changed the title [Work In Progress]: Update material-ui to version 4 Update material-ui to version 4 Jun 5, 2019
Copy link
Member

@eneufeld eneufeld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the version update!
I think the spacing is now different to the old one and I would like to not mix these two changes.
Although I personally like the bigger spacing on the horizontal layout.

@@ -79,7 +79,7 @@ export const MaterialListWithDetailRenderer =
addItem={addItem}
createDefault={handleCreateDefaultValue}
/>
<Grid container direction='row' spacing={16}>
<Grid container direction='row' spacing={10}>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand the documentation from https://material-ui.com/guides/migration-v3/#layout correctly, then the old 16 should be a 2 now?

Copy link
Contributor Author

@Lily418 Lily418 Jun 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah that makes sense, this is the relevant PR.
mui/material-ui#14099

Will update that now

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

@Lily418
Copy link
Contributor Author

Lily418 commented Jun 6, 2019

@eneufeld have updated the spacing 🛰

@@ -72,7 +72,7 @@ export const MaterialLayoutRenderer = ({
<Grid
container
direction={direction}
spacing={direction === 'row' ? 16 : 0}
spacing={direction === 'row' ? 10 : 0}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fear you missed this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very spooky 👻

Have updated!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, my German to English translation failed me ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha 😂 not at all! Apologies I was making a bad joke.

Copy link
Member

@eneufeld eneufeld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much!

@eneufeld eneufeld added this to the 2.3.0 milestone Jun 6, 2019
@eneufeld eneufeld merged commit 7d5b69a into eclipsesource:master Jun 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants