Skip to content

Commit

Permalink
fix(Dialog): fix Modal backdrop / overlay false-positive click issue …
Browse files Browse the repository at this point in the history
…while e.g. selecting (#1463)
  • Loading branch information
tujoworker authored Jun 16, 2022
1 parent de17f4d commit 91e69a8
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,7 @@ exports[`Dialog component snapshot should match component snapshot 1`] = `
className="dnb-modal__content dnb-modal__content--auto-fullscreen dnb-dialog__root dnb-modal__content--custom"
id="dnb-modal-dialog_id"
onClick={[Function]}
onMouseDown={[Function]}
role="dialog"
style={null}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,7 @@ exports[`Drawer component snapshot should match component snapshot 1`] = `
className="dnb-modal__content dnb-modal__content--auto-fullscreen dnb-modal__content--right dnb-drawer__root dnb-modal__content--custom"
id="dnb-modal-drawer_id"
onClick={[Function]}
onMouseDown={[Function]}
role="dialog"
style={null}
>
Expand Down
25 changes: 23 additions & 2 deletions packages/dnb-eufemia/src/components/modal/ModalContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export default class ModalContent extends React.PureComponent<

_contentRef: React.RefObject<HTMLElement>
_scrollRef: React.RefObject<HTMLElement>
_overlayClickRef: { current: null | HTMLElement }
_id: string
_lockTimeout: NodeJS.Timeout
_focusTimeout: NodeJS.Timeout
Expand All @@ -67,6 +68,7 @@ export default class ModalContent extends React.PureComponent<
super(props)
this._contentRef = this.props.content_ref || React.createRef()
this._scrollRef = this.props.scroll_ref || React.createRef()
this._overlayClickRef = React.createRef()

// NB: The ""._id" is used in the __modalStack as "last._id"
this._id = props.id
Expand Down Expand Up @@ -284,11 +286,29 @@ export default class ModalContent extends React.PureComponent<
}
}

onCloseClickHandler = (event) => {
onCloseClickHandler = (event: React.SyntheticEvent) => {
this.closeModalContent(event, { triggeredBy: 'button' })
}

onContentClickHandler = (event) => {
onContentMouseDownHandler = (event: React.SyntheticEvent) => {
this._overlayClickRef.current =
event.target === event.currentTarget
? (event.target as HTMLElement)
: null
}

onContentClickHandler = (event: React.SyntheticEvent) => {
/**
* Prevent false-positive Modal close,
* when e.g. selecting text inside and moving the mouse outside,
* we would still get this event fired. There we check if the current click,
* has the same target as where the click got initiated.
*/
if (this._overlayClickRef.current !== event.target) {
return // stop here
}
this._overlayClickRef.current = null

const { prevent_overlay_close } = this.props

if (!isTrue(prevent_overlay_close)) {
Expand Down Expand Up @@ -424,6 +444,7 @@ export default class ModalContent extends React.PureComponent<

content_class
),
onMouseDown: this.onContentMouseDownHandler,
onClick: this.onContentClickHandler,
}

Expand Down
47 changes: 47 additions & 0 deletions packages/dnb-eufemia/src/components/modal/__tests__/Modal.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,7 @@ describe('Modal component', () => {
expect(on_close_prevent).toHaveBeenCalledTimes(1)

// trigger the close on the overlay
Comp.find('div.dnb-modal__content').simulate('mousedown')
Comp.find('div.dnb-modal__content').simulate('click')

expect(on_close_prevent).toHaveBeenCalledTimes(2)
Expand All @@ -666,6 +667,7 @@ describe('Modal component', () => {
preventClose = false

// trigger the close on the overlay
Comp.find('div.dnb-modal__content').simulate('mousedown')
Comp.find('div.dnb-modal__content').simulate('click')

expect(Comp.exists('div.dnb-modal__content')).toBe(true)
Expand All @@ -691,13 +693,58 @@ describe('Modal component', () => {
expect(testTriggeredBy).toBe(null)

// trigger the close on the overlay
Comp.find('div.dnb-modal__content').simulate('mousedown')
Comp.find('div.dnb-modal__content').simulate('click')

expect(on_close).toHaveBeenCalledTimes(1)
expect(testTriggeredBy).toBe('overlay')
expect(Comp.exists('div.dnb-modal__content')).toBe(false)
})

it('will omit close when no mousedown was fired', () => {
const on_close = jest.fn()
const Comp = mount(<Component {...props} on_close={on_close} />)
Comp.find('button').simulate('click')

// trigger the close on the overlay
Comp.find('div.dnb-modal__content').simulate('click')

expect(on_close).toHaveBeenCalledTimes(0)
expect(Comp.exists('div.dnb-modal__content')).toBe(true)
})

it('will only close when mousedown and click DOM targets are the same', () => {
const on_close = jest.fn()
const Comp = mount(<Component {...props} on_close={on_close} />)

Comp.find('button').simulate('click')

const contentElement = Comp.find('div.dnb-modal__content')
const target = contentElement.instance()
const currentTarget = contentElement.instance()
const differentTarget = document.createElement('DIV')

// trigger the close on the overlay
contentElement.simulate('mousedown', {
target,
currentTarget,
})
contentElement.simulate('click', { target: differentTarget }) // simulate with different target

expect(on_close).toHaveBeenCalledTimes(0)
expect(Comp.exists('div.dnb-modal__content')).toBe(true)

// trigger the close on the overlay
contentElement.simulate('mousedown', {
target,
currentTarget,
})
contentElement.simulate('click', { target })

expect(on_close).toHaveBeenCalledTimes(1)
expect(Comp.exists('div.dnb-modal__content')).toBe(false)
})

it('has working open event and close event on changing the "open_state"', () => {
let testTriggeredBy = null
const on_close = jest.fn(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,7 @@ exports[`Modal component have to match snapshot 1`] = `
className="dnb-modal__content dnb-modal__content--auto-fullscreen dnb-modal__content--modal"
id="dnb-modal-modal_id"
onClick={[Function]}
onMouseDown={[Function]}
role="dialog"
style={null}
>
Expand Down

0 comments on commit 91e69a8

Please sign in to comment.