marigold-ui / marigold

Design System based on react-aria and Tailwind CSS
https://marigold-ui.io
MIT License
101 stars 7 forks source link

OverlayProvider ist added multiple times with the MarigoldProvider #1195

Closed sebald closed 2 years ago

sebald commented 2 years ago

Description

The <MarigoldProvider> always adds react-aria's OverlayProvider. Especially if you using cascading theming. There should only be a single instance of the provider at any time.

Otherwise this can lead to inaccessible overlays.

viktoria-schwarz commented 2 years ago

Could this be a solution to the problem (in useTheme.tsx) since we are stacking the providers there?

sebald commented 2 years ago

Not quite, the lib solved a problem before hooks existed (requiring multiple context consumers). This is why it is deprecated now.

The solution is actually much simpler and stupid 😄

ti10le commented 2 years ago

@sebald Should I just try to check if the provider is there before adding it?

sebald commented 2 years ago

@sebald Should I just try to check if the provider is there before adding it?

@ti10le that's one solution, but how do you check that? 🙂

ti10le commented 2 years ago

If it is possible at all I have to find out first 😀

ti10le commented 2 years ago

@sebald How do you see that OverlayProvider added multiple times? I can only see this in devTools. Looks like theres only one time: image

sebald commented 2 years ago

See description -> cascading

ti10le commented 2 years ago

Ok now I get it. @sebald we have just asked ourselves the question whether we now only use the MarigoldProvider or also the ThemeProvider if we want to cascade e.g. only the theme?

sebald commented 2 years ago

My gut says <MarigoldProvider> since most users will only use the components package, hopefully. And they works be confused what the difference between the two is (since it's an Implementation detail).

Also: what if you don't know if the "outer" stuff uses marigold!?

ti10le commented 2 years ago

waiting for #1317