Skip to content

Commit

Permalink
fix(Dialog): prevent onClose event from firing in StrictMode when o…
Browse files Browse the repository at this point in the history
…pening a new dialog (#4525)

- Also add `handler` to `triggeredBy` when the `close` method is called.
- Improve the documentation of the `triggeredBy` property.
- More
[info](https://dnb-it.slack.com/archives/CMXABCHEY/p1738574258572699?thread_ts=1738072824.587899&cid=CMXABCHEY)
about the approach.
- Thank you Ahmet for the [good
reprod](https://codesandbox.io/p/sandbox/connectwithpath-forked-h2wr7p).
🫶
  • Loading branch information
tujoworker authored Feb 4, 2025
1 parent 1a92512 commit d24c0d3
Show file tree
Hide file tree
Showing 8 changed files with 327 additions and 66 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
| Events | Description |
| ------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| `onOpen` / `on_open` | _(optional)_ This event gets triggered once the modal shows up. Returns the modal id: `{ id }`. |
| `onClose` / `on_close` | _(optional)_ this event gets triggered once the modal gets closed. Returns the modal id: `{ id, event, triggeredBy }`. |
| `onClose` / `on_close` | _(optional)_ this event gets triggered once the modal gets closed. Returns the modal id: `{ id, event, triggeredBy }`. More info about the `triggeredBy` down below. |
| `onClosePrevent` / `on_close_prevent` | _(optional)_ this event gets triggered once the user tries to close the modal, but `prevent_close` is set to **true**. Returns a callback `close` you can call to trigger the close mechanism. More details below. Returns the modal id: `{ id, event, close: Method, triggeredBy }` |

## `triggeredBy`

The `triggeredBy` property is given when the `onClose` or the `onClosePrevent` event is triggered. It can contain one of the following values:

- `button`: The close button that triggered the event.
- `handler`: The `close` handler given by the function (as the content/children).
- `keyboard`: The escape key that triggered the event.
- `overlay`: The overlay element that triggered the event.
- `unmount`: The unmount event that triggered the `openState` prop change.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,23 @@ beforeEach(() => {
window.__modalStack = undefined
})

const log = global.console.log
beforeEach(() => {
global.console.log = jest.fn((...args) => {
if (
!String(args[1]).includes(
'A Dialog or Drawer needs a h1 as its first element!'
)
) {
log(...args)
}
})
})
afterEach(() => {
global.console.log = log
jest.resetAllMocks()
})

describe('Dialog', () => {
it('will run bodyScrollLock with disableBodyScroll', () => {
render(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,23 @@ beforeEach(() => {
window.__modalStack = undefined
})

const log = global.console.log
beforeEach(() => {
global.console.log = jest.fn((...args) => {
if (
!String(args[1]).includes(
'A Dialog or Drawer needs a h1 as its first element!'
)
) {
log(...args)
}
})
})
afterEach(() => {
global.console.log = log
jest.resetAllMocks()
})

describe('Drawer', () => {
it('will run bodyScrollLock with disableBodyScroll', () => {
render(
Expand Down
10 changes: 8 additions & 2 deletions packages/dnb-eufemia/src/components/modal/Modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ class Modal extends React.PureComponent<
_tryToOpenTimeout: NodeJS.Timeout
activeElement: Element
isInTransition: boolean
modalContentCloseRef: React.RefObject<any>

state = {
hide: false,
Expand Down Expand Up @@ -170,6 +171,7 @@ class Modal extends React.PureComponent<
this._id = props.id || makeUniqueId('modal-')

this._triggerRef = React.createRef()
this.modalContentCloseRef = React.createRef()

this._onUnmount = []
}
Expand Down Expand Up @@ -350,8 +352,12 @@ class Modal extends React.PureComponent<

close = (
event: Event,
{ ifIsLatest, triggeredBy = null } = { ifIsLatest: true }
{ ifIsLatest, triggeredBy = 'handler' } = {
ifIsLatest: true,
}
) => {
this.modalContentCloseRef.current?.(event, { triggeredBy })

const { prevent_close = false } = this.props

if (isTrue(prevent_close)) {
Expand Down Expand Up @@ -448,7 +454,6 @@ class Modal extends React.PureComponent<
vertical_alignment = 'center',

id, // eslint-disable-line
open_state, // eslint-disable-line
open_delay, // eslint-disable-line

omit_trigger_button = false,
Expand Down Expand Up @@ -534,6 +539,7 @@ class Modal extends React.PureComponent<
close={this.close}
hide={hide}
title={rest.title || fallbackTitle}
modalContentCloseRef={this.modalContentCloseRef}
/>
)}
</>
Expand Down
81 changes: 64 additions & 17 deletions packages/dnb-eufemia/src/components/modal/ModalContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ import {
} from '../../shared/component-helper'
import ModalContext from './ModalContext'
import { IS_IOS, IS_SAFARI, IS_MAC, isAndroid } from '../../shared/helpers'
import { ModalContentProps } from './types'
import {
CloseHandlerParams,
ModalContentProps,
TriggeredBy,
} from './types'
import {
getListOfModalRoots,
getModalRoot,
Expand All @@ -34,8 +38,6 @@ import { Context } from '../../shared'
import { ContextProps } from '../../shared/Context'

interface ModalContentState {
triggeredBy: string
triggeredByEvent: Event
color: string
}

Expand All @@ -53,7 +55,7 @@ export default class ModalContent extends React.PureComponent<
ModalContentProps,
ModalContentState
> {
state = { triggeredBy: null, triggeredByEvent: null, color: null }
state = { color: null }

_contentRef: React.RefObject<HTMLElement>
_scrollRef: React.RefObject<HTMLElement>
Expand All @@ -64,6 +66,9 @@ export default class ModalContent extends React.PureComponent<
_androidFocusTimeout: NodeJS.Timeout
_ii: InteractionInvalidation
_iiLocal: InteractionInvalidation
_triggeredBy: TriggeredBy
_triggeredByEvent: React.SyntheticEvent
_isControlled = false

static contextType = Context

Expand All @@ -74,6 +79,9 @@ export default class ModalContent extends React.PureComponent<
this._contentRef = this.props.content_ref || React.createRef()
this._scrollRef = this.props.scroll_ref || React.createRef()
this._overlayClickRef = React.createRef()
if (this.props.modalContentCloseRef) {
this.props.modalContentCloseRef.current = this.setModalContentState
}

// NB: The ""._id" is used in the __modalStack as "last._id"
this._id = props.id
Expand Down Expand Up @@ -111,6 +119,8 @@ export default class ModalContent extends React.PureComponent<
} else {
this._lockTimeout = setTimeout(this.lockBody, timeoutDuration * 1.2) // a little over --modal-animation-duration
}

this.setIsControlled()
}

componentWillUnmount() {
Expand All @@ -119,6 +129,29 @@ export default class ModalContent extends React.PureComponent<
this.removeLocks()
}

setIsControlled() {
const { open_state } = this.props
if (typeof open_state !== 'undefined' && open_state !== null) {
this._isControlled = true
}
}

componentDidUpdate() {
this.setIsControlled()
}

wasOpenedManually() {
if (this._triggeredBy) {
return true
}

if (this._isControlled) {
return true
}

return false
}

lockBody = () => {
const modalRoots = getListOfModalRoots()
const firstLevel = modalRoots[0]
Expand Down Expand Up @@ -182,13 +215,14 @@ export default class ModalContent extends React.PureComponent<

this.removeAndroidFocusHelper()

const id = this.props.id
const { triggeredBy, triggeredByEvent } = this.state
dispatchCustomElementEvent(this, 'on_close', {
id,
event: triggeredByEvent,
triggeredBy: triggeredBy || 'unmount',
})
if (this.wasOpenedManually()) {
const id = this.props.id
dispatchCustomElementEvent(this, 'on_close', {
id,
event: this._triggeredByEvent,
triggeredBy: this._triggeredBy || 'unmount',
})
}

if (typeof document !== 'undefined') {
document.removeEventListener('keydown', this.onKeyDownHandler)
Expand Down Expand Up @@ -349,13 +383,26 @@ export default class ModalContent extends React.PureComponent<
}
}

closeModalContent(event, { triggeredBy, ...params }) {
setModalContentState = (
event: React.SyntheticEvent,
{ triggeredBy }: CloseHandlerParams
) => {
this._triggeredBy = triggeredBy
this._triggeredByEvent = event
}

closeModalContent(
event,
{
triggeredBy,
...params
}: CloseHandlerParams & { ifIsLatest?: boolean }
) {
event?.persist?.()
this.setState({ triggeredBy, triggeredByEvent: event }, () => {
this.props.close(event, {
triggeredBy,
...params,
})

this.props.close(event, {
triggeredBy,
...params,
})
}

Expand Down
3 changes: 3 additions & 0 deletions packages/dnb-eufemia/src/components/modal/ModalRoot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ export interface ModalRootProps extends ModalContentProps {
* The content which will appear when triggering the modal/drawer.
*/
children?: ReactChildType

/** For internal use only */
modalContentCloseRef?: React.RefObject<any>
}

interface ModalRootState {
Expand Down
Loading

0 comments on commit d24c0d3

Please sign in to comment.