-
Notifications
You must be signed in to change notification settings - Fork 224
updated dependencies, changed component to pure function using hooks #181
base: master
Are you sure you want to change the base?
Conversation
zgabievi
commented
Feb 20, 2019
- Updated dependencies to latest versions
- Changed class component to pure function
- Change states and lifecycles to hooks
Duuuuude yes! @balloob, @markusenglund anyway we can get this merged in? |
Would be so awesome to get this merged! |
I haven't really had the time to review PRs or maintain the library unfortunately. Maybe we could give commit access to someone else if people think it's important that it gets updated. |
} | ||
} | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we try to call this hook only when touchParams.identifier
changes, could be something like this:
useEffect(() => {
saveSidebarWidth();
}, [touchParams.identifier]);
<div | ||
className={props.sidebarClassName} | ||
style={sidebarStyle} | ||
ref={saveSidebarRef} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should here we just pass the sidebarRef
? We created the ref with useRef
hook so the old function maybe is unnecessary
ref={sidebarRef}
saveSidebarRef(node) { | ||
this.sidebar = node; | ||
} | ||
const saveSidebarRef = node => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we are creating the ref with useRef
hook I think we don't need this function anymore, what do you think?