-
-
Notifications
You must be signed in to change notification settings - Fork 17
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
Use custom browsing context instead of global window object if provided #133
Conversation
Someone is attempting to deploy a commit to a Personal Account owned by @nerdyman on Vercel. @nerdyman first needs to authorize it. |
Thanks for opening this @martin-pettersson, it's a cool use case! The code LGTM and thanks for updating the docs too 🙌 I haven't done much with binding to alternate event targets before, If you're able to provide the code you tried in Storybook I'll take a look and see if I can figure something out for the tests. Edit: It looks like this is breaking the SSR Tests. I think it's because it's referring to |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
I tried just running the Storybook without any changes but couldn't get it up and running locally. I can try again in a bit but please see if these changes fixes the SSR tests. I tried assigning the default value when the component is created instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @martin-pettersson, sorry for the delay in getting this reviewed. I pulled down your branch and found that you can use globalThis
as the default forbrowsingContext
safely on the client and server.
Good idea! I've implemented your suggestions. I'm sorry I can't run the tests locally. |
Thanks for your PR @martin-pettersson! I've been pretty busy recently so haven't been able to give the repo as much attention as I'd like. I'll drop a message when the release goes out. |
Still need to do some more testing on this so have published a prerelease https://github.com/nerdyman/react-compare-slider/releases/tag/v3.0.2-1 |
This was released in |
I ran into a scenario where I needed to use this library in a popup window and the easiest solution was to render the component using a React portal. The issue then was that this library was listening for events on the "origin" window object instead of the popup window.
If we allow users to pass in a custom browsing context as a prop it's fairly easy to solve this issue. Here's a trivial example with some sudo-ish code to illustrate the solution.
I couldn't for the life of me get the storybook to run locally without any errors so I didn't create any additional documentation, examples or tests. Since this solution falls back to the global
window
object if no browsing context is provided this doesn't break anything.If you need any additional explanation of examples please let me know.