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

OverlayProvider ist added multiple times with the MarigoldProvider #1195

Closed
sebald opened this issue Jul 28, 2021 · 10 comments · Fixed by #1498
Closed

OverlayProvider ist added multiple times with the MarigoldProvider #1195

sebald opened this issue Jul 28, 2021 · 10 comments · Fixed by #1498
Assignees
Labels
status:ready Ready for development type:bug Something isn't working type:feature New feature or component

Comments

@sebald
Copy link
Member

sebald commented Jul 28, 2021

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.

@sebald sebald added status:ready Ready for development type:bug Something isn't working type:feature New feature or component labels Jul 28, 2021
@viktoria-schwarz
Copy link
Contributor

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

@sebald
Copy link
Member Author

sebald commented Aug 23, 2021

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 ti10le self-assigned this Aug 23, 2021
@ti10le
Copy link
Contributor

ti10le commented Aug 24, 2021

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

@sebald
Copy link
Member Author

sebald commented Aug 24, 2021

@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
Copy link
Contributor

ti10le commented Aug 24, 2021

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

@ti10le
Copy link
Contributor

ti10le commented Aug 24, 2021

@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
Copy link
Member Author

sebald commented Aug 24, 2021

See description -> cascading

@ti10le
Copy link
Contributor

ti10le commented Aug 24, 2021

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
Copy link
Member Author

sebald commented Aug 24, 2021

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
Copy link
Contributor

ti10le commented Sep 28, 2021

waiting for #1317

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:ready Ready for development type:bug Something isn't working type:feature New feature or component
Projects
None yet
3 participants