Skip to content

Commit b520400

Browse files
Maxime Jantonbobylito
authored andcommitted
fix(autoHideContainer): dont prevent render with shouldComponentUpdate (#2076)
If you click on a DOM element which has a state (ex radio button), React wont see this state change because children component of `autoHideContainer` wont be updated because of the implementation of `shouldComponentUpdate`. It's a weird bug where UI and state aren't in sync because the navigator saves on it's own the state of the DOM element. fixes #2066
1 parent 44d27de commit b520400

File tree

2 files changed

+23
-44
lines changed

2 files changed

+23
-44
lines changed

src/decorators/__tests__/autoHideContainer-test.js

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,18 @@ describe('autoHideContainer', () => {
2222
const AutoHide = autoHideContainer(TestComponent);
2323
renderer.render(<AutoHide {...props} />);
2424
const out = renderer.getRenderOutput();
25-
expect(out).toEqualJSX(<TestComponent hello="son" />);
25+
expect(out).toEqualJSX(
26+
<div style={{display: ''}}>
27+
<TestComponent hello="son" />
28+
</div>
29+
);
2630
});
2731

2832
context('props.shouldAutoHideContainer', () => {
2933
let AutoHide;
3034
let component;
3135
let container;
36+
let innerContainer;
3237

3338
beforeEach(() => {
3439
AutoHide = autoHideContainer(TestComponent);
@@ -48,14 +53,15 @@ describe('autoHideContainer', () => {
4853
sinon.spy(component, 'render');
4954
props.shouldAutoHideContainer = true;
5055
ReactDOM.render(<AutoHide {...props} />, container);
56+
innerContainer = container.firstElementChild;
5157
});
5258

5359
it('hides the container', () => {
54-
expect(container.style.display).toEqual('none');
60+
expect(innerContainer.style.display).toEqual('none');
5561
});
5662

57-
it('does not call component.render()', () => {
58-
expect(component.render.called).toBe(false);
63+
it('call component.render()', () => {
64+
expect(component.render.called).toBe(true);
5965
});
6066

6167
context('when set back to false', () => {
@@ -65,11 +71,11 @@ describe('autoHideContainer', () => {
6571
});
6672

6773
it('shows the container', () => {
68-
expect(container.style.display).toNotEqual('none');
74+
expect(innerContainer.style.display).toNotEqual('none');
6975
});
7076

7177
it('calls component.render()', () => {
72-
expect(component.render.calledOnce).toBe(true);
78+
expect(component.render.calledTwice).toBe(true);
7379
});
7480
});
7581
});
Lines changed: 11 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,17 @@
1-
/* eslint-disable react/no-find-dom-node */
1+
import React, {Component, PropTypes} from 'react';
22

3-
import React from 'react';
4-
import ReactDOM from 'react-dom';
5-
6-
function autoHideContainer(ComposedComponent) {
7-
class AutoHide extends React.Component {
8-
componentDidMount() {
9-
this._hideOrShowContainer(this.props);
10-
}
11-
12-
componentWillReceiveProps(nextProps) {
13-
if (this.props.shouldAutoHideContainer === nextProps.shouldAutoHideContainer) {
14-
return;
15-
}
16-
17-
this._hideOrShowContainer(nextProps);
18-
}
19-
20-
shouldComponentUpdate(nextProps) {
21-
return nextProps.shouldAutoHideContainer === false;
22-
}
23-
24-
_hideOrShowContainer(props) {
25-
const container = ReactDOM.findDOMNode(this).parentNode;
26-
container.style.display = props.shouldAutoHideContainer === true ? 'none' : '';
27-
}
3+
export default function(ComposedComponent) {
4+
return class AutoHide extends Component {
5+
static displayName = `${ComposedComponent.name}-AutoHide`
6+
static propTypes = {shouldAutoHideContainer: PropTypes.bool.isRequired}
287

298
render() {
30-
return <ComposedComponent {...this.props} />;
9+
const {shouldAutoHideContainer} = this.props;
10+
return (
11+
<div style={{display: shouldAutoHideContainer ? 'none' : ''}}>
12+
<ComposedComponent {...this.props} />
13+
</div>
14+
);
3115
}
32-
}
33-
34-
AutoHide.propTypes = {
35-
shouldAutoHideContainer: React.PropTypes.bool.isRequired,
3616
};
37-
38-
// precise displayName for ease of debugging (react dev tool, react warnings)
39-
AutoHide.displayName = `${ComposedComponent.name}-AutoHide`;
40-
41-
return AutoHide;
4217
}
43-
44-
export default autoHideContainer;

0 commit comments

Comments
 (0)