[utils] Add useResizeObserver hook #27736
Conversation
|
Details of bundle changes (experimental) @material-ui/core: parsed: +0.05% , gzip: +0.09% |
There was a problem hiding this comment.
It could be interesting to write the hook in TypeScript directly, e.g. https://github.com/react-restart/hooks/blob/master/src/useResizeObserver.ts
hbjORbj
force-pushed
the
resize-observer-hook
branch
from
435304e to
e59b9b4
Compare
|
Thank you for your feedback. I will solidify it further as I look more closely into the components that can benefit from this hook; e.g., the five components linked in #22519. In the meantime, an example of how this hook will be used for a component ( |
There was a problem hiding this comment.
I don't see why this should be a generic hook. Especially since this is synchronous works that re-renders whenever the rect changes even though it's probably only worth it to re-render on either width or height.
Let's not build something nobody has asked for.
|
|
||
| export default function useResizeObserver(): [Rect | null, (elem: Element) => () => void] { | ||
| const [rect, setRect] = React.useState<Rect | null>(null); | ||
| const ref = React.useCallback((elem: Element) => { |
There was a problem hiding this comment.
How would this be used? We can't just pass it to a ref prop since a ref prop does not support cleanup.
There was a problem hiding this comment.
I removed useCallback and tried this way: 3f49153
Let me know!
@eps1lon It does feel wrong to add it without solving a problem end-to-end. In #22519, we link 4 unrelated issues where we can directly use this hook. Maybe we could pick (solve) any one of them to prove the module works (is the best solution), keep the module unstable/undocumented until we cover them all. |
@oliviertassinari I agree. I made commits |
hbjORbj
force-pushed
the
resize-observer-hook
branch
from
0d289a5 to
2d14721
Compare
hbjORbj
force-pushed
the
resize-observer-hook
branch
from
a3260a7 to
0ddba62
Compare
|
let's fix the individual issues first and then think about the abstraction. Not the other way around. I guarantee you the current abstraction is incorrect. |
|
And I really mean each individual issue. This is not nearly as straight forward as you think it is. We need to slow down in areas where we lack the expertise. Not just push everything through and assume we know better. |
|
@hbjORbj should we close this one? Looks like we have dedicated PRs for |
Yes, I will close this. @oliviertassinari also mentioned here that this hook may not be needed. |
This pull request is a continuation of this issue: #22519