Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Dialog): prevent onClose event from firing in StrictMode when opening a new dialog #4525

Merged
merged 1 commit into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading