Skip to content

Commit 23a3561

Browse files
eddywashereeddywashere
authored
fix(Modal): handle prop updates (#35) (#40)
* build(enzyme): update to 2.3.0 * fix(Fade): bind onLeave & onEnter * fix(Modal): rerender on all prop changes
1 parent 843f63c commit 23a3561

4 files changed

Lines changed: 59 additions & 17 deletions

File tree

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@
6969
"coveralls": "^2.11.9",
7070
"css-loader": "^0.23.1",
7171
"ejs": "^2.4.1",
72-
"enzyme": "^2.2.0",
72+
"enzyme": "^2.3.0",
7373
"eslint": "^2.9.0",
7474
"eslint-config-airbnb": "^9.0.0",
7575
"eslint-plugin-import": "^1.7.0",

‎src/Fade.jsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ class Fade extends React.Component {
3434
this.state = {
3535
mounted: !props.transitionAppear,
3636
};
37+
38+
this.onLeave = this.onLeave.bind(this);
39+
this.onEnter = this.onEnter.bind(this);
3740
}
3841

3942
onEnter(cb) {

‎src/Modal.jsx‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class Modal extends React.Component {
2020
constructor(props) {
2121
super(props);
2222

23-
this.handleProps = this.handleProps.bind(this);
23+
this.togglePortal = this.togglePortal.bind(this);
2424
this.handleBackdropClick = this.handleBackdropClick.bind(this);
2525
this.handleEscape = this.handleEscape.bind(this);
2626
this.destroy = this.destroy.bind(this);
@@ -36,7 +36,11 @@ class Modal extends React.Component {
3636

3737
componentDidUpdate(prevProps) {
3838
if (this.props.isOpen !== prevProps.isOpen) {
39-
this.handleProps();
39+
// handle portal events/dom updates
40+
this.togglePortal();
41+
} else if (this._element) {
42+
// rerender portal
43+
this.renderIntoSubtree();
4044
}
4145
}
4246

@@ -71,16 +75,17 @@ class Modal extends React.Component {
7175
}
7276
}
7377

74-
handleProps() {
78+
togglePortal() {
7579
if (this.props.isOpen) {
7680
this.show();
81+
this._focus = true;
7782
} else {
7883
this.hide();
7984
}
8085
}
8186

8287
destroy() {
83-
let classes = document.body.className.replace('modal-open', '');
88+
const classes = document.body.className.replace('modal-open', '');
8489
this.removeEvents();
8590

8691
if (this._element) {
@@ -117,7 +122,6 @@ class Modal extends React.Component {
117122
);
118123

119124
this.renderIntoSubtree();
120-
this._element.focus();
121125
}
122126

123127
renderIntoSubtree() {
@@ -126,36 +130,44 @@ class Modal extends React.Component {
126130
this.renderChildren(),
127131
this._element
128132
);
133+
134+
// check if modal should receive focus
135+
if (this._focus) {
136+
this._element.focus();
137+
this._focus = false;
138+
}
129139
}
130140

131141
renderChildren() {
132142
return (
133143
<TransitionGroup component="div">
134-
{ this.props.isOpen && (
144+
{this.props.isOpen && (
135145
<Fade
136-
key={Math.random()}
146+
key="modal-dialog"
137147
onEnter={this.onEnter}
138148
onLeave={this.onExit}
139149
transitionAppearTimeout={300}
140150
transitionEnterTimeout={300}
141151
transitionLeaveTimeout={300}
142152
className="modal"
143153
style={{ display: 'block' }}
144-
tabIndex="-1">
154+
tabIndex="-1"
155+
>
145156
<div className="modal-dialog" role="document" ref={(c) => this._dialog = c }>
146157
<div className="modal-content">
147-
{ this.props.children }
158+
{this.props.children}
148159
</div>
149160
</div>
150161
</Fade>
151162
)}
152-
{ this.props.isOpen && (
163+
{this.props.isOpen && (
153164
<Fade
154-
key={Math.random()}
165+
key="modal-backdrop"
155166
transitionAppearTimeout={150}
156167
transitionEnterTimeout={150}
157168
transitionLeaveTimeout={150}
158-
className="modal-backdrop"/>
169+
className="modal-backdrop"
170+
/>
159171
)}
160172
</TransitionGroup>
161173
);

‎test/Modal.spec.js‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@ describe('Modal', () => {
116116
wrapper.unmount();
117117
});
118118

119-
it('should not call handleProps when isOpen does not change', () => {
120-
spyOn(Modal.prototype, 'handleProps').and.callThrough();
119+
it('should not call togglePortal when isOpen does not change', () => {
120+
spyOn(Modal.prototype, 'togglePortal').and.callThrough();
121121
spyOn(Modal.prototype, 'componentDidUpdate').and.callThrough();
122122
const wrapper = mount(
123123
<Modal isOpen={isOpen} toggle={toggle}>
@@ -127,7 +127,7 @@ describe('Modal', () => {
127127

128128
jasmine.clock().tick(300);
129129
expect(isOpen).toBe(false);
130-
expect(Modal.prototype.handleProps).not.toHaveBeenCalled();
130+
expect(Modal.prototype.togglePortal).not.toHaveBeenCalled();
131131
expect(Modal.prototype.componentDidUpdate).not.toHaveBeenCalled();
132132

133133
wrapper.setProps({
@@ -136,12 +136,39 @@ describe('Modal', () => {
136136
jasmine.clock().tick(300);
137137

138138
expect(isOpen).toBe(false);
139-
expect(Modal.prototype.handleProps).not.toHaveBeenCalled();
139+
expect(Modal.prototype.togglePortal).not.toHaveBeenCalled();
140140
expect(Modal.prototype.componentDidUpdate).toHaveBeenCalled();
141141

142142
wrapper.unmount();
143143
});
144144

145+
it('should renderIntoSubtree when props updated', () => {
146+
isOpen = true;
147+
spyOn(Modal.prototype, 'togglePortal').and.callThrough();
148+
spyOn(Modal.prototype, 'renderIntoSubtree').and.callThrough();
149+
const wrapper = mount(
150+
<Modal isOpen={isOpen} toggle={toggle}>
151+
Yo!
152+
</Modal>
153+
);
154+
155+
jasmine.clock().tick(300);
156+
expect(isOpen).toBe(true);
157+
expect(Modal.prototype.togglePortal.calls.count()).toEqual(0);
158+
expect(Modal.prototype.renderIntoSubtree.calls.count()).toEqual(1);
159+
160+
wrapper.setProps({
161+
isOpen: isOpen
162+
});
163+
jasmine.clock().tick(300);
164+
165+
expect(isOpen).toBe(true);
166+
expect(Modal.prototype.togglePortal.calls.count()).toEqual(0);
167+
expect(Modal.prototype.renderIntoSubtree.calls.count()).toEqual(2);
168+
169+
wrapper.unmount();
170+
});
171+
145172
it('should close modal when escape key pressed', () => {
146173
isOpen = true;
147174
const wrapper = mount(

0 commit comments

Comments
 (0)