react-components icon indicating copy to clipboard operation
react-components copied to clipboard

Modals should be inside Portals

Open huwshimi opened this issue 3 years ago • 5 comments

To prevent Modals being accidentally cut off by parent styles we should wrap the modal component in a Portal, like we do for contextual menus:

https://github.com/canonical-web-and-design/react-components/blob/master/src/components/ContextualMenu/ContextualMenu.tsx#L196

We'd need to figure out how to control the display of the Modal as the functions to display/hide the modal would be inside the component:

const { openPortal, closePortal, isOpen, Portal, ref } = usePortal({

huwshimi avatar Aug 05 '21 23:08 huwshimi

In the Juju Dashboard We have a wrapper around the Modal component to do just this. You can see the component here and one example of the usage here

What you'll notice is that the Modal visibility is controlled by a parent. If you want to close it we use an externally supplied callback which modifies the state on the parent to close the model. I prefer this approach as the Modal should always have an 'owner'.

hatched avatar Aug 06 '21 02:08 hatched

@hatched @huwshimi do you know the direction of this, do you need a meeting for this?

cristinadresch avatar Aug 12 '21 08:08 cristinadresch

nah, when the work is being tackled we can have a quick pre-imp.

hatched avatar Aug 12 '21 15:08 hatched

Yeah, I'm not really sure what the right solution is for this. It's not really pressing as there are easy work arounds when using the component, just a nice-to-have feature at some point.

huwshimi avatar Aug 12 '21 22:08 huwshimi

Triage: seems to be high effort with relatively low impact. This can be considered for the new architecture.

bartaz avatar Sep 30 '24 10:09 bartaz