[docs] Add a Drawer section #6113

ArcanisCz · 2017-02-10T22:07:03Z

Created missing Drawer section base on same section on master branch. Included only what is already done in Drawer component

ArcanisCz · 2017-02-10T22:08:39Z

src/Drawer/Drawer.js

@@ -72,6 +72,7 @@ export default class Drawer extends Component {
    docked: PropTypes.bool,
    enterTransitionDuration: PropTypes.number,
    leaveTransitionDuration: PropTypes.number,
+    onRequestClose: PropTypes.func,


I am not sure about this one. It is true that additional props specified will be passed to Modal, but that means for most basic use case (controlled open/close Drawer) you have to know implementation internals of Drawer. Which is why i included this explicit prop, which is only passed.
Any recommendations how to handle this type of situation?

I think that you made a good call. Wrapping component should be exposing critical properties.
I know that it means duplication and violation of the DRY principal, but we haven't found better solutions.

/** * Callback fired when the internal modal requests to be closed. */ onRequestClose: PropTypes.func,

mbrookes · 2017-02-10T22:36:57Z

@ArcanisCz Thanks for your ongoing contributions to next! You're right to expose any sub-component props that are useful in the parent component, but no need to pull them out in the spread, they can just pass through in ...other.

Would you mind though adding comment to each one though, either copying from master or writing a new one? Those are used to build the API docs.

Once you've done that you could generate updated API docs.

oliviertassinari

The example is simple, and that's ok to begging with. We could try to follow a more real life situation.

oliviertassinari · 2017-02-10T23:26:36Z

docs/site/src/demos/drawers/UndockedDrawer.js

+import Button from 'material-ui/Button';
+import { MenuItem } from 'material-ui/Menu';
+
+const styleSheet = createStyleSheet('Drawers', () => ({


('UndockedDrawer', (

oliviertassinari · 2017-02-10T23:28:51Z

docs/site/src/demos/drawers/UndockedDrawer.js

+    return (
+      <div>
+        <Button onClick={this.handleOpen}>Open Drawer</Button>
+        <Drawer open={this.state.open} anchor={this.state.anchor} onRequestClose={this.handleClose}>


this.state.anchor?

oliviertassinari · 2017-02-10T23:32:24Z

src/Drawer/Drawer.js

@@ -72,6 +72,7 @@ export default class Drawer extends Component {
    docked: PropTypes.bool,
    enterTransitionDuration: PropTypes.number,
    leaveTransitionDuration: PropTypes.number,
+    onRequestClose: PropTypes.func,


I think that you made a good call. Wrapping component should be exposing critical properties.
I know that it means duplication and violation of the DRY principal, but we haven't found better solutions.

oliviertassinari · 2017-02-10T23:33:29Z

src/Drawer/Drawer.js

@@ -72,6 +72,7 @@ export default class Drawer extends Component {
    docked: PropTypes.bool,
    enterTransitionDuration: PropTypes.number,
    leaveTransitionDuration: PropTypes.number,
+    onRequestClose: PropTypes.func,


/** * Callback fired when the internal modal requests to be closed. */ onRequestClose: PropTypes.func,

oliviertassinari · 2017-02-10T23:33:52Z

src/Drawer/Drawer.js

@@ -132,6 +134,7 @@ export default class Drawer extends Component {

    const containerProps = {
      className: classNames(classes.modal, className),
+      onRequestClose,


We might not need to explicit it here. To try, eslint may complains.

ArcanisCz · 2017-02-11T01:28:58Z

src/List/ListItem.js

@@ -38,6 +39,16 @@ export const styleSheet = createStyleSheet('ListItem', (theme) => {
      paddingLeft: 16,
      paddingRight: 16,
    },
+    button: {


Stolen from Button, but not sure if correct approach. But <ListItem button> didnt have proper hover effect of Button

That's not directly linked but we can nest a button inside a <ListItem />. We do it with the documentation drawer nav items.

I think that you made a good call here too.

Well, tried that Button in ListItem, but it seems its not ready yet, styles and paddings were weird.

ArcanisCz · 2017-02-11T01:29:45Z

src/svg-icons/delete.js

@@ -0,0 +1,16 @@
+/* eslint-disable */


Added some icons from master, but i suppose they will be transferred all at some point?

Yes, once #6104 is done, we should be able to replace them.

ArcanisCz · 2017-02-11T01:31:41Z

@mbrookes Sure, i completely overlooked that api generation! thought that its manula, when .md files were in sources :X When tried to re-generate docs, it generated much more changes than only my Drawer, is it ok to commit that? (aside from some clrf problems)
@oliviertassinari Nicer example added :)

oliviertassinari · 2017-02-11T11:25:52Z

docs/site/src/demos/drawers/UndockedDrawer.js

+
+const styleSheet = createStyleSheet('UndockedDrawer', () => ({
+  list: {
+    width: '280px',


Why not using the same width than the documentation drawer? By the way, you don't need to add this px.

width: 250,

noticed that width in specs, but then realized that width prop is missing in Drawer anyway, so havent bothered with this much now. Sorry, will change to master's value!

oliviertassinari · 2017-02-11T11:27:53Z

docs/site/src/demos/drawers/UndockedDrawer.js

+        <Button onClick={this.handleOpen}>Open Drawer</Button>
+        <Drawer open={this.state.open} onRequestClose={this.handleClose}>
+          <List className={classes.list} padding={false}>
+            <ListItem button onClick={this.handleClose}>


If you want to reduce boilerplate, you can move all those onClick handlers to the Drawer. The event propagation will work fine.

oliviertassinari · 2017-02-11T11:28:30Z

src/Drawer/Drawer.js

@@ -56,6 +56,9 @@ export const styleSheet = createStyleSheet('Drawer', (theme) => {
 */
 export default class Drawer extends Component {
  static propTypes = {
+    /**
+     * Side, which will `Drawer` appears from


For consistency, we add dots at the end of all our descriptions .

oliviertassinari · 2017-02-11T11:28:49Z

src/Drawer/Drawer.js

+     */
+    onRequestClose: PropTypes.func,
+    /**
+     * If true, the `Drawer` is opened.


... is open.

oliviertassinari · 2017-02-11T11:32:23Z

src/svg-icons/delete.js

@@ -0,0 +1,16 @@
+/* eslint-disable */


Yes, once #6104 is done, we should be able to replace them.

oliviertassinari · 2017-02-11T11:38:18Z

src/List/ListItem.js

@@ -38,6 +39,16 @@ export const styleSheet = createStyleSheet('ListItem', (theme) => {
      paddingLeft: 16,
      paddingRight: 16,
    },
+    button: {
+      transition: transitions.multi(['background-color', 'box-shadow'], '250ms'),


No need to transition on the box-shadow.

oliviertassinari · 2017-02-11T11:41:24Z

src/List/ListItem.js

@@ -38,6 +39,16 @@ export const styleSheet = createStyleSheet('ListItem', (theme) => {
      paddingLeft: 16,
      paddingRight: 16,
    },
+    button: {


That's not directly linked but we can nest a button inside a <ListItem />. We do it with the documentation drawer nav items.

I think that you made a good call here too.

oliviertassinari · 2017-02-11T11:42:27Z

src/svg-icons/delete.js

+  </SvgIcon>
+);
+ActionDelete = pure(ActionDelete);
+ActionDelete.displayName = 'ActionDelete';


No need for the displayName, but don't worry, this folder will soon be gone.

oliviertassinari · 2017-02-11T11:43:07Z

@ArcanisCz It's almost good to be merged 🎉

mbrookes · 2017-02-11T18:06:37Z

@ArcanisCz Sorry, could you fix the merge conflict? Nothing major - renaming of zDepth to elevation beside one of your changes.

Created missing Drawer section base on same section on master branch. Included only what is already done in Drawer component

ArcanisCz · 2017-02-12T00:29:43Z

Sure, no problem!

oliviertassinari · 2017-02-12T09:29:09Z

@ArcanisCz Thanks again!

muibot added the PR: Needs Review label Feb 10, 2017

ArcanisCz commented Feb 10, 2017

View reviewed changes

ArcanisCz changed the title ~~[docs] added Drawer section~~ [docs] added Drawer section (next) Feb 10, 2017

mbrookes added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI docs Improvements or additions to the documentation next and removed PR: Needs Review labels Feb 10, 2017

oliviertassinari requested changes Feb 10, 2017

View reviewed changes

muibot added the PR: Needs Review label Feb 11, 2017

ArcanisCz commented Feb 11, 2017

View reviewed changes

oliviertassinari requested changes Feb 11, 2017

View reviewed changes

oliviertassinari removed the PR: Needs Review label Feb 11, 2017

muibot added the PR: Needs Review label Feb 11, 2017

oliviertassinari changed the title ~~[docs] added Drawer section (next)~~ [docs] added Drawer section Feb 11, 2017

oliviertassinari approved these changes Feb 11, 2017

View reviewed changes

oliviertassinari added PR: Review Accepted and removed PR: Needs Review PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI labels Feb 11, 2017

mbrookes added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 11, 2017

ArcanisCz added 5 commits February 12, 2017 01:20

[docs] added Drawer section

2426a0b

Created missing Drawer section base on same section on master branch. Included only what is already done in Drawer component

[docs] Drawer api synced with component

247827d

[docs] commented Drawer propTypes

c544a1d

[Docs] more complex Drawer demo

fd666a2

[Docs] more complex Drawer demo (changes afer CR)

9aab549

ArcanisCz force-pushed the feature/drawerDocs branch from 5e12608 to 9aab549 Compare February 12, 2017 00:28

muibot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 12, 2017

muibot added the PR: Needs Review label Feb 12, 2017

oliviertassinari removed the PR: Needs Review label Feb 12, 2017

oliviertassinari merged commit bf32550 into mui:next Feb 12, 2017

ArcanisCz deleted the feature/drawerDocs branch February 12, 2017 10:25

oliviertassinari changed the title ~~[docs] added Drawer section~~ [docs] Add a Drawer section Feb 12, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[docs] Add a Drawer section #6113

[docs] Add a Drawer section #6113

ArcanisCz commented Feb 10, 2017

ArcanisCz Feb 10, 2017 •

edited

Loading

oliviertassinari Feb 10, 2017 •

edited

Loading

oliviertassinari Feb 10, 2017

mbrookes commented Feb 10, 2017

oliviertassinari left a comment

oliviertassinari Feb 10, 2017 •

edited

Loading

oliviertassinari Feb 10, 2017

oliviertassinari Feb 10, 2017 •

edited

Loading

oliviertassinari Feb 10, 2017

oliviertassinari Feb 10, 2017

ArcanisCz Feb 11, 2017

oliviertassinari Feb 11, 2017

ArcanisCz Feb 11, 2017

ArcanisCz Feb 11, 2017

oliviertassinari Feb 11, 2017

ArcanisCz commented Feb 11, 2017 •

edited

Loading

oliviertassinari Feb 11, 2017

ArcanisCz Feb 11, 2017

oliviertassinari Feb 11, 2017

oliviertassinari Feb 11, 2017

oliviertassinari Feb 11, 2017

oliviertassinari Feb 11, 2017

oliviertassinari Feb 11, 2017

oliviertassinari Feb 11, 2017

oliviertassinari Feb 11, 2017

oliviertassinari commented Feb 11, 2017

mbrookes commented Feb 11, 2017

ArcanisCz commented Feb 12, 2017

oliviertassinari commented Feb 12, 2017

[docs] Add a Drawer section #6113

[docs] Add a Drawer section #6113

Conversation

ArcanisCz commented Feb 10, 2017

ArcanisCz Feb 10, 2017 • edited Loading

Choose a reason for hiding this comment

oliviertassinari Feb 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbrookes commented Feb 10, 2017

oliviertassinari left a comment

Choose a reason for hiding this comment

oliviertassinari Feb 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari Feb 10, 2017 • edited Loading

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

ArcanisCz commented Feb 11, 2017 • edited Loading

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

oliviertassinari commented Feb 11, 2017

mbrookes commented Feb 11, 2017

ArcanisCz commented Feb 12, 2017

oliviertassinari commented Feb 12, 2017

ArcanisCz Feb 10, 2017 •

edited

Loading

oliviertassinari Feb 10, 2017 •

edited

Loading

oliviertassinari Feb 10, 2017 •

edited

Loading

oliviertassinari Feb 10, 2017 •

edited

Loading

ArcanisCz commented Feb 11, 2017 •

edited

Loading