Skip to content

Commit

Permalink
fix(autoHideContainer): dont prevent render with shouldComponentUpdate
Browse files Browse the repository at this point in the history
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
  • Loading branch information
iam4x committed Mar 28, 2017
1 parent 0db6caf commit 8c4b13f
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 45 deletions.
21 changes: 15 additions & 6 deletions src/decorators/__tests__/autoHideContainer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,21 @@ describe('autoHideContainer', () => {
const AutoHide = autoHideContainer(TestComponent);
renderer.render(<AutoHide shouldAutoHideContainer {...props} />);
const out = renderer.getRenderOutput();
expect(out).toEqualJSX(<TestComponent shouldAutoHideContainer hello="son" />);

expect(out).toEqualJSX(
<div style={{display: 'none'}}>
<TestComponent
hello="son"
shouldAutoHideContainer />
</div>
);
});

describe('props.shouldAutoHideContainer', () => {
let AutoHide;
let component;
let container;
let innerContainer;

beforeEach(() => {
AutoHide = autoHideContainer(TestComponent);
Expand All @@ -55,14 +63,15 @@ describe('autoHideContainer', () => {
sinon.spy(component, 'render');
props.shouldAutoHideContainer = true;
ReactDOM.render(<AutoHide {...props} />, container);
innerContainer = container.firstElementChild;
});

it('hides the container', () => {
expect(container.style.display).toEqual('none');
expect(innerContainer.style.display).toEqual('none');
});

it('does not call component.render()', () => {
expect(component.render.called).toBe(false);
it('call component.render()', () => {
expect(component.render.called).toBe(true);
});

describe('when set back to false', () => {
Expand All @@ -72,11 +81,11 @@ describe('autoHideContainer', () => {
});

it('shows the container', () => {
expect(container.style.display).toNotEqual('none');
expect(innerContainer.style.display).toNotEqual('none');
});

it('calls component.render()', () => {
expect(component.render.calledOnce).toBe(true);
expect(component.render.calledTwice).toBe(true);
});
});
});
Expand Down
50 changes: 11 additions & 39 deletions src/decorators/autoHideContainer.js
Original file line number Diff line number Diff line change
@@ -1,45 +1,17 @@
/* eslint-disable react/no-find-dom-node */
import React, {Component, PropTypes} from 'react';

import React from 'react';
import ReactDOM from 'react-dom';

function autoHideContainer(ComposedComponent) {
class AutoHide extends React.Component {
componentDidMount() {
this._hideOrShowContainer(this.props);
}

componentWillReceiveProps(nextProps) {
if (this.props.shouldAutoHideContainer === nextProps.shouldAutoHideContainer) {
return;
}

this._hideOrShowContainer(nextProps);
}

shouldComponentUpdate(nextProps) {
return nextProps.shouldAutoHideContainer === false;
}

_hideOrShowContainer(props) {
const container = ReactDOM.findDOMNode(this).parentNode;
container.style.display = props.shouldAutoHideContainer === true ? 'none' : '';
}
export default function(ComposedComponent) {
return class AutoHide extends Component {
static displayName = `${ComposedComponent.name}-AutoHide`
static propTypes = {shouldAutoHideContainer: PropTypes.bool.isRequired}

render() {
return <ComposedComponent {...this.props} />;
const {shouldAutoHideContainer} = this.props;
return (
<div style={{display: shouldAutoHideContainer ? 'none' : ''}}>
<ComposedComponent {...this.props} />
</div>
);
}
}

AutoHide.propTypes = {
shouldAutoHideContainer: React.PropTypes.bool.isRequired,
...ComposedComponent.propTypes,
};

// precise displayName for ease of debugging (react dev tool, react warnings)
AutoHide.displayName = `${ComposedComponent.name}-AutoHide`;

return AutoHide;
}

export default autoHideContainer;

0 comments on commit 8c4b13f

Please sign in to comment.