`CModal` closing if clicked element is removed from dom
ncavallo-dell opened this issue · 12 comments
Somewhere along the line CModal's click listener that evaluates if you clicked outside the model or not has changed. It used to be attached to the modal content div but now it's attached to document, meaning it runs after React event handlers.
If you have a modal where elements are removed as they're clicked, to replace with something else, the modal will close prematurely.
Please see my test case here --> stackblitz
- Windows
- Edge/Firefox
In my case,
When i click CFormCheckbox element inside CModal then visible prop is changed false and onClose function is fired.
@ncavallo-dell I'm working on it
@mrholek: I'm having the similar problem too
CModal closes even when I click the CFormCheck button, it doesn't happen if it's normal html input. but with some custome component like react-select, rc-date-picker
Check my CSB: https://codesandbox.io/p/github/codesandbox/codesandbox-template-vite-react/csb-n7wj5l/coreui-hf-modal-errs
I found the problem is in the handleClickOutside function, const isInclude = modalContentRef.current.contains(event.target as HTMLElement) returns false when i click CFormCheck radio
But I don't know how to fix it. By the way my project is using useOnClickOutside of usehooks-ts so I did the temparory solution:
- Clone CModal.tsx into new component
- Remove click listenr
useEffect(() => {
- document.addEventListener('click', handleClickOutside);
document.addEventListener('keydown', handleKeyDown);
return() => {
- document.removeEventListener('click', handleClickOutside);
document.removeEventListener('keydown', handleKeyDown);
};
}, [_visible]);
- Then use
+ import { useOnClickOutside } from 'usehooks-ts';
+ useOnClickOutside(modalContentRef, handleDismiss);I hope you solve this problem soon💥
@nhahv @nhahv thank you for your solution, but I that easier will be change document.addEventListener('click', handleClickOutside) to document.addEventListener('mousedown', handleClickOutside) because the logic of handleClickOutside and useOnClickOutside is the same.
I need to investigate why CFormCheck causes this issue when normal input works well.
@mrholek Same problem here. We are using Core UI React Pro and the problem occurred when updating from @coreui/react-pro@4.6.0 to 4.7.0. (and still exists in 4.12.4). Problem appears when clicking on a select option from react-select wich uses a portal.
Looks like the bug was introduced in 2b0b70d (CModal.tsxL132-L137).
Forking CModal.tsx and changing this:
useEffect(() => {
document.addEventListener('click', handleClickOutside)
document.addEventListener('keydown', handleKeyDown)
return() => {
document.removeEventListener('click', handleClickOutside)
document.removeEventListener('keydown', handleKeyDown)
};
}, [_visible]);
back to this:
useEffect(() => {
modalRef.current && modalRef.current.addEventListener('click', handleClickOutside)
modalRef.current && modalRef.current.addEventListener('keydown', handleKeyDown)
return() => {
modalRef.current && modalRef.current.removeEventListener('click', handleClickOutside)
modalRef.current && modalRef.current.removeEventListener('keydown', handleKeyDown)
};
}, [_visible]);
seems to solve it for now (I also had to fork CModalHeader.tsx for it to import the correct CModalContext from my forked CModal component). Will it cause any problems that I don't see yet?
Here is a simple stackblitz with a custom portal to simulate this behavior.
@danielhoegel I will check this, but not until the beginning of next week.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions
@mrholek this bug has bitten us really badly, could we get some eyes on it?
FYI to fix this, we added this component to our component library.
The only change is this function:
const handleClickOutside = (event: Event) => {
// modalRef.current is the modal backdrop. If we detect a click on that, then dismiss the modal.
// See https://github.com/coreui/coreui-react/issues/361
if (modalContentRef.current && modalRef.current == event.target) {
handleDismiss()
}
}Full component for copy-paste convenience:
import classNames from 'classnames'
import React, { HTMLAttributes, createContext, forwardRef, useEffect, useLayoutEffect, useRef, useState } from 'react'
import { Transition } from 'react-transition-group'
import { CBackdrop, CConditionalPortal, CModalContent, CModalDialog, useForkedRef } from '@coreui/react-pro'
export interface ShimmedCModalProps extends HTMLAttributes<HTMLDivElement> {
/**
* Align the modal in the center or top of the screen.
*/
alignment?: 'top' | 'center'
/**
* Apply a backdrop on body while modal is open.
*/
backdrop?: boolean | 'static'
/**
* A string of all className you want applied to the base component.
*/
className?: string
/**
* @ignore
*/
duration?: number
/**
* Puts the focus on the modal when shown.
*
* @since v4.10.0
*/
focus?: boolean
/**
* Set modal to covers the entire user viewport.
*/
fullscreen?: boolean | 'sm' | 'md' | 'lg' | 'xl' | 'xxl'
/**
* Closes the modal when escape key is pressed.
*/
keyboard?: boolean
/**
* Callback fired when the component requests to be closed.
*/
onClose?: () => void
/**
* Callback fired when the component requests to be closed.
*/
onClosePrevented?: () => void
/**
* Callback fired when the modal is shown, its backdrop is static and a click outside the modal or an escape key press is performed with the keyboard option set to false.
*/
onShow?: () => void
/**
* Generates modal using createPortal.
*/
portal?: boolean
/**
* Create a scrollable modal that allows scrolling the modal body.
*/
scrollable?: boolean
/**
* Size the component small, large, or extra large.
*/
size?: 'sm' | 'lg' | 'xl'
/**
* Remove animation to create modal that simply appear rather than fade in to view.
*/
transition?: boolean
/**
* By default the component is unmounted after close animation, if you want to keep the component mounted set this property to false.
*/
unmountOnClose?: boolean
/**
* Toggle the visibility of modal component.
*/
visible?: boolean
}
interface ModalContextProps {
visible?: boolean
setVisible: React.Dispatch<React.SetStateAction<boolean | undefined>>
}
export const CModalContext = createContext({} as ModalContextProps)
export const ShimmedCModal = forwardRef<HTMLDivElement, ShimmedCModalProps>(
(
{
children,
alignment,
backdrop = true,
className,
duration = 150,
focus = true,
fullscreen,
keyboard = true,
onClose,
onClosePrevented,
onShow,
portal = true,
scrollable,
size,
transition = true,
unmountOnClose = true,
visible,
...rest
},
ref
) => {
const activeElementRef = useRef<HTMLElement | null>(null)
const modalRef = useRef<HTMLDivElement>(null)
const modalContentRef = useRef<HTMLDivElement>(null)
const forkedRef = useForkedRef(ref, modalRef)
const [_visible, setVisible] = useState(visible)
const [staticBackdrop, setStaticBackdrop] = useState(false)
const contextValues = {
visible: _visible,
setVisible,
}
useEffect(() => {
setVisible(visible)
}, [visible])
useEffect(() => {
if (_visible) {
activeElementRef.current = document.activeElement as HTMLElement | null
document.addEventListener('mouseup', handleClickOutside)
document.addEventListener('keydown', handleKeyDown)
} else {
activeElementRef.current?.focus()
}
return () => {
document.removeEventListener('mouseup', handleClickOutside)
document.removeEventListener('keydown', handleKeyDown)
}
}, [_visible])
const handleDismiss = () => {
if (backdrop === 'static') {
return setStaticBackdrop(true)
}
setVisible(false)
// biome-ignore lint/complexity/useOptionalChain: copy-paste from CoreUI-react
return onClose && onClose()
}
useLayoutEffect(() => {
// biome-ignore lint/complexity/useOptionalChain: copy-paste from CoreUI-react
onClosePrevented && onClosePrevented()
setTimeout(() => setStaticBackdrop(false), duration)
}, [staticBackdrop])
// Set focus to modal after open
useLayoutEffect(() => {
if (_visible) {
document.body.classList.add('modal-open')
if (backdrop) {
document.body.style.overflow = 'hidden'
document.body.style.paddingRight = '0px'
}
setTimeout(
() => {
focus && modalRef.current?.focus()
},
transition ? duration : 0
)
} else {
document.body.classList.remove('modal-open')
if (backdrop) {
document.body.style.removeProperty('overflow')
document.body.style.removeProperty('padding-right')
}
}
return () => {
document.body.classList.remove('modal-open')
if (backdrop) {
document.body.style.removeProperty('overflow')
document.body.style.removeProperty('padding-right')
}
}
}, [_visible])
const handleClickOutside = (event: Event) => {
// modalRef.current is the modal backdrop. If we detect a click on that, then dismiss the modal.
// See https://github.com/coreui/coreui-react/issues/361
if (modalContentRef.current && modalRef.current == event.target) {
handleDismiss()
}
}
const handleKeyDown = (event: KeyboardEvent) => {
if (event.key === 'Escape' && keyboard) {
handleDismiss()
}
}
return (
<>
<Transition
in={_visible}
mountOnEnter
nodeRef={modalRef}
onEnter={onShow}
onExit={onClose}
unmountOnExit={unmountOnClose}
timeout={transition ? duration : 0}
>
{(state) => (
<CConditionalPortal portal={portal}>
<CModalContext.Provider value={contextValues}>
<div
className={classNames(
'modal',
{
'modal-static': staticBackdrop,
fade: transition,
show: state === 'entered',
},
className
)}
tabIndex={-1}
{...(_visible ? { 'aria-modal': true, role: 'dialog' } : { 'aria-hidden': 'true' })}
style={{
...(state !== 'exited' && { display: 'block' }),
}}
{...rest}
ref={forkedRef}
>
<CModalDialog alignment={alignment} fullscreen={fullscreen} scrollable={scrollable} size={size}>
<CModalContent ref={modalContentRef}>{children}</CModalContent>
</CModalDialog>
</div>
</CModalContext.Provider>
</CConditionalPortal>
)}
</Transition>
{backdrop && (
<CConditionalPortal portal={portal}>
<CBackdrop visible={_visible} />
</CConditionalPortal>
)}
</>
)
}
)@aidanlister thank you for your help. I'm testing your solution.
I just released a new version, please check it.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions