Skip to content

Commit

Permalink
Let React handle updates to style properties (#22)
Browse files Browse the repository at this point in the history
* fix(Collapse): Add displayName

* fix(Collapse): Add default className

* fix(Collapse): Add default className

* fix(Collapse): Use PureComponent for efficiency and use React to update styles

* tests(Collapse): Update tests to check the component for style changes
  • Loading branch information
deshiknaves authored and torleifhalseth committed Aug 31, 2018
1 parent 3452956 commit 32d1135
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 57 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ Callback function for when your transition on `height` (specified in `className`

#### `className`: PropType.string

You can specify a className with your desired style and animation
You can specify a className with your desired style and animation. By default `react-css-collapse-transition` will be added to the component.

```scss
.react-css-collapse-transition {
Expand Down
60 changes: 31 additions & 29 deletions src/components/Collapse.jsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
import React, { Component } from 'react';
import React, { PureComponent } from 'react';
import PropTypes from 'prop-types';
import util from '../util';

const initialStyle = {
willChange: 'height',
height: '0px',
overflow: 'hidden',
visibility: 'hidden',
};

class Collapse extends Component {
constructor() {
super();
class Collapse extends PureComponent {
constructor(props) {
super(props);
this.onTransitionEnd = this.onTransitionEnd.bind(this);
this.setExpanded = this.setExpanded.bind(this);
this.setCollapsed = this.setCollapsed.bind(this);

this.state = {
willChange: 'height',
height: '0',
overflow: 'hidden',
visibility: 'hidden',
};
}

componentDidMount() {
Expand All @@ -31,22 +31,26 @@ class Collapse extends Component {
// expand
if (!this.props.isOpen && nextProps.isOpen) {
// have the element transition to the height of its inner content
this.setContentStyleProperty('height', `${this.content.scrollHeight}px`);
this.setContentStyleProperty('visibility', 'visible');
this.setState({
height: `${this.content.scrollHeight}px`,
visibility: 'visible',
});
}

// collapse
if (this.props.isOpen && !nextProps.isOpen) {
// explicitly set the element's height to its current pixel height, so we
// aren't transitioning out of 'auto'
this.setContentStyleProperty('height', `${this.content.scrollHeight}px`);
this.setState({ height: `${this.content.scrollHeight}px` });
util.requestAnimationFrame(() => {
// "pausing" the JavaScript execution to let the rendering threads catch up
// http://stackoverflow.com/questions/779379/why-is-settimeoutfn-0-sometimes-useful
setTimeout(() => {
this.setContentStyleProperty('height', '0px');
this.setContentStyleProperty('overflow', 'hidden');
}, 0);
this.setState({
height: '0',
overflow: 'hidden',
});
});
});
}
}
Expand All @@ -66,20 +70,16 @@ class Collapse extends Component {
}
}

setContentStyleProperty(property, value) {
if (this.content) {
this.content.style[property] = value;
}
}

setCollapsed() {
this.setContentStyleProperty('visibility', 'hidden');
this.setState({ visibility: 'hidden' });
}

setExpanded() {
this.setContentStyleProperty('height', 'auto');
this.setContentStyleProperty('overflow', 'visible');
this.setContentStyleProperty('visibility', 'visible');
this.setState({
height: 'auto',
overflow: 'visible',
visibility: 'visible',
});
}

render() {
Expand All @@ -88,7 +88,7 @@ class Collapse extends Component {
ref={(el) => {
this.content = el;
}}
style={initialStyle}
style={this.state}
className={this.props.className}
onTransitionEnd={this.onTransitionEnd}
>
Expand All @@ -98,9 +98,11 @@ class Collapse extends Component {
}
}

Collapse.displayName = 'Collapse';

Collapse.defaultProps = {
isOpen: false,
className: null,
className: 'react-css-collapse-transition',
children: null,
onRest: null,
};
Expand Down
50 changes: 23 additions & 27 deletions test/components/Collapse.spec.js
Original file line number Diff line number Diff line change
@@ -1,53 +1,49 @@
import React from 'react';
import { expect } from 'chai';
import sinon from 'sinon';
import { mount } from 'enzyme';
import { mount, shallow } from 'enzyme';
import util from '../../src/util';
import Collapse from '../../src/components/Collapse';

describe('<Collapse />', () => {
let requestAnimationFrameStub;
let setExpandedSpy;
let setContentStylePropertySpy;
let componentDidMountSpy;
let componentWillReceivePropsSpy;

beforeEach(() => {
requestAnimationFrameStub = sinon.stub(util, 'requestAnimationFrame');
setExpandedSpy = sinon.spy(Collapse.prototype, 'setExpanded');
setContentStylePropertySpy = sinon.spy(
Collapse.prototype,
'setContentStyleProperty',
);
componentDidMountSpy = sinon.spy(Collapse.prototype, 'componentDidMount');
componentWillReceivePropsSpy = sinon.spy(
Collapse.prototype,
'componentWillReceiveProps',
);
});

afterEach(() => {
requestAnimationFrameStub.restore();
setExpandedSpy.restore();
setContentStylePropertySpy.restore();
componentDidMountSpy.restore();
componentWillReceivePropsSpy.restore();
});

context('DOM element', () => {
it('should include className when defined', () => {
const className = 'collapse';
expect(
mount(<Collapse className={className} />).prop('className'),
shallow(<Collapse className={className} />).prop('className'),
).to.equal(className);
});

it('inner block should have height: 0px when collapsed', () => {
expect(
mount(<Collapse />)
.find('div')
.props().style.height,
).to.equal('0px');
const wrapper = shallow(<Collapse />);
expect(wrapper.prop('style').height).to.equal('0');
});

it('inner block should have height: 0px when open', () => {
const wrapper = mount(<Collapse isOpen />);
expect(wrapper.find('div').props().style.height).to.equal('0px');
const wrapper = shallow(<Collapse isOpen />);
expect(wrapper.prop('style').height).to.equal('0');
});
});
context('Component', () => {
Expand All @@ -59,35 +55,35 @@ describe('<Collapse />', () => {
);
sinon.assert.notCalled(requestAnimationFrameStub);
});

it('calls componentDidMount and setContentHeight with args auto', () => {
mount(<Collapse isOpen />);
sinon.assert.called(Collapse.prototype.componentDidMount);
expect(Collapse.prototype.setExpanded.calledOnce).to.equal(true);
});
it('calls componentWillReceiveProps when opened and calls setContentHeight', () => {

it('should update height when isOpen prop is changed to true', () => {
const wrapper = mount(<Collapse />);
wrapper.setProps({ isOpen: true });
sinon.assert.called(Collapse.prototype.componentWillReceiveProps);
expect(
Collapse.prototype.setContentStyleProperty.calledWith('height', '0px'),
).to.equal(true);
expect(wrapper.find('div').prop('style').height).to.equal('0px');
});
it('calls componentWillReceiveProps when opened and calls setContentVisibility', () => {

it('should update visibility when isOpen prop is changed to true', () => {
const wrapper = mount(<Collapse />);
wrapper.setProps({ isOpen: true });
sinon.assert.called(Collapse.prototype.componentWillReceiveProps);
expect(
Collapse.prototype.setContentStyleProperty.calledWith('visibility', 'visible'),
).to.equal(true);
expect(wrapper.find('div').prop('style').visibility).to.equal('visible');
});
it('calls componentWillReceiveProps when collapsed and calls setContentHeight', () => {

it('should update the height when isOpen is changed to false', () => {
const wrapper = mount(<Collapse isOpen />);
wrapper.setProps({ isOpen: false });
wrapper.update();
sinon.assert.called(Collapse.prototype.componentWillReceiveProps);
expect(
Collapse.prototype.setContentStyleProperty.calledWith('height', '0px'),
).to.equal(true);
expect(wrapper.find('div').prop('style').height).to.equal('0px');
});

it('calls requestAnimationFrame when collapsed', () => {
const wrapper = mount(<Collapse isOpen />);
wrapper.setProps({ isOpen: false });
Expand Down

0 comments on commit 32d1135

Please sign in to comment.