[core] Add grid support for Box #17326
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@material-ui/core: parsed: +0.27% , gzip: +0.17% Details of bundle changes.Comparing: e3952a5...7d16f09
|
LavaToaster
force-pushed
the
feature/box-grid
branch
from
121e9e9 to
3641dbf
Compare
|
Tests failure due to connection problem with github |
|
The changes are relatively small. I think that we can keep the discussion under a single place :) |
|
Leaving us in the slightly comical situation that Grid uses (flex)box, and Box, uses grid! 😄 |
|
I've been waiting more than a month for feedback. If you need me to do some more work on this to get this over the line, then please ask. |
|
@Lavoaster I didn't answer so far because I would like to revamp the whole implementation of the system. It's been out for almost a year now. We had people playing with it. We should be able to leverage these learnings. Regarding the change, I have one concern. What's the performance implication of the changes? Regarding the need for the grid support, I think that tailwindlabs/tailwindcss#32 upvotes are a good validation. |
|
@oliviertassinari That's a good question. I'm not sure how to measure performance on these specific changes in comparison to the last. Do you have any resources on how I may be able to do this? |
|
@Lavoaster Sure: cd packages/material-ui-benchmark
yarn system |
oliviertassinari
added
new feature
|
Hey guys, my team is looking at using CSS Grid more and would love to be able to access these inside Material-UI. how far away are we from merging this PR into the Repo? @Lavoaster @oliviertassinari Any updates on performance metrics? thanks! |
|
LGTM |
Provides the foundations for mui#15878
oliviertassinari
approved these changes
Feb 1, 2020
oliviertassinari
force-pushed
the
feature/box-grid
branch
from
3641dbf to
2af95b1
Compare
|
Thanks, we can add the documentation later on. |
|
Sorry for comment it merged PR, but we can reduce a code size to 730 bytes |
|
@popuguytheparrot What did you use for the terser options? |
EsoterikStare
pushed a commit
to EsoterikStare/material-ui
that referenced
this pull request
Feb 13, 2020
This was referenced Mar 12, 2020
This was referenced Apr 16, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds css grid support for the Box component. Resolves #15878.