[#1] Replace MUI Drawer component with own implementation #43
Conversation
| } | ||
| } | ||
|
|
||
| getDrawerClassName = () => classNames(styles.dragArea); |
There was a problem hiding this comment.
Seems like this method is never used
| transition: 'opacity .3s ease-out, visibility .3s ease-out', | ||
| backgroundColor: 'rgba(0,0,0,.54)', | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Why these styles are inlined in JS? Why not to place them at CSS file?
|
|
||
| constructor(props) { | ||
| super(props); | ||
| this.state = { |
There was a problem hiding this comment.
tip: you could define initial state without using the constructor
class Drawer extends React.Component {
state = {
touchId: null,
/* ... */
};
}
| startY: null, | ||
| currentX: null, | ||
| currentY: null, | ||
| sidebarWidth: 256, |
There was a problem hiding this comment.
This value doesn't change, so it shouldn't be in state, it's more like a constant.
And I see that it's duplicated in styles. Maybe it's better to set ref on a drawer node and get width from it when neccessary:
class Drawer extends React.Component {
getSidebarWidth = () => this.drawerNode.clientWidth;
}
| } | ||
|
|
||
| .drawer { | ||
| color: rgba(0, 0, 0, 0.87); |
There was a problem hiding this comment.
There are mixed tabs and spaces. Consider using EditorConfig plugin for your text editor to prevent this kind of issues
| font-family: Roboto, sans-serif; | ||
| -webkit-tap-highlight-color: rgba(0, 0, 0, 0); | ||
| box-shadow: rgba(0, 0, 0, 0.16) 0px 3px 10px, rgba(0, 0, 0, 0.23) 0px 3px 10px; | ||
| border-radius: 0px; |
There was a problem hiding this comment.
Why it's needed to reset border-radius, shouldn't it be 0 by default?
| @@ -0,0 +1,23 @@ | |||
| .drawernone { | |||
| display: block; | |||
There was a problem hiding this comment.
Seems like this class doesn't change anything because div has display: block by default
| const drawer = classNames({ | ||
| [`${styles.drawer}`]: true, | ||
| }); | ||
| const dragHandle = ( |
There was a problem hiding this comment.
It's used only once, so it don't have to be stored in a variable
There was a problem hiding this comment.
Also currently dragHandle overlaps menu icon, so only a part of this icon is clickable:
| @@ -141,10 +143,8 @@ const EventPage = React.createClass({ | |||
| return ( | |||
| <Drawer | |||
| onRequestChange={(menuOpen) => this.setState({ menuOpen })} | |||
There was a problem hiding this comment.
It'd be better to split onRequestChange to two props: onOpen and onClose. Currently onRequestChange accepts a function with a single boolean argument, that's a bad practice (and that's MUI fault, not yours)
|
|
||
| getDrawerClassName = () => classNames(styles.dragArea); | ||
|
|
||
| overlayClick = () => { |
There was a problem hiding this comment.
I'd suggest renaming overlayClick to handleOverlayClick
No description provided.