From 10ee477b879c6e7b1cd87eb8ae7e4340a0b4953d Mon Sep 17 00:00:00 2001 From: Bertho Date: Thu, 10 Jan 2019 12:19:37 +0100 Subject: [PATCH 1/5] reusable functions --- src/Collapse.jsx | 64 +++++++++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/src/Collapse.jsx b/src/Collapse.jsx index 9dd2c83..0f45bfd 100644 --- a/src/Collapse.jsx +++ b/src/Collapse.jsx @@ -60,40 +60,44 @@ class Collapse extends Component { this.setActiveKey(activeKey); } - getItems() { + getNewChild(child, index) { + if (!child) return null; + const activeKey = this.state.activeKey; - const { prefixCls, accordion, destroyInactivePanel, expandIcon, children } = this.props; - const newChildren = []; + const { prefixCls, accordion, destroyInactivePanel, expandIcon } = this.props; + // If there is no key provide, use the panel order as default key + const key = child.key || String(index); + const { header, headerClass, disabled } = child.props; + let isActive = false; + if (accordion) { + isActive = activeKey[0] === key; + } else { + isActive = activeKey.indexOf(key) > -1; + } + + const props = { + key, + header, + headerClass, + isActive, + prefixCls, + destroyInactivePanel, + openAnimation: this.state.openAnimation, + accordion, + children: child.props.children, + onItemClick: disabled ? null : this.onClickItem.bind(this, key), + expandIcon, + }; + + return React.cloneElement(child, props); + } + + getItems() { + const { children } = this.props; const childList = isFragment(children) ? children.props.children : children; - Children.forEach(childList, (child, index) => { - if (!child) return; - // If there is no key provide, use the panel order as default key - const key = child.key || String(index); - const { header, headerClass, disabled } = child.props; - let isActive = false; - if (accordion) { - isActive = activeKey[0] === key; - } else { - isActive = activeKey.indexOf(key) > -1; - } + const newChildren = Children.map(childList, this.getNewChild.bind(this)); - const props = { - key, - header, - headerClass, - isActive, - prefixCls, - destroyInactivePanel, - openAnimation: this.state.openAnimation, - accordion, - children: child.props.children, - onItemClick: disabled ? null : () => this.onClickItem(key), - expandIcon, - }; - - newChildren.push(React.cloneElement(child, props)); - }); // ref: https://github.com/ant-design/ant-design/issues/13884 if (isFragment(children)) { return ( From 080cfeda96e86c13e5fe6dd01f7eea4c4149a17e Mon Sep 17 00:00:00 2001 From: Bertho Date: Thu, 10 Jan 2019 15:41:00 +0100 Subject: [PATCH 2/5] do not bind parameter in render --- src/Collapse.jsx | 12 ++++++++---- src/Panel.jsx | 10 ++++++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/Collapse.jsx b/src/Collapse.jsx index 0f45bfd..b459242 100644 --- a/src/Collapse.jsx +++ b/src/Collapse.jsx @@ -1,4 +1,4 @@ -import React, { Component, Children } from 'react'; +import React, { PureComponent, Children } from 'react'; import PropTypes from 'prop-types'; import CollapsePanel from './Panel'; import openAnimationFactory from './openAnimationFactory'; @@ -13,7 +13,7 @@ function toArray(activeKey) { return currentActiveKey; } -class Collapse extends Component { +class Collapse extends PureComponent { constructor(props) { super(props); @@ -27,6 +27,9 @@ class Collapse extends Component { openAnimation: this.props.openAnimation || openAnimationFactory(this.props.prefixCls), activeKey: toArray(currentActiveKey), }; + + this.getNewChild = this.getNewChild.bind(this); + this.onClickItem = this.onClickItem.bind(this); } componentWillReceiveProps(nextProps) { @@ -77,6 +80,7 @@ class Collapse extends Component { const props = { key, + panelKey: key, header, headerClass, isActive, @@ -85,7 +89,7 @@ class Collapse extends Component { openAnimation: this.state.openAnimation, accordion, children: child.props.children, - onItemClick: disabled ? null : this.onClickItem.bind(this, key), + onItemClick: disabled ? null : this.onClickItem, expandIcon, }; @@ -96,7 +100,7 @@ class Collapse extends Component { const { children } = this.props; const childList = isFragment(children) ? children.props.children : children; - const newChildren = Children.map(childList, this.getNewChild.bind(this)); + const newChildren = Children.map(childList, this.getNewChild); // ref: https://github.com/ant-design/ant-design/issues/13884 if (isFragment(children)) { diff --git a/src/Panel.jsx b/src/Panel.jsx index 7df2b5a..86670b4 100644 --- a/src/Panel.jsx +++ b/src/Panel.jsx @@ -1,13 +1,15 @@ -import React, { Component } from 'react'; +import React, { PureComponent } from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; import PanelContent from './PanelContent'; import Animate from 'rc-animate'; -class CollapsePanel extends Component { +class CollapsePanel extends PureComponent { handleItemClick = () => { - if (this.props.onItemClick) { - this.props.onItemClick(); + const { onItemClick, panelKey } = this.props; + + if (typeof onItemClick === 'function') { + onItemClick(panelKey); } } From 25769ab027235861f125c4140e4851d115b6d069 Mon Sep 17 00:00:00 2001 From: Bertho Date: Thu, 10 Jan 2019 16:25:48 +0100 Subject: [PATCH 3/5] use shallowEqual instead of PureComponent --- package.json | 3 ++- src/Collapse.jsx | 11 ++++++++--- src/Panel.jsx | 10 ++++++++-- src/PanelContent.jsx | 3 ++- 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index ab538a5..e44aebf 100644 --- a/package.json +++ b/package.json @@ -71,6 +71,7 @@ "css-animation": "1.x", "prop-types": "^15.5.6", "rc-animate": "2.x", - "react-is": "^16.7.0" + "react-is": "^16.7.0", + "shallowequal": "^1.1.0" } } diff --git a/src/Collapse.jsx b/src/Collapse.jsx index b459242..de92359 100644 --- a/src/Collapse.jsx +++ b/src/Collapse.jsx @@ -1,9 +1,10 @@ -import React, { PureComponent, Children } from 'react'; +import React, { Component, Children } from 'react'; import PropTypes from 'prop-types'; import CollapsePanel from './Panel'; import openAnimationFactory from './openAnimationFactory'; import classNames from 'classnames'; import { isFragment } from 'react-is'; +import shallowEqual from 'shallowequal'; function toArray(activeKey) { let currentActiveKey = activeKey; @@ -13,7 +14,7 @@ function toArray(activeKey) { return currentActiveKey; } -class Collapse extends PureComponent { +class Collapse extends Component { constructor(props) { super(props); @@ -45,6 +46,10 @@ class Collapse extends PureComponent { } } + shouldComponentUpdate(nextProps, nextState) { + return !shallowEqual(this.props, nextProps) || !shallowEqual(this.state, nextState); + } + onClickItem(key) { let activeKey = this.state.activeKey; if (this.props.accordion) { @@ -137,7 +142,7 @@ class Collapse extends PureComponent { Collapse.propTypes = { children: PropTypes.any, - prefixCls: PropTypes.string, + prefixCls: PropTypes.number, activeKey: PropTypes.oneOfType([ PropTypes.string, PropTypes.arrayOf(PropTypes.string), diff --git a/src/Panel.jsx b/src/Panel.jsx index 86670b4..09a8c27 100644 --- a/src/Panel.jsx +++ b/src/Panel.jsx @@ -1,10 +1,15 @@ -import React, { PureComponent } from 'react'; +import React, { Component } from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; import PanelContent from './PanelContent'; import Animate from 'rc-animate'; +import shallowEqual from 'shallowequal'; + +class CollapsePanel extends Component { + shouldComponentUpdate(nextProps) { + return !shallowEqual(this.props, nextProps); + } -class CollapsePanel extends PureComponent { handleItemClick = () => { const { onItemClick, panelKey } = this.props; @@ -107,6 +112,7 @@ CollapsePanel.propTypes = { accordion: PropTypes.bool, forceRender: PropTypes.bool, expandIcon: PropTypes.func, + panelKey: PropTypes.any, }; CollapsePanel.defaultProps = { diff --git a/src/PanelContent.jsx b/src/PanelContent.jsx index 8512d90..f53da61 100644 --- a/src/PanelContent.jsx +++ b/src/PanelContent.jsx @@ -1,10 +1,11 @@ import React, { Component } from 'react'; import PropTypes from 'prop-types'; import classnames from 'classnames'; +import shallowEqual from 'shallowequal'; class PanelContent extends Component { shouldComponentUpdate(nextProps) { - return this.props.forceRender || this.props.isActive || nextProps.isActive; + return this.props.forceRender || !shallowEqual(this.props, nextProps); } render() { From 558ca71f79ccb94eb084cc40162f92a3d851164a Mon Sep 17 00:00:00 2001 From: Bertho Date: Thu, 10 Jan 2019 16:57:45 +0100 Subject: [PATCH 4/5] fix propTypes --- src/Collapse.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Collapse.jsx b/src/Collapse.jsx index 3d371ff..48febb2 100644 --- a/src/Collapse.jsx +++ b/src/Collapse.jsx @@ -141,7 +141,7 @@ class Collapse extends Component { Collapse.propTypes = { children: PropTypes.any, - prefixCls: PropTypes.number, + prefixCls: PropTypes.string, activeKey: PropTypes.oneOfType([ PropTypes.string, PropTypes.arrayOf(PropTypes.string), From a2fe261f4d8ef012b7de5a5ede954d20fdbe741c Mon Sep 17 00:00:00 2001 From: Bertho Date: Mon, 14 Jan 2019 11:47:50 +0100 Subject: [PATCH 5/5] arrow functions instead of binding --- src/Collapse.jsx | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/Collapse.jsx b/src/Collapse.jsx index 48febb2..e7b5393 100644 --- a/src/Collapse.jsx +++ b/src/Collapse.jsx @@ -28,9 +28,6 @@ class Collapse extends Component { openAnimation: this.props.openAnimation || openAnimationFactory(this.props.prefixCls), activeKey: toArray(currentActiveKey), }; - - this.getNewChild = this.getNewChild.bind(this); - this.onClickItem = this.onClickItem.bind(this); } componentWillReceiveProps(nextProps) { @@ -50,7 +47,7 @@ class Collapse extends Component { return !shallowEqual(this.props, nextProps) || !shallowEqual(this.state, nextState); } - onClickItem(key) { + onClickItem = key => { let activeKey = this.state.activeKey; if (this.props.accordion) { activeKey = activeKey[0] === key ? [] : [key]; @@ -68,7 +65,7 @@ class Collapse extends Component { this.setActiveKey(activeKey); } - getNewChild(child, index) { + getNewChild = (child, index) => { if (!child) return null; const activeKey = this.state.activeKey; @@ -101,7 +98,7 @@ class Collapse extends Component { return React.cloneElement(child, props); } - getItems() { + getItems = () => { const { children } = this.props; const childList = isFragment(children) ? children.props.children : children; const newChildren = Children.map(childList, this.getNewChild); @@ -118,7 +115,7 @@ class Collapse extends Component { return newChildren; } - setActiveKey(activeKey) { + setActiveKey = activeKey => { if (!('activeKey' in this.props)) { this.setState({ activeKey }); }