[Layout] Add a new component #5808

oliviertassinari · 2016-12-20T00:07:47Z

PR has tests / docs demo, and is linted.
Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).
Document the properties
Write some unit tests
Add some regressions tests

This is a port of the layout component we use at @doctolib.
We can see that effort as an alternative to #5267 that might or not succeed.
I have made some important API / implementation tradeoffs.

We do not rely on the matchMedia API to be fully server side rendering compliant. Each xs, sm, md and lg variations increase the size of the style injected in the DOM.
It's using a 12 grids system as suggested in the specification. Instead of a more granular flex property.
It's exposing a single component that can be an item or a container. There is no <Col /> nor <Row /> components.

Closes #3614

rosskevin · 2016-12-21T17:17:30Z

src/Layout/LayoutBlock.js

+import classNames from 'classnames';
+
+const GUTTERS = [8, 16, 24, 40];
+const GIRD_SIZES = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12];


rosskevin · 2016-12-21T17:31:57Z

Looks good. Without using it my only concern is reducing verbosity where possible.

For example:

<LayoutBlock type="item" xsGrid={6} smGrid={3}>

I think this would be better written as:

<Layout xs={6} sm={3}>

Details:

Block adds no value as a description to the component - Layout is sufficient
*Grid The primary used props are for the responsive grid sizes - all labeled *Grid. Whereas I totally understand that there are other grid based props e.g. xsDirection, I feel that this one could really be just xs, sm, md and leaving xsDirection etc as-is
type could be defaulted as item as it will be the most common use

I think these would be good changes and will not reduce clarity given the frequency of use.

oliviertassinari · 2016-12-21T20:03:42Z

@rosskevin Thanks for the feedback!

type could be defaulted as item as it will be the most common use

I completely agree.

*Grid The primary used props are for the responsive grid sizes - all labeled *Grid.

That's a good idea.

Block adds no value as a description to the component - Layout is sufficient

You raised a good point. I wasn't happy with that LayoutBlock name.
I have some other ideas:

LayoutGrid
Grid

Bootstrap just dropped the float implementation of the layout. I'm gonna see there if I can't reuse some good part: http://v4-alpha.getbootstrap.com/layout/flexbox-grid/

oliviertassinari · 2016-12-21T21:55:48Z

@rosskevin I have updated the PR with your feedbacks.
That Layout component can be improved in many ways. For instance, by:

Writing more tests
Writing more documentation, bootstrap have much more
Adding more flex properties like xsOrder or md variations.

I have tried to keep things simple. We should be able to continue the effort in a second run once we feel the needs for it.

stunaz · 2016-12-23T00:35:58Z

Hi @oliviertassinari, really nice start, here are a few questions/comment :

Why there is a type of Layout (item or container) - I think it should not exist a type, a block may be able to content block without having end user specifying the type
I dont understand the role/need of the xs attribute as a boolean value i.e in the Auto-Layout example, does this mean that they will be an sm, md, lg boolean attributes too ? AngularMaterial just use the flex attributes for that.
The docs should indicate the breakpoints that you are using for xs, sm=600px? , md=960px?, lg=1200 ? ...
I see props like xsGutter, xsDirection, xsJustify, xsAlign, I hope to also see smGutter, mdGutter, smDirection ... to have different comportement applied based on the breakpoint.
Apppart for properties like xsOrder or md variations as you mentioned, it will be nice to see others like xsVisible, smVisible ... or xsHide, smHide ....

PhilipGarnero · 2016-12-23T11:49:37Z

Good work @oliviertassinari !

I have one small question : why is the layout component not split into two distinct components ? Is there any practical reason the container and the item version should live together ? I didn't notice props that were used by the two components in your examples.
It is just a matter of tastes, but rather than a type attribute, I'd prefer two distinct components. Unless of course, you have a technical reason for them to be under the same component.

Anyways, thank you for your work 👍

oliviertassinari · 2016-12-23T16:51:11Z

Why there is a type of Layout (item or container)

why is the layout component not split into two distinct components?

That's ambivalence is at the heart of the CSS’s flexbox model. Just reading the first line of the specification
You gonna find:

In the flex layout model, the children of a flex container.

So why not having two separate components?
Because you might want to have a <Layout /> behaving as a container and an item at the same time.
That was my original implementation but ultimately, I reverted as I had gutter issues and thought it would be better to be closer to the bootstrap model, i.e. simpler.

I dont understand the role/need of the xs attribute as a boolean value

That's basically adding flex-grow: 1. So you can get shared screen width without having to think about the right xs value (12 / number_of_items).

The docs should indicate the breakpoints that you are using

I have updated the documentation

I hope to also see smGutter, mdGutter, smDirection ... to have different compartment applied based on the breakpoint.

Yes, that xs prefix was added to anticipate that need. I don't want to implement it for now. But the API / implementation is allowing us to grow into that direction.

Apppart for properties like xsOrder or md variations as you mentioned, it will be nice to see others like xsVisible, smVisible ... or xsHide, smHide ....

I agree, as the previous answer. I want to keep thing simple to begin with.

stunaz · 2016-12-23T17:02:25Z

That's basically adding flex-grow: 1. So you can get shared screen width without having to think about the right xs value (12 / x).

I now get it, xs applied to all screens size, I first thought it was a breakpoint, for screens smaller than sm

nathanmarks · 2016-12-25T00:06:45Z

src/Layout/Layout.spec.js

+    });
+  });
+
+  describe('prop: component', () => {


prop: xs?

nathanmarks · 2016-12-25T00:07:04Z

src/Layout/Layout.spec.js

+  });
+
+  describe('prop: component', () => {
+    it('should change the component', () => {


We should also test spreading props properly to the component.

nathanmarks · 2016-12-25T00:17:08Z

@oliviertassinari What are your thoughts RE the amount of styles that would be pre-created if sm, md, lg variants were added for various options? Non-issue? I think users will ask for this. Or at least one more set.

Overall looking really awesome.

oliviertassinari · 2016-12-25T10:18:56Z

hat are your thoughts RE the amount of styles that would be pre-created

@nathanmarks Thanks for the review.
We have 5 properties missing the variations. That's gonna make 25 more CSS properties injected in the DOM. I don't feel like it's much, but that just an intuition, I would need to benchmark that.

Regarding other set of properties that people might want to add, I see some candidates:

offset
padding
margin
order
hide: I would keep that into a different component
show: I would keep that into a different component

Or politic could be to let users implement the style in their side until we have a way to lazy inject the style or some benchmark showing us it's fine.

oliviertassinari · 2016-12-25T21:23:23Z

Let's move forward. I'm gonna merge that PR. I'm quite happy with the API / implementation.
We will see how the community respond when trying it out.

oliviertassinari · 2016-12-25T21:23:51Z

Thanks everybody for your feedbacks!

gaguirre · 2016-12-29T18:38:11Z

I tried with npm install material-ui@next but Layout is not there.
From where should I get the next version? @oliviertassinari

mbrookes · 2016-12-29T21:55:56Z

@gaguirre It isn't published yet, but you can find the code here: https://github.com/callemall/material-ui/tree/next

gaguirre · 2016-12-29T22:04:45Z

Thanks @mbrookes, do you have plans to publish a new version soon?

oliviertassinari · 2016-12-29T22:08:41Z

@nathanmarks might be working on automating releases of beta version of the next branch on a fix period basis.

gaguirre · 2016-12-30T01:34:27Z

Great! Thanks @oliviertassinari @mbrookes

Extra-lightwill · 2017-01-05T21:42:55Z

Hi @oliviertassinari I've been testing out this layout component and it’s really great - however, i’m just wondering how you would reference the xsAlign, xsDirection, xsJustify and xsWrap props (I can’t seem to figure it out and for me, it's not clear from your commits in the PR.) Including it like below, throws an error:

 <Layout type="item" 
          xs={6}
          xsJustify="center"
          xsAlign="center"
          >

ReferenceError: xsJustify is not defined

I've been looking for a comprehensive component based layout solution with React/flexbox and stumbled upon this - although it's ofc quite new, it seems to really fit the bill and would love to use it. Thanks.

oliviertassinari · 2017-01-06T12:13:46Z

@Extra-lightwill Please open an issue. Have a look at that docs regarding how to use it https://material-ui-1dab0.firebaseapp.com/#/layout/responsive-ui.

RohovDmytro · 2017-01-12T21:20:33Z

How about treating xs={true} as xsVisible={true}? And so on.

oliviertassinari · 2017-01-12T21:27:03Z

@rogovdm I'm wondering if xsVisible should be handled at the Layout level. That sounds like a generic helper. I think that we could provide it with a Higer-order Component or a wrapper component specialized in that.

RohovDmytro · 2017-01-12T21:32:10Z

@oliviertassinari maybe, but I'm not sure. I use react-grid-system components and it become quite annoying to wrap thing with additional Hidden component all over the place. My pros is that grid building is tightly coupled with hiding stuff based on xs, sm, lg etc.

Will there be much overhead if Layout will be used is some cases only for hiding or making visible?

oliviertassinari · 2017-01-12T22:03:36Z

@rogovdm I get your tradeoff. I think we should wait having more feedbacks.

jaredkwright · 2017-02-08T05:53:47Z

I am starting a greenfields project using the next branch and I am currently looking for a good way to hide/render components based on screen size. For example, I might want to use a simplified logo on mobile but the full logo on desktop. Does Layout currently support a way of doing this, or am I thinking about the problem wrong?

oliviertassinari · 2017-02-08T19:57:08Z

Does Layout currently support a way of doing this

@jaredkwright No, he doesn't. I think that we could be providing a component / Higher-order Component for that use case.

mikeatm · 2017-02-24T23:37:16Z

@oliviertassinari looks quite neat, is there a plan to backport it to <1.0.0 release? I tried the next branch, but its still missing a bunch of components (or better yet is there an ETA on 1.0.0 with layout and all components?)

oliviertassinari · 2017-02-25T11:09:12Z

looks quite neat, is there a plan to backport it to

@mikeatm Thanks. No, we don't have the resources to maintained two versions same, we don't have the resource to give a ETA.

advance512 · 2017-03-16T16:31:50Z

I also attempted using it with the existing release - no dice, there is no styleManager in the existing release's MuiThemeProvider, but it is required to generate styles in the Layout class.

Using two MuiThemeProviders from the existing release and next release, together, seems a bit unhealthy, not to mention it's quite hacky getting both existing and next working together.

Is there any recommendation for a Layout that can be used /today/ with Material UI to allow automatic layouts, similar to the Material.io layouts?

mikeatm · 2017-03-16T16:37:15Z

ran into this today, https://jsxmachina.github.io/react-grid-system/ , no idea if works, yet to test it with material-ui

damianobarbati · 2017-03-31T14:06:49Z

Yet another essential/cool thing... just in the next branch :(
Is it possible to use this in current material-ui version?

Great work as always guys!

oliviertassinari · 2017-03-31T14:08:13Z

@damianobarbati It's published under the 1.0.0-alpha.x version. You can play with it, today, (docs).

damianobarbati · 2017-03-31T14:38:00Z

@oliviertassinari thanks so much for the quick answer, trying right now! Is there a documentation referring to the branch? I see lots has changed in components importing structure (icon, button) and theme (getMuiTheme not defined) and so on.
Thanks!

oliviertassinari · 2017-03-31T15:17:16Z

I have linked the documentation in the previous comment. It's still work in progress.

oliviertassinari added the next label Dec 20, 2016

muibot added the PR: Needs Review label Dec 20, 2016

oliviertassinari changed the title ~~[WIP][LayoutBloc] Add a new component~~ [WIP][LayoutBlock] Add a new component Dec 20, 2016

muibot added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI PR: Needs Review and removed PR: Needs Review labels Dec 20, 2016

oliviertassinari mentioned this pull request Dec 21, 2016

[Layout]: Add new Layout components #5267

Closed

3 tasks

oliviertassinari changed the title ~~[WIP][LayoutBlock] Add a new component~~ [LayoutBlock] Add a new component Dec 21, 2016

oliviertassinari removed the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Dec 21, 2016

oliviertassinari mentioned this pull request Dec 21, 2016

Responsive Material-UI #3614

Closed

rosskevin reviewed Dec 21, 2016

View reviewed changes

oliviertassinari changed the title ~~[LayoutBlock] Add a new component~~ [Layout] Add a new component Dec 21, 2016

muibot added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI PR: Needs Review and removed PR: Needs Review labels Dec 21, 2016

oliviertassinari removed the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Dec 21, 2016

oliviertassinari assigned nathanmarks Dec 21, 2016

nathanmarks reviewed Dec 25, 2016

View reviewed changes

src/Layout/Layout.spec.js

});

});

describe('prop: component', () => {

Copy link

Member

nathanmarks Dec 25, 2016

Choose a reason for hiding this comment

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

prop: xs?

oliviertassinari reacted with thumbs up emoji

nathanmarks reviewed Dec 25, 2016

View reviewed changes

[Layout] Add a new component

919d1f0

oliviertassinari merged commit 692e74c into mui:next Dec 25, 2016

oliviertassinari deleted the layout-block branch December 25, 2016 21:23

oliviertassinari mentioned this pull request Jan 1, 2017

[Docs] Add responsive grid recommendations #3066

Closed

rosskevin mentioned this pull request Apr 27, 2017

[Layout] responsive-based visibility of items #6726

Closed

zannager added the docs Improvements or additions to the documentation label Mar 15, 2023

[Layout] Add a new component #5808

[Layout] Add a new component #5808

Conversation

oliviertassinari commented Dec 20, 2016 • edited

rosskevin Dec 21, 2016

Choose a reason for hiding this comment

rosskevin commented Dec 21, 2016

oliviertassinari commented Dec 21, 2016

oliviertassinari commented Dec 21, 2016 • edited

stunaz commented Dec 23, 2016

PhilipGarnero commented Dec 23, 2016 • edited

oliviertassinari commented Dec 23, 2016 • edited

stunaz commented Dec 23, 2016

nathanmarks Dec 25, 2016

Choose a reason for hiding this comment

nathanmarks Dec 25, 2016

Choose a reason for hiding this comment

nathanmarks commented Dec 25, 2016 • edited

oliviertassinari commented Dec 25, 2016 • edited

oliviertassinari commented Dec 25, 2016 • edited

oliviertassinari commented Dec 25, 2016

gaguirre commented Dec 29, 2016

mbrookes commented Dec 29, 2016 • edited

gaguirre commented Dec 29, 2016

oliviertassinari commented Dec 29, 2016 • edited

gaguirre commented Dec 30, 2016

Extra-lightwill commented Jan 5, 2017 • edited by oliviertassinari

oliviertassinari commented Jan 6, 2017

RohovDmytro commented Jan 12, 2017 • edited

oliviertassinari commented Jan 12, 2017

RohovDmytro commented Jan 12, 2017 • edited

oliviertassinari commented Jan 12, 2017

jaredkwright commented Feb 8, 2017

oliviertassinari commented Feb 8, 2017

mikeatm commented Feb 24, 2017

oliviertassinari commented Feb 25, 2017

advance512 commented Mar 16, 2017

mikeatm commented Mar 16, 2017

damianobarbati commented Mar 31, 2017

oliviertassinari commented Mar 31, 2017 • edited

damianobarbati commented Mar 31, 2017

oliviertassinari commented Mar 31, 2017

oliviertassinari commented Dec 20, 2016 •

edited

oliviertassinari commented Dec 21, 2016 •

edited

PhilipGarnero commented Dec 23, 2016 •

edited

oliviertassinari commented Dec 23, 2016 •

edited

nathanmarks commented Dec 25, 2016 •

edited

oliviertassinari commented Dec 25, 2016 •

edited

oliviertassinari commented Dec 25, 2016 •

edited

mbrookes commented Dec 29, 2016 •

edited

oliviertassinari commented Dec 29, 2016 •

edited

Extra-lightwill commented Jan 5, 2017 •

edited by oliviertassinari

RohovDmytro commented Jan 12, 2017 •

edited

RohovDmytro commented Jan 12, 2017 •

edited

oliviertassinari commented Mar 31, 2017 •

edited