Skip to content

Commit 8f92ac8

Browse files
layershifterlayershifter
committed
fix(Modal|Portal|Popup): use proposed value for open in onOpen & onClose callbacks (#4030)
fix(Modal|Portal|Popup): use proposed value for `open` in `onOpen` & `onClose` callbacks
1 parent 5602041 commit 8f92ac8

6 files changed

Lines changed: 105 additions & 43 deletions

File tree

‎src/addons/Portal/Portal.js‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,7 @@ class Portal extends Component {
180180
open = (e) => {
181181
debug('open()')
182182

183-
const { onOpen } = this.props
184-
if (onOpen) onOpen(e, this.props)
185-
183+
_.invoke(this.props, 'onOpen', e, { ...this.props, open: true })
186184
this.setState({ open: true })
187185
}
188186

@@ -198,9 +196,7 @@ class Portal extends Component {
198196
close = (e) => {
199197
debug('close()')
200198

201-
const { onClose } = this.props
202-
if (onClose) onClose(e, this.props)
203-
199+
_.invoke(this.props, 'onClose', e, { ...this.props, open: false })
204200
this.setState({ open: false })
205201
}
206202

‎src/modules/Modal/Modal.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ class Modal extends Component {
5959
handleClose = (e) => {
6060
debug('close()')
6161

62-
_.invoke(this.props, 'onClose', e, this.props)
62+
_.invoke(this.props, 'onClose', e, { ...this.props, open: false })
6363
this.setState({ open: false })
6464
}
6565

@@ -80,7 +80,7 @@ class Modal extends Component {
8080
)
8181
return
8282

83-
_.invoke(this.props, 'onClose', e, this.props)
83+
_.invoke(this.props, 'onClose', e, { ...this.props, open: false })
8484
this.setState({ open: false })
8585
}
8686

@@ -94,7 +94,7 @@ class Modal extends Component {
9494
handleOpen = (e) => {
9595
debug('open()')
9696

97-
_.invoke(this.props, 'onOpen', e, this.props)
97+
_.invoke(this.props, 'onOpen', e, { ...this.props, open: true })
9898
this.setState({ open: true })
9999
}
100100

‎src/modules/Popup/Popup.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,12 @@ export default class Popup extends Component {
112112

113113
handleClose = (e) => {
114114
debug('handleClose()')
115-
_.invoke(this.props, 'onClose', e, this.props)
115+
_.invoke(this.props, 'onClose', e, { ...this.props, open: false })
116116
}
117117

118118
handleOpen = (e) => {
119119
debug('handleOpen()')
120-
_.invoke(this.props, 'onOpen', e, this.props)
120+
_.invoke(this.props, 'onOpen', e, { ...this.props, open: true })
121121
}
122122

123123
handlePortalMount = (e) => {

‎test/specs/addons/Portal/Portal-test.js‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,36 @@ describe('Portal', () => {
186186
})
187187
})
188188

189+
describe('onOpen', () => {
190+
it('is called on trigger click', () => {
191+
const onOpen = sandbox.spy()
192+
wrapperMount(
193+
<Portal onOpen={onOpen} trigger={<div id='trigger' />}>
194+
<p />
195+
</Portal>,
196+
)
197+
198+
wrapper.find('#trigger').simulate('click')
199+
onOpen.should.have.been.calledOnce()
200+
onOpen.should.have.been.calledWithMatch({}, { open: true })
201+
})
202+
})
203+
204+
describe('onClose', () => {
205+
it('is called on body click', () => {
206+
const onClose = sandbox.spy()
207+
wrapperMount(
208+
<Portal defaultOpen onClose={onClose} trigger={<div />}>
209+
<p />
210+
</Portal>,
211+
)
212+
213+
domEvent.click(document.body)
214+
onClose.should.have.been.called()
215+
onClose.should.have.been.calledWithMatch({}, { open: false })
216+
})
217+
})
218+
189219
describe('trigger', () => {
190220
it('renders null when not set', () => {
191221
wrapperMount(

‎test/specs/modules/Modal/Modal-test.js‎

Lines changed: 38 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -300,98 +300,104 @@ describe('Modal', () => {
300300

301301
describe('onOpen', () => {
302302
it('is called on trigger click', () => {
303-
const spy = sandbox.spy()
304-
wrapperMount(<Modal onOpen={spy} trigger={<div id='trigger' />} />)
303+
const onOpen = sandbox.spy()
304+
wrapperMount(<Modal onOpen={onOpen} trigger={<div id='trigger' />} />)
305305

306306
wrapper.find('#trigger').simulate('click')
307-
spy.should.have.been.calledOnce()
307+
onOpen.should.have.been.calledOnce()
308+
onOpen.should.have.been.calledWithMatch({}, { open: true })
308309
})
309310

310311
it('is not called on body click', () => {
311-
const spy = sandbox.spy()
312-
wrapperMount(<Modal onOpen={spy} />)
312+
const onOpen = sandbox.spy()
313+
wrapperMount(<Modal onOpen={onOpen} />)
313314

314315
domEvent.click(document.body)
315-
spy.should.not.have.been.called()
316+
onOpen.should.not.have.been.called()
316317
})
317318
})
318319

319320
describe('onClose', () => {
320-
let spy
321-
322-
beforeEach(() => {
323-
spy = sandbox.spy()
324-
})
325-
326321
it('is called on dimmer click', () => {
327-
wrapperMount(<Modal onClose={spy} defaultOpen />)
322+
const onClose = sandbox.spy()
323+
wrapperMount(<Modal onClose={onClose} defaultOpen />)
328324

329325
domEvent.click('.ui.dimmer')
330-
spy.should.have.been.calledOnce()
326+
onClose.should.have.been.calledOnce()
327+
onClose.should.have.been.calledWithMatch({}, { open: false })
331328
})
332329

333330
it('is called on click outside of the modal', () => {
334-
wrapperMount(<Modal onClose={spy} defaultOpen />)
331+
const onClose = sandbox.spy()
332+
wrapperMount(<Modal onClose={onClose} defaultOpen />)
335333

336334
domEvent.click(document.querySelector('.ui.modal').parentNode)
337-
spy.should.have.been.calledOnce()
335+
onClose.should.have.been.calledOnce()
338336
})
339337

340338
it('is not called on mousedown inside and mouseup outside of the modal', () => {
341-
wrapperMount(<Modal onClose={spy} defaultOpen />)
339+
const onClose = sandbox.spy()
340+
wrapperMount(<Modal onClose={onClose} defaultOpen />)
342341

343342
domEvent.mouseDown(document.querySelector('.ui.modal'))
344343
domEvent.click(document.querySelector('.ui.modal').parentNode)
345-
spy.should.not.have.been.called()
344+
onClose.should.not.have.been.called()
346345
})
347346

348347
it('is not called on click inside of the modal', () => {
349-
wrapperMount(<Modal onClose={spy} defaultOpen />)
348+
const onClose = sandbox.spy()
349+
wrapperMount(<Modal onClose={onClose} defaultOpen />)
350350

351351
domEvent.click(document.querySelector('.ui.modal'))
352-
spy.should.not.have.been.called()
352+
onClose.should.not.have.been.called()
353353
})
354354

355355
it('is not called on body click', () => {
356-
wrapperMount(<Modal onClose={spy} defaultOpen />)
356+
const onClose = sandbox.spy()
357+
wrapperMount(<Modal onClose={onClose} defaultOpen />)
357358

358359
domEvent.click(document.body)
359-
spy.should.not.have.been.calledOnce()
360+
onClose.should.not.have.been.calledOnce()
360361
})
361362

362363
it('is called when pressing escape', () => {
363-
wrapperMount(<Modal onClose={spy} defaultOpen />)
364+
const onClose = sandbox.spy()
365+
wrapperMount(<Modal onClose={onClose} defaultOpen />)
364366

365367
domEvent.keyDown(document, { key: 'Escape' })
366-
spy.should.have.been.calledOnce()
368+
onClose.should.have.been.calledOnce()
367369
})
368370

369371
it('is not called when the open prop changes to false', () => {
370-
wrapperMount(<Modal onClose={spy} defaultOpen />)
372+
const onClose = sandbox.spy()
373+
wrapperMount(<Modal onClose={onClose} defaultOpen />)
371374

372375
wrapper.setProps({ open: false })
373-
spy.should.not.have.been.called()
376+
onClose.should.not.have.been.called()
374377
})
375378

376379
it('is not called when open changes to false programmatically', () => {
377-
wrapperMount(<Modal onClose={spy} defaultOpen />)
380+
const onClose = sandbox.spy()
381+
wrapperMount(<Modal onClose={onClose} defaultOpen />)
378382

379383
wrapper.setProps({ open: false })
380-
spy.should.not.have.been.called()
384+
onClose.should.not.have.been.called()
381385
})
382386

383387
it('is not called on dimmer click when closeOnDimmerClick is false', () => {
384-
wrapperMount(<Modal onClose={spy} defaultOpen closeOnDimmerClick={false} />)
388+
const onClose = sandbox.spy()
389+
wrapperMount(<Modal onClose={onClose} defaultOpen closeOnDimmerClick={false} />)
385390

386391
domEvent.click('.ui.dimmer')
387-
spy.should.not.have.been.called()
392+
onClose.should.not.have.been.called()
388393
})
389394

390395
it('is not called on body click when closeOnDocumentClick is false', () => {
391-
wrapperMount(<Modal onClose={spy} defaultOpen closeOnDocumentClick={false} />)
396+
const onClose = sandbox.spy()
397+
wrapperMount(<Modal onClose={onClose} defaultOpen closeOnDocumentClick={false} />)
392398

393399
domEvent.click(document.body)
394-
spy.should.not.have.been.called()
400+
onClose.should.not.have.been.called()
395401
})
396402
})
397403

‎test/specs/modules/Popup/Popup-test.js‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,36 @@ describe('Popup', () => {
217217
})
218218
})
219219

220+
describe('onOpen', () => {
221+
it('is called on trigger click', () => {
222+
const onOpen = sandbox.spy()
223+
wrapperMount(
224+
<Popup onOpen={onOpen} trigger={<div id='trigger' />}>
225+
<p />
226+
</Popup>,
227+
)
228+
229+
wrapper.find('#trigger').simulate('click')
230+
onOpen.should.have.been.calledOnce()
231+
onOpen.should.have.been.calledWithMatch({}, { open: true })
232+
})
233+
})
234+
235+
describe('onClose', () => {
236+
it('is called on body click', () => {
237+
const onClose = sandbox.spy()
238+
wrapperMount(
239+
<Popup defaultOpen onClose={onClose} trigger={<div />}>
240+
<p />
241+
</Popup>,
242+
)
243+
244+
domEvent.click(document.body)
245+
onClose.should.have.been.called()
246+
onClose.should.have.been.calledWithMatch({}, { open: false })
247+
})
248+
})
249+
220250
describe('open', () => {
221251
it('is not open by default', () => {
222252
wrapperMount(<Popup />)

0 commit comments

Comments
 (0)