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.

Sign up for GitHub

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

Jump to bottom

Question: Intent on how overlay-kit Provider works #63

Closed
XionWCFM opened this issue Jul 16, 2024 · 2 comments · Fixed by #64 or #65 · May be fixed by jgjgill/overlay-kit#1
Closed

Question: Intent on how overlay-kit Provider works #63

XionWCFM opened this issue Jul 16, 2024 · 2 comments · Fixed by #64 or #65 · May be fixed by jgjgill/overlay-kit#1
Assignees
@jungpaeng
Labels
bug Something isn't working

Comments

@XionWCFM
Copy link
Contributor

Summery

The overlay kit provider seems to behave differently than other libraries. Is this intended behavior?

Detail

I realized that the following actions were needed in the test code to test the operation of overlay-kit.

afterEach(() => {
  overlay.unmountAll();
});

The reason this is necessary is because the OverlayProvider is separate from the elements mounted in the actual DOM.

Therefore, in most cases providing different OverlayProviders for individual tests does not make sense from the perspective of making the tests independent.

const wrapper = ({ children }: PropsWithChildren) => <OverlayProvider>{children}</OverlayProvider>;

However, libraries such as jotai can guarantee test independence by providing different providers.

import { Provider as JotaiProvider } from 'jotai'
const wrapper = ({ children }: PropsWithChildren) => <JotaiProvider>{children}</JotaiProvider>;

Personally, I think jotai's approach is closer to the provider role that users expect.

My Opinion

If this is the intended design, it would be good to provide documentation on how to test with overlay-kit.

If this wasn't intended, I think we need to think about how to do it.

I'm curious about your opinion,

Thank you for making a great library thanks! 🙂
@jungpaeng
Copy link
Member

Hello, and thank you for your interest and questions regarding overlay-kit.

You’re correct in noting that OverlayProviders are not intended to operate independently. The primary purpose of this component is to store state in a ContextAPI Provider and to render overlays alongside children.

This design choice was made with the intent for OverlayProviders to function as a singleton, under the assumption that there wouldn’t be a scenario where OverlayProviders would need to unmount in a service context. However, this overlooks scenarios involving mocking during testing, where OverlayProviders might indeed unmount.

To address this, we will make changes so that overlay.unmountAll() is called automatically when OverlayProviders unmount.

@jungpaeng jungpaeng self-assigned this Jul 16, 2024
@jungpaeng jungpaeng added the bug Something isn't working label Jul 16, 2024
@XionWCFM
Copy link
Contributor Author

Thank you for your interest in my opinion.

calling unmountAll when OverlayProvider is unmounted seems to solve the problem in the test scenario.

I think writing tests will be more enjoyable once this is done. Thank you.

@jungpaeng jungpaeng mentioned this issue Jul 17, 2024
Merged
11 tasks
This was referenced Jul 17, 2024
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jungpaeng jungpaeng

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
2 participants
@jungpaeng @XionWCFM