search

canonical / react-components

A set of components based on Vanilla Framework
https://canonical.github.io/react-components
99 stars 55 forks source link

Modals should be inside Portals #567

Open huwshimi opened 3 years ago

huwshimi commented 3 years ago

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({

hatched commented 3 years ago

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'.

cristinadresch commented 3 years ago

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

hatched commented 3 years ago

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

huwshimi commented 3 years ago

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.

bartaz commented 2 days ago

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