Skip to content

Commit 87c6c4e

Browse files
SteveByerlyTheSharpieOne
SteveByerly
authored and
TheSharpieOne
committed
feat(Modal): onOpened and onClosed callbacks for modals (#434)
add onOpened and onClosed callbacks for modals fix logic for where new props are called. re-label methods to align with purpose Closes #306 Breaks Modal onEnter and onExit props are not longer based on when the modal is opened and closed, they are based on when the component mounts and unmounts. onOpened and onClosed are now used for when the modal opens and closes.
1 parent 9c6154c commit 87c6c4e

3 files changed

Lines changed: 117 additions & 61 deletions

File tree

‎docs/lib/Components/ModalsPage.js‎

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -45,43 +45,51 @@ export default class ModalsPage extends React.Component {
4545
<pre>
4646
<PrismCode className="language-jsx">
4747
{`Modal.propTypes = {
48-
isOpen: PropTypes.bool,
4948
// boolean to control the state of the popover
50-
toggle: PropTypes.func,
51-
// callback for toggling isOpen in the controlling component
49+
isOpen: PropTypes.bool,
50+
autoFocus: PropTypes.bool,
5251
size: PropTypes.string,
52+
// callback for toggling isOpen in the controlling component
53+
toggle: PropTypes.func,
54+
keyboard: PropTypes.bool,
5355
// control backdrop, see http://v4-alpha.getbootstrap.com/components/modal/#options
5456
backdrop: PropTypes.oneOfType([
5557
PropTypes.bool,
5658
PropTypes.oneOf(['static'])
5759
]),
58-
keyboard: PropTypes.bool,
59-
// zIndex defaults to 1000.
60-
zIndex: PropTypes.oneOfType([
61-
PropTypes.number,
62-
PropTypes.string,
63-
]),
60+
// called on componentDidMount
61+
onEnter: PropTypes.func,
62+
// called on componentWillUnmount
63+
onExit: PropTypes.func,
64+
onOpened: PropTypes.func,
65+
onClosed: PropTypes.func,
6466
className: PropTypes.string,
6567
wrapClassName: PropTypes.string,
6668
modalClassName: PropTypes.string,
6769
backdropClassName: PropTypes.string,
6870
contentClassName: PropTypes.string,
6971
// boolean to control whether the fade transition occurs (default: true)
7072
fade: PropTypes.bool,
71-
// modalTransitionTimeout - controls appear, enter, and leave (default: 300)
72-
// If you need different values for appear v. enter v. leave, use the more
73-
// specific props like modalTransitionAppearTimeout.
74-
modalTransitionTimeout: PropTypes.number,
75-
modalTransitionAppearTimeout: PropTypes.number,
76-
modalTransitionEnterTimeout: PropTypes.number,
77-
modalTransitionLeaveTimeout: PropTypes.number,
73+
cssModule: PropTypes.object,
74+
// zIndex defaults to 1000.
75+
zIndex: PropTypes.oneOfType([
76+
PropTypes.number,
77+
PropTypes.string,
78+
]),
7879
// backdropTransitionTimeout - controls appear, enter, and leave (default: 150)
7980
// If you need different values for appear v. enter v. leave, use the more
8081
// specific props like backdropTransitionAppearTimeout.
8182
backdropTransitionTimeout: PropTypes.number
8283
backdropTransitionAppearTimeout: PropTypes.number,
8384
backdropTransitionEnterTimeout: PropTypes.number,
8485
backdropTransitionLeaveTimeout: PropTypes.number,
86+
// modalTransitionTimeout - controls appear, enter, and leave (default: 300)
87+
// If you need different values for appear v. enter v. leave, use the more
88+
// specific props like modalTransitionAppearTimeout.
89+
modalTransitionTimeout: PropTypes.number,
90+
modalTransitionAppearTimeout: PropTypes.number,
91+
modalTransitionEnterTimeout: PropTypes.number,
92+
modalTransitionLeaveTimeout: PropTypes.number,
8593
}`}
8694
</PrismCode>
8795
</pre>

‎src/Modal.js‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ const propTypes = {
2424
]),
2525
onEnter: PropTypes.func,
2626
onExit: PropTypes.func,
27+
onOpened: PropTypes.func,
28+
onClosed: PropTypes.func,
2729
children: PropTypes.node,
2830
className: PropTypes.string,
2931
wrapClassName: PropTypes.string,
@@ -69,14 +71,17 @@ class Modal extends React.Component {
6971
this.handleBackdropClick = this.handleBackdropClick.bind(this);
7072
this.handleEscape = this.handleEscape.bind(this);
7173
this.destroy = this.destroy.bind(this);
72-
this.onEnter = this.onEnter.bind(this);
73-
this.onExit = this.onExit.bind(this);
74+
this.onOpened = this.onOpened.bind(this);
75+
this.onClosed = this.onClosed.bind(this);
7476
}
7577

7678
componentDidMount() {
7779
if (this.props.isOpen) {
7880
this.togglePortal();
7981
}
82+
if (this.props.onEnter) {
83+
this.props.onEnter();
84+
}
8085
}
8186

8287
componentDidUpdate(prevProps) {
@@ -90,19 +95,22 @@ class Modal extends React.Component {
9095
}
9196

9297
componentWillUnmount() {
93-
this.onExit();
98+
this.destroy();
99+
if (this.props.onExit) {
100+
this.props.onExit();
101+
}
94102
}
95103

96-
onEnter() {
97-
if (this.props.onEnter) {
98-
this.props.onEnter();
104+
onOpened() {
105+
if (this.props.onOpened) {
106+
this.props.onOpened();
99107
}
100108
}
101109

102-
onExit() {
110+
onClosed() {
103111
this.destroy();
104-
if (this.props.onExit) {
105-
this.props.onExit();
112+
if (this.props.onClosed) {
113+
this.props.onClosed();
106114
}
107115
}
108116

@@ -137,12 +145,12 @@ class Modal extends React.Component {
137145
}
138146
this.show();
139147
if (!this.hasTransition()) {
140-
this.onEnter();
148+
this.onOpened();
141149
}
142150
} else {
143151
this.hide();
144152
if (!this.hasTransition()) {
145-
this.onExit();
153+
this.onClosed();
146154
}
147155
}
148156
}
@@ -247,8 +255,8 @@ class Modal extends React.Component {
247255
{isOpen && (
248256
<Fade
249257
key="modal-dialog"
250-
onEnter={this.onEnter}
251-
onLeave={this.onExit}
258+
onEnter={this.onOpened}
259+
onLeave={this.onClosed}
252260
transitionAppearTimeout={
253261
typeof this.props.modalTransitionAppearTimeout === 'number'
254262
? this.props.modalTransitionAppearTimeout

‎src/__tests__/Modal.spec.js‎

Lines changed: 72 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -277,23 +277,23 @@ describe('Modal', () => {
277277
wrapper.unmount();
278278
});
279279

280-
it('should call onExit & onEnter', () => {
281-
spyOn(Modal.prototype, 'onEnter').and.callThrough();
282-
spyOn(Modal.prototype, 'onExit').and.callThrough();
283-
const onEnter = jasmine.createSpy('spy');
284-
const onExit = jasmine.createSpy('spy');
280+
it('should call onClosed & onOpened', () => {
281+
spyOn(Modal.prototype, 'onOpened').and.callThrough();
282+
spyOn(Modal.prototype, 'onClosed').and.callThrough();
283+
const onOpened = jasmine.createSpy('spy');
284+
const onClosed = jasmine.createSpy('spy');
285285
const wrapper = mount(
286-
<Modal isOpen={isOpen} onEnter={onEnter} onExit={onExit} toggle={toggle}>
286+
<Modal isOpen={isOpen} onOpened={onOpened} onClosed={onClosed} toggle={toggle}>
287287
Yo!
288288
</Modal>
289289
);
290290

291291
jasmine.clock().tick(300);
292292
expect(isOpen).toBe(false);
293-
expect(onEnter).not.toHaveBeenCalled();
294-
expect(Modal.prototype.onEnter).not.toHaveBeenCalled();
295-
expect(onExit).not.toHaveBeenCalled();
296-
expect(Modal.prototype.onExit).not.toHaveBeenCalled();
293+
expect(onOpened).not.toHaveBeenCalled();
294+
expect(Modal.prototype.onOpened).not.toHaveBeenCalled();
295+
expect(onClosed).not.toHaveBeenCalled();
296+
expect(Modal.prototype.onClosed).not.toHaveBeenCalled();
297297

298298
toggle();
299299
wrapper.setProps({
@@ -302,10 +302,10 @@ describe('Modal', () => {
302302
jasmine.clock().tick(300);
303303

304304
expect(isOpen).toBe(true);
305-
expect(onEnter).toHaveBeenCalled();
306-
expect(Modal.prototype.onEnter).toHaveBeenCalled();
307-
expect(onExit).not.toHaveBeenCalled();
308-
expect(Modal.prototype.onExit).not.toHaveBeenCalled();
305+
expect(onOpened).toHaveBeenCalled();
306+
expect(Modal.prototype.onOpened).toHaveBeenCalled();
307+
expect(onClosed).not.toHaveBeenCalled();
308+
expect(Modal.prototype.onClosed).not.toHaveBeenCalled();
309309

310310
toggle();
311311
wrapper.setProps({
@@ -314,29 +314,29 @@ describe('Modal', () => {
314314
jasmine.clock().tick(300);
315315

316316
expect(isOpen).toBe(false);
317-
expect(onExit).toHaveBeenCalled();
318-
expect(Modal.prototype.onExit).toHaveBeenCalled();
317+
expect(onClosed).toHaveBeenCalled();
318+
expect(Modal.prototype.onClosed).toHaveBeenCalled();
319319

320320
wrapper.unmount();
321321
});
322322

323-
it('should call onExit & onEnter when fade={false}', () => {
324-
spyOn(Modal.prototype, 'onEnter').and.callThrough();
325-
spyOn(Modal.prototype, 'onExit').and.callThrough();
326-
const onEnter = jasmine.createSpy('spy');
327-
const onExit = jasmine.createSpy('spy');
323+
it('should call onClosed & onOpened when fade={false}', () => {
324+
spyOn(Modal.prototype, 'onOpened').and.callThrough();
325+
spyOn(Modal.prototype, 'onClosed').and.callThrough();
326+
const onOpened = jasmine.createSpy('spy');
327+
const onClosed = jasmine.createSpy('spy');
328328
const wrapper = mount(
329-
<Modal isOpen={isOpen} onEnter={onEnter} onExit={onExit} toggle={toggle} fade={false}>
329+
<Modal isOpen={isOpen} onOpened={onOpened} onClosed={onClosed} toggle={toggle} fade={false}>
330330
Yo!
331331
</Modal>
332332
);
333333

334334
jasmine.clock().tick(1);
335335
expect(isOpen).toBe(false);
336-
expect(onEnter).not.toHaveBeenCalled();
337-
expect(Modal.prototype.onEnter).not.toHaveBeenCalled();
338-
expect(onExit).not.toHaveBeenCalled();
339-
expect(Modal.prototype.onExit).not.toHaveBeenCalled();
336+
expect(onOpened).not.toHaveBeenCalled();
337+
expect(Modal.prototype.onOpened).not.toHaveBeenCalled();
338+
expect(onClosed).not.toHaveBeenCalled();
339+
expect(Modal.prototype.onClosed).not.toHaveBeenCalled();
340340

341341
toggle();
342342
wrapper.setProps({
@@ -345,10 +345,10 @@ describe('Modal', () => {
345345
jasmine.clock().tick(1);
346346

347347
expect(isOpen).toBe(true);
348-
expect(onEnter).toHaveBeenCalled();
349-
expect(Modal.prototype.onEnter).toHaveBeenCalled();
350-
expect(onExit).not.toHaveBeenCalled();
351-
expect(Modal.prototype.onExit).not.toHaveBeenCalled();
348+
expect(onOpened).toHaveBeenCalled();
349+
expect(Modal.prototype.onOpened).toHaveBeenCalled();
350+
expect(onClosed).not.toHaveBeenCalled();
351+
expect(Modal.prototype.onClosed).not.toHaveBeenCalled();
352352

353353
toggle();
354354
wrapper.setProps({
@@ -357,8 +357,8 @@ describe('Modal', () => {
357357
jasmine.clock().tick(1);
358358

359359
expect(isOpen).toBe(false);
360-
expect(onExit).toHaveBeenCalled();
361-
expect(Modal.prototype.onExit).toHaveBeenCalled();
360+
expect(onClosed).toHaveBeenCalled();
361+
expect(Modal.prototype.onClosed).toHaveBeenCalled();
362362

363363
wrapper.unmount();
364364
});
@@ -623,4 +623,44 @@ describe('Modal', () => {
623623

624624
wrapper.unmount();
625625
});
626+
627+
it('should call onEnter & onExit props if provided', () => {
628+
const onEnter = jasmine.createSpy('spy');
629+
const onExit = jasmine.createSpy('spy');
630+
const wrapper = mount(
631+
<Modal isOpen={isOpen} onEnter={onEnter} onExit={onExit} toggle={toggle}>
632+
Yo!
633+
</Modal>
634+
);
635+
636+
expect(isOpen).toBe(false);
637+
expect(onEnter).toHaveBeenCalled();
638+
expect(onExit).not.toHaveBeenCalled();
639+
640+
onEnter.calls.reset();
641+
onExit.calls.reset();
642+
643+
toggle();
644+
wrapper.setProps({
645+
isOpen: isOpen
646+
});
647+
jasmine.clock().tick(300);
648+
649+
expect(isOpen).toBe(true);
650+
expect(onEnter).not.toHaveBeenCalled();
651+
expect(onExit).not.toHaveBeenCalled();
652+
653+
onEnter.calls.reset();
654+
onExit.calls.reset();
655+
656+
toggle();
657+
wrapper.setProps({
658+
isOpen: isOpen
659+
});
660+
jasmine.clock().tick(300);
661+
662+
wrapper.unmount();
663+
expect(onEnter).not.toHaveBeenCalled();
664+
expect(onExit).toHaveBeenCalled();
665+
});
626666
});

0 commit comments

Comments
 (0)