-
Notifications
You must be signed in to change notification settings - Fork 30
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
Dependencies are needed #24
Comments
useMeasure(null, [scale])
useMeasure({ ... }, [scale]) or ... (not sure if that's a good idea due to typescript) useMeasure([scale])
useMeasure({ ... }, [scale]) ? would you want to add a pr for this? |
btw there is a forceRefresh for such cases: https://codesandbox.io/s/quizzical-margulis-95c09?file=/src/App.js:284-380 const [scale, setScale] = useState(1)
const [ref, size, refresh] = useMeasure()
useEffect(() => void refresh(), [scale]) but i agree, dependencies would probably be cleaner. |
@drcmda thanks for the tip, didn't know that. Would be great to add info about refresh to docs. |
Please check PR #25. I think to pass dependencies in the Options object is much better for maintainability instead of second argument. If it is ok, I can update docs. |
If this ever revives, maybe the dependencies could be upgraded to not only support [dependency] but to also support [{dependency, delay}] since dependencies could be css transitions and you shouldn't refresh until the transition is finished. Could you a custom useTimeout instead of useEffect. For example, css { transition: margin 500ms ease-in-out;} I suggest a delay would be cleaner than listening for a transitionend event and adding state to provide a dependency that is triggered after the transition ends. |
Would be great to pass dependencies to this hook. For example, if a div scales with css transform useMeasure does not handle these changes, you need to resize window to get an actual size.
Please check an example: https://codesandbox.io/s/nostalgic-shannon-ckeq4?file=/src/App.js
So, the solution might be, like:
The text was updated successfully, but these errors were encountered: