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

renderProps / hooks #2

Closed
diegohaz opened this issue Mar 9, 2019 · 8 comments
Closed

renderProps / hooks #2

diegohaz opened this issue Mar 9, 2019 · 8 comments
Labels

Comments

@diegohaz
Copy link

diegohaz commented Mar 9, 2019

Hi @theKashey. I found this library while researching for solutions to build accessible dialogs. Thanks for your work.

Do you have plans to provide render prop and/or hooks API?

@theKashey
Copy link
Owner

It's not possible - there is no place for API such this.
RemoveScroll should be "declarative" not only to make application a bit more accessible, but to prevent real incidents - last year we have a few, with a page scroll locked when it should not.
Scroll/Focus locks are quite dangerous things, let's keep API a bit more "safe".

@theKashey
Copy link
Owner

Anyway - maybe we are talking about different aspects. Could you give an example from your mind?

@diegohaz
Copy link
Author

My use case is that I'm shipping a bunch of hooks. They return props like ref and event handlers. I can't use this library right now because (1) it's a component and (2) I can't combine props.

@theKashey
Copy link
Owner

The only way to expose hooked API is to rewrite everything to hooks, then redo component API using hooks, and then export component and hooked API for everyone. In the same time it will reduce the package size.
The problem is - I would have to apply this to the all my "locking" stack, as long as they supposed to work together and share common libraries. So - it would take time.

PS: I am still not sure about "safety" of this approach.

@theKashey
Copy link
Owner

@diegohaz
Copy link
Author

I ended up using this imperative library within React.useEffect. It seems to work well.

@theKashey
Copy link
Owner

body-scroll-lock is not bad, I would recommend scroll-lock as a better alternative.
But the main driver for react-remove-scroll creation were portals. And that's one of the reasons why it is "component" based - I need to hook into React Event propagation system. Nowadays portals are everywhere, and every second modal contain a react-based select/dropdown - so it's a much-have.

@stale
Copy link

stale bot commented Apr 30, 2023

This issue has been marked as "stale" because there has been no activity for 2 months. If you have any new information or would like to continue the discussion, please feel free to do so. If this issue got buried among other tasks, maybe this message will reignite the conversation. Otherwise, this issue will be closed in 7 days. Thank you for your contributions so far.

@stale stale bot added the state label Apr 30, 2023
@stale stale bot closed this as completed May 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants