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(Modal): remove unecessary stop propagation #4614

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
5 changes: 5 additions & 0 deletions .changeset/gorgeous-jokes-give.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@ultraviolet/ui": patch
---

Fix `<Modal />` stop propagation so you can close `<SelectInputV2 />` within it
Original file line number Diff line number Diff line change
@@ -1,8 +1,24 @@
import type { StoryFn } from '@storybook/react'
import { Button } from '../../Button'
import { SelectInputV2 } from '../../SelectInputV2'
import { Stack } from '../../Stack'
import { Modal } from '../index'

const OPTIONS = [
{
label: 'Option 1',
value: 'option-1',
},
{
label: 'Option 2',
value: 'option-2',
},
{
label: 'Option 3',
value: 'option-3',
},
]

export const NestedModal: StoryFn = props => (
<Modal {...props} disclosure={<Button>Open Parent Modal</Button>}>
<Stack gap={1}>
Expand Down Expand Up @@ -32,6 +48,11 @@ export const NestedModal: StoryFn = props => (
esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat
cupidatat non proident, sunt in culpa qui officia deserunt mollit
anim id est laborum.
<SelectInputV2
label="Choose an option"
name="example"
options={OPTIONS}
/>
</Modal>
</Stack>
</Modal>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import type { StoryFn } from '@storybook/react'
import { Modal } from '..'
import { Button } from '../../Button'
import { SelectInputV2 } from '../../SelectInputV2'

const OPTIONS = [
{
label: 'Option 1',
value: 'option-1',
},
{
label: 'Option 2',
value: 'option-2',
},
{
label: 'Option 3',
value: 'option-3',
},
]

export const WithSelectInput: StoryFn = props => (
<Modal disclosure={<Button>Open Modal with SelectInput</Button>} {...props}>
<div style={{ display: 'flex', flexDirection: 'column', gap: 32 }}>
<SelectInputV2
label="Choose an option"
name="example"
options={OPTIONS}
/>
</div>
</Modal>
)
WithSelectInput.parameters = {
docs: {
description: {
story:
'Having a select input inside a modal is a common use case and shows you how to modal and select input can work together.',
},
},
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export { HideOnEsc } from './HideOnEsc.stories'
export { IsClosable } from './IsClosable.stories'
export { WithDisclosureFunction } from './WithDisclosureFunction.stories'
export { WithDisclosureBeingANativeElement } from './WithDisclosureBeingANativeElement.stories'
export { WithSelectInput } from './WithSelectInput.stories'
export { FunctionChildren } from './FunctionChildren.stories'
export { WithTooltip } from './WithTooltip.stories'
export { AutoFocus } from './AutoFocus.stories'
Expand Down
15 changes: 7 additions & 8 deletions packages/ui/src/components/Modal/components/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -182,11 +182,6 @@ export const Dialog = ({
event.stopPropagation()
}, [])

// Stop click to prevent unexpected dialog close
const stopClick: MouseEventHandler = useCallback(event => {
event.stopPropagation()
}, [])

// handle key up : used when having inputs in modals - useful for hideOnEsc
const handleKeyUp: KeyboardEventHandler = useCallback(
event => {
Expand All @@ -202,7 +197,13 @@ export const Dialog = ({
const handleClose: MouseEventHandler = useCallback(
event => {
event.stopPropagation()
if (hideOnClickOutside) {

// if the user actually click outside of modal
if (
hideOnClickOutside &&
dialogRef.current &&
!dialogRef.current.contains(event.target as Node)
) {
onCloseRef.current()
} else {
// Because overlay is not focusable we can't handle hideOnEsc properly
Expand Down Expand Up @@ -306,10 +307,8 @@ export const Dialog = ({
data-placement={placement}
data-size={size}
open
onClick={stopClick}
onCancel={stopCancel}
onClose={stopCancel}
onMouseDown={stopClick}
aria-modal
ref={dialogRef}
tabIndex={0}
Expand Down
Loading