search

grommet / grommet

a react-based framework that provides accessibility, modularity, responsiveness, and theming in a tidy package
https://grommet.io
Apache License 2.0
8.33k stars 1.02k forks source link

useEventHandler custom hook suggestion #3552

Open ShimiSun opened 4 years ago

ShimiSun commented 4 years ago

onResize depends on width and height variables from state. So whenever these variables will change, our effect would re-run and add/remove the handler again.. and again. If it's not much of a problem, I can move the handler into the effect.

I suggested above that we should extract this as a custom hook useEventHandler based on this example: https://codesandbox.io/s/z64on3ypm I did the same thing with onResize as in the above example. There multiple components in which we would be able to use this hook.

Originally posted by @husseyexplores in https://github.com/grommet/grommet/pull/3458

ericsoderberghp commented 2 years ago

Which components are you referring to for onResize here?

husseyexplores commented 2 years ago

I can chime in, although I haven't contributed to this repo in a while.

This comment was posted in the context of Diagram.js component. Here is the link.

I assumed that this was a fairly common use-case (adding handlers) and many components will be able to use this hook. But, I may be wrong here.

g4rry420 commented 2 years ago

@husseyexplores Free feel to hop on PR, if you wanted to suggest anymore usecases.

husseyexplores commented 2 years ago

@g4rry420 I'm out of the loop and don't have the bandwidth, for now, so can't work on that. I never opened any PR on this though, it was a mere suggestion.

Just a bit more context behind the suggestion can be found here. https://github.com/grommet/grommet/pull/3458/files/a3e7d247f008f07498ae396b4706a074b88b2cfe#diff-1480b2584abc66e805d27e993dd3fbb493aa36b758acae32e0f3541c40370685R77