[Layout]: Add new `Layout` components #5267

msuntharesan · 2016-09-27T20:16:06Z

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).

First of all Thank you for the awesome library. I love how every thing just works. One thing that is missing is a layout component. I've been custom rolling the awesome angular material with react. As I am following the changes to the next branch, I see the potential to contribute my little Layout component. This is only a very basic layout component. Appreciate all the feedback. Thank you.

This is an initial take on layout feature heavily influenced by angular material. There are 2 components exposed here.

<Block /> Component. Works very similar to the layout attributes in angular material
withLayout HOC. which wraps any component with layout properties which gets rendered as className property.

Also included is the option to set layout properties for media query breakpoints.

Example use of <Block /> component

import { List, ListItem, Toolbar, ToolbarGroup } from 'material-ui';
import { Block } from 'material-ui/Layout';
import { Link } from 'react-router';
import SvgIconHome from 'material-ui/svg-icons/action/home';

const ToolbarBlock = withLayout(Toolbar);
const BlockLink = withLayout(Link);

export default (props) => (
  <Block flex layout="column">
    <Block flex="none">
      <ToolbarBlock layout="row">
        <ToolbarGroup>
          <Block layout="row" align="center" justify="start">
            <Block hide={['xs', 'sm']}>Hello</Block>
            <IconButton><SvgIconHome /></IconButton>
          </Block>
        </ToolbarGroup>
      </ToolbarBlock>
    </Block>
    <Block flex layout="row" scroll>
     <Block flex="33">
        <List>
          <ListItem primaryText="Inbox" />
          <ListItem primaryText="Starred" />
          <ListItem primaryText="Sent mail" />
          <ListItem primaryText="Drafts" />
          <ListItem primaryText="Inbox" />
        </List>
      </Block>
      <List>
        <ListItem primaryText="Inbox" />
        <ListItem primaryText="Starred" />
        <ListItem primaryText="Sent mail" />
        <ListItem primaryText="Drafts" />
        <ListItem primaryText="Inbox" />
      </List>  
    </Block>
  </Block>
);

Add `priority` and `isMatch` properties to `createBreakpoints`. priority is large to small.

This is an initial take on layout feature heavily influenced by [angular material](https://material.angularjs.org/latest/layout/introduction). There are 2 components exposed here. - `<Block />` Component. Works very similar to the `layout` attributes in **angular material** - `withLayout` HOC. which wraps any component with layout properties which gets rended as `className` property. Also included is the option to set layout properties for media query breakpoints. Example use of `<Block />` component ```jsx import { List, ListItem, Toolbar, ToolbarGroup } from 'material-ui'; import { Block } from 'material-ui/Layout'; import { Link } from 'react-router'; import SvgIconHome from 'material-ui/svg-icons/action/home'; const ToolbarBlock = withLayout(Toolbar); const BlockLink = withLayout(Link); export default (props) => ( <Block flex layout="column"> <Block flex="none"> <ToolbarBlock layout="row"> <ToolbarGroup> <Block layout="row" align="center" justify="start"> <Block hide={['xs', 'sm']}>Hello</Block> <IconButton><SvgIconHome /></IconButton> </Block> </ToolbarGroup> </ToolbarBlock> </Block> <Block flex layout="column" scroll> <List> <ListItem primaryText="Inbox" /> <ListItem primaryText="Starred" /> <ListItem primaryText="Sent mail" /> <ListItem primaryText="Drafts" /> <ListItem primaryText="Inbox" /> </List> </Block> </Block> ); ```

oliviertassinari

Wow, that seems like a quite evolved solution.
I'm still wondering about the problem solved.
How is the problem solved related to the Material Design Specification?

Well, I start to find some answer to this question under the Layout section. Feels like it's going in the right direction

It's based on a 12 column grid system
They focus on consistent margins and gutters
A persistent drawer pushs the responsive grid
A temporary drawer does not affect the grid

oliviertassinari · 2016-10-02T20:45:45Z

src/styles/breakpoints.js

-  return { keys, values, up, down, only, getWidth };
+  let matched = priority
+    .reduce((a, k) => {
+      const media = window.matchMedia(breakpoints[k]);


How does matchMedia behave when doing server-side rendering?

Thanks for the feedback. Well I haven't fully thought about it. But one way is to add some listeners on componentWillMount and setState. Currently I've been using redux to monitor media query changes.

My biggest concern is regarding having a correct layout during the first paint. And not waiting for the js to load in order to get the correct one. As far as I know, we can't use the matchMedia API with this constrainte.
What are we using it for? Why not using the CSS media queries?

Well on the flip side, If we do use media queries for all, the number of classes will go up significantly. I am not sure the performance consequence of JSS rendering so many classes in the style tag. If the performance is negligible This can be done.

Well, from my point of view, taking the server-side rendering into account, we don't have much choice.
The other option is not to render the layout on the server to prevent a flash of content.
Hopefully, JSS is quite performant and that shouldn't be a real issue.

- Some of the flex styles weren't applied as expected. - add some tests for flex

Closes mui#5276.

Address mui#5269 (comment).

oliviertassinari · 2016-10-27T22:51:30Z

I have just came across http://stackoverflow.com/questions/33671469/material-ui-and-grid-system. That's just to add some context on how much people are looking for an answer here.
Layout could be one.

timothyklim · 2016-11-11T12:25:10Z

Is there any progress?

rosskevin · 2016-11-15T16:49:05Z

@timothyklim see this for an alternative: #3614 (comment)

- fixed `align` PropTypes values - use camelCase for all classnames - make it easy to read

Demo to show whats possible with this component and how flexible (no pun intended) and easy it is to work with.

oliviertassinari

The new docs demo is quite cool 👍 .
The Angular Layout approach is really interesting.
It's quite powerful. I hope we can move this PR forward into next.
Notice that the component would be a good candidate to write test regression against.

oliviertassinari · 2016-11-25T21:06:41Z

src/styles/breakpoints.js

-  return { keys, values, up, down, only, getWidth };
+  let matched = priority
+    .reduce((a, k) => {
+      const media = window.matchMedia(breakpoints[k]);


Well, from my point of view, taking the server-side rendering into account, we don't have much choice.
The other option is not to render the layout on the server to prevent a flash of content.
Hopefully, JSS is quite performant and that shouldn't be a real issue.

oliviertassinari · 2016-11-25T21:08:15Z

src/Layout/Block.js

+/*
+* This is the Base <Block /> Element
+*/
+export default withLayout('div');


I'm curious about this API decision. What's the benefit of having a Higer order Component over a component that accepts a component property?

oliviertassinari · 2016-11-25T21:10:21Z

src/Layout/with-layout.js

+  order = 0,
+  offset = 0,
+  align = 'stretch', // Cross Axis
+  justify = 'start', // Main Axis


I think that it would be better to use the defaultProps of React over a default argument value.
That would allow us to automatically generate the documentation.

Would the defaultProps have impact on the this component being pure?

oliviertassinari · 2016-11-25T21:11:17Z

src/Layout/with-layout.js

+      md,
+      lg,
+      xl,
+      ...others


other to be coherent with the code base.

oliviertassinari · 2016-11-25T21:13:27Z

src/Layout/with-layout.spec.js

+import { createMountWithContext } from 'test/utils';
+import withLayout, { styleSheet } from './with-layout';
+
+describe('withFlex HOC', () => {


withLayout?

naming. I am not 100% set on choosing withLayout over withFlex for the HOC. Care to suggest a name?

oliviertassinari · 2016-11-25T21:16:24Z

docs/site/src/demos/layout/Interactive.js

+import IconButton from 'material-ui/IconButton';
+import { teal, deepPurple } from 'material-ui/styles/colors';
+
+const styleSheet = createStyleSheet('Interactive', ({ shadows, palette }) => ({


Interactive is in a global namespace. It would be better to scope it.

oliviertassinari · 2016-11-25T22:01:50Z

src/Layout/layout-styles.js

+
+const flexOrder = () => ({
+  flexOrder: { order: 0 },
+  ...generateProps((i) => ({ [`flexOrder${i}`]: { order: i } }), -20, 20),


[-20,20] is quite a big range. Would using inline-style for this be a good approach? Or using a smaller window? I know that angular/material is using this value.

oliviertassinari · 2016-11-25T22:04:29Z

src/Layout/layout-styles.js

+});
+
+const boxSizing = 'border-box';
+const generateFlexProps = partialRight(generateProps, [0, 20]);


Again, I'm wondering about this 20 granularity, having a smaller set of possibilities could enforce having a more consistent UI.

I think 12 should do it. as it is used in other grid layouts or make it configurable using theme?

oliviertassinari · 2016-11-25T22:19:43Z

docs/site/src/demos/layout/Interactive.js

+              cells.map((cell, i) => (
+                <Block {...cell} layout="row" align="center">
+                  <IconButton accent onClick={this.setCell(i)}>delete</IconButton>
+                  <Button>{`Cell ${i + 1}`}</Button>


I'm not sure we need a button here.

I added the button to make it a menu to set flex value to that block. I don't know if I am overdoing the demo.

oliviertassinari · 2016-11-25T22:20:12Z

docs/site/src/demos/layout/Interactive.js

+            {
+              cells.map((cell, i) => (
+                <Block {...cell} layout="row" align="center">
+                  <IconButton accent onClick={this.setCell(i)}>delete</IconButton>


For the sake of simplicity, I would keep a fixed number of children.

I had it at 3. Then just for fun I made it dynamic.

nathanmarks · 2016-11-27T01:30:40Z

Do we know what's going on with the commits here?

Conflicts: src/styles/breakpoints.js

from `Interactive` on the demo also make variables coherent

msuntharesan · 2016-12-02T02:27:00Z

@nathanmarks I think I rebased instead of merging. If it is a problem I can close this PR and resubmit

mbrookes · 2016-12-02T19:15:24Z

@VanthiyaThevan You should be able to clean up on a new branch by cherry picking the relevant commits. I see you're developing directly on next, but in general you might find it easier to work on a separate branch, and keep next in sync with upstream.

also generate property name based on type. add test for offset and order

oliviertassinari · 2016-12-05T20:42:56Z

@VanthiyaThevan Hey, I have been implementing a LayoutBlock component at the office taking quite some inspiration from this PR (Angular inspiration) and @rosskevin one (flexboxgrid, Bootstrap inspiration). I'm will share the API I have in mind with you.

We have a tradeoff to make between how much things can be customized depending on the screen size and the overall CSS we inject.

msuntharesan · 2016-12-07T01:03:22Z

@mbrookes Thanks for the advice. I've already stated doing that.

@oliviertassinari That sounds cool. I would love to see your take on this.

As for the server side rendering and media query, we could incorporate react-media. I've never tried it but seem interesting

msuntharesan · 2016-12-20T00:27:40Z

I have a dilemma with regards to the documentation. Right now the document doesn't get generaged because the proptypes are defined inside the HOC. The solutions I came up to get around are

Drop the HOC and go with just the Block component
switch to it flow type and export it

May be another option is make document generator to look for HOC components as well. Any suggestion is appreciated.

oliviertassinari · 2016-12-21T00:18:08Z

@VanthiyaThevan Hey, I have started working on that Layout component by giving it a shot in #5808. I have been using quite some different library as inspiration, included your PR that helped me. My version is quite simple. I would love to get your feedback.

oliviertassinari · 2016-12-25T21:35:39Z

@VanthiyaThevan Thanks a lot for your effort here 💯 . I wouldn't have been able to get #5808 out without your PR!
If you find anything that could be improved, fell free to raise your voice / open a PR. Thanks.

A preview is available here.

emaillenin · 2016-12-28T06:06:50Z

@oliviertassinari Can you share the source of that preview? It has the source only for specific sections. I am looking for the code to make the AppBar with menu responsive.

oliviertassinari · 2016-12-28T12:31:10Z

@emaillenin Everything is public, it's from that PR: #5808.

msuntharesan · 2016-12-28T13:40:49Z

Thanks. Glad to have been able to help. This is cool. Lot simpler and a lot less classes.

Suntharesan Mohan added 3 commits September 27, 2016 15:40

[style]: Add priority and isMatch

48700a8

Add `priority` and `isMatch` properties to `createBreakpoints`. priority is large to small.

[mocha]: jsdom stub window.matchMedia function

460df18

muibot added the PR: Needs Review label Sep 27, 2016

msuntharesan changed the title ~~Next~~ [Layout]: Add a new Layout components Sep 27, 2016

msuntharesan changed the title ~~[Layout]: Add a new Layout components~~ [Layout]: Add new Layout components Sep 27, 2016

stunaz mentioned this pull request Sep 29, 2016

Responsive Material-UI #3614

Closed

oliviertassinari added the next label Sep 29, 2016

oliviertassinari reviewed Oct 2, 2016

View reviewed changes

[Layout]: fix some of the flex issues

b4309bf

- Some of the flex styles weren't applied as expected. - add some tests for flex

muibot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: Needs Review labels Oct 5, 2016

oliviertassinari and others added 15 commits October 5, 2016 22:40

[babel] Fix plugin format

2f451e3

Closes mui#5276.

[test] Improve the solution

9f72b65

Address mui#5269 (comment).

[docs] Import the Related projects section from master

73e232c

[docs] Migrate required-knowledge section from master

93ed119

Add card component basics

8d85105

Rebase and update docs

a5cf000

Fix lint error

377755b

Fix flow errors

1dc8259

[eslint] Go deeper in the folder strucutre

4696d17

[test] Fix the regression test suite on travis

97a262b

[test] Fix the regression test suite

1b2aa01

[TextField] Add required prop

d590328

[TextField] Add a visual regression test

657760d

[test] Run eslint on more files

2339048

[TextField] Follow the spec more closely

c845258

muibot removed the PR: Needs Review label Nov 23, 2016

[git]: fix merge conflict

658e001

msuntharesan force-pushed the next branch from ec14d37 to 658e001 Compare November 23, 2016 13:51

muibot added the PR: Needs Review label Nov 23, 2016

Suntharesan Mohan added 3 commits November 23, 2016 10:38

[layout]: better nesting using jss-nested

12c906c

- fixed `align` PropTypes values - use camelCase for all classnames - make it easy to read

[layout]: flexNone should be 0 0 auto

fe2eb6c

[Layout]: interactive demo

7fd0af1

Demo to show whats possible with this component and how flexible (no pun intended) and easy it is to work with.

oliviertassinari reviewed Nov 25, 2016

View reviewed changes

oliviertassinari removed the PR: Needs Review label Nov 25, 2016

muibot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 30, 2016

Merge remote-tracking branch 'refs/remotes/upstream/next'

157cec5

Conflicts: src/styles/breakpoints.js

muibot added PR: Needs Review and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Dec 2, 2016

[Layout]: renaming to LayoutInteractive

8c3e8c5

from `Interactive` on the demo also make variables coherent

[Layout]: fix typo

238b670

[layout]: fix offset referencing order prop.

dbe2e25

also generate property name based on type. add test for offset and order

oliviertassinari mentioned this pull request Dec 8, 2016

[Docs] Add responsive grid recommendations #3066

Closed

oliviertassinari mentioned this pull request Dec 20, 2016

[Layout] Add a new component #5808

Merged

6 tasks

oliviertassinari closed this Dec 25, 2016

[Layout]: Add new Layout components #5267

[Layout]: Add new Layout components #5267

Conversation

msuntharesan commented Sep 27, 2016 • edited Loading

oliviertassinari left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msuntharesan Oct 3, 2016 • edited Loading

Choose a reason for hiding this comment

oliviertassinari Oct 3, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari commented Oct 27, 2016

timothyklim commented Nov 11, 2016

rosskevin commented Nov 15, 2016

oliviertassinari left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nathanmarks commented Nov 27, 2016 • edited Loading

msuntharesan commented Dec 2, 2016

mbrookes commented Dec 2, 2016

oliviertassinari commented Dec 5, 2016 • edited Loading

msuntharesan commented Dec 7, 2016 • edited Loading

msuntharesan commented Dec 20, 2016

oliviertassinari commented Dec 21, 2016 • edited Loading

oliviertassinari commented Dec 25, 2016 • edited Loading

emaillenin commented Dec 28, 2016

oliviertassinari commented Dec 28, 2016

msuntharesan commented Dec 28, 2016

[Layout]: Add new `Layout` components #5267

[Layout]: Add new `Layout` components #5267

msuntharesan commented Sep 27, 2016 •

edited

Loading

msuntharesan Oct 3, 2016 •

edited

Loading

oliviertassinari Oct 3, 2016 •

edited

Loading

nathanmarks commented Nov 27, 2016 •

edited

Loading

oliviertassinari commented Dec 5, 2016 •

edited

Loading

msuntharesan commented Dec 7, 2016 •

edited

Loading

oliviertassinari commented Dec 21, 2016 •

edited

Loading

oliviertassinari commented Dec 25, 2016 •

edited

Loading