-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
compose: Add types to useResizeObserver
and type checking to useIsomorphicLayoutEffect
#32111
Conversation
Size Change: 0 B Total Size: 1.86 MB ℹ️ View Unchanged
|
@@ -10,7 +10,7 @@ import { useState, useCallback } from '@wordpress/element'; | |||
/** | |||
* Hook which allows to listen the resize event of any target element when it changes sizes. | |||
* | |||
* @return {Array} An array of {Element} `resizeListener` and {?Object} `sizes` with properties `width` and `height` | |||
* @return {[JSX.Element, { width: number, height: number } | null]} An array of {Element} `resizeListener` and {?Object} `sizes` with properties `width` and `height` |
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.
Why is the React Native version a completely different implementation of the web version?
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.
I'm asuming react-resize-aware
doesn't work for react native 🤷♀️ In any case it's outside the scope of this PR. They have the exact same API FWIW.
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.
This might be a little controversial as I'm actually just relying on the fact that we're re-exporting react-resize-aware and that the types should come from there. This slightly worsens the WP docs but with a link to the upstream docs I think it's for the best.
I think this is definitely a good step forward.
Should we pin a specific version of the react-size-aware
package when linking out to its repo / docs?
63f1a90
to
bc68297
Compare
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.
LGTM 👍
Description
Adds types and type checking to
useResizeObserver
anduseIsomorphicLayoutEffect
.This might be a little controversial as I'm actually just relying on the fact that we're re-exporting
react-resize-aware
and that the types should come from there. This slightly worsens the WP docs but with a link to the upstream docs I think it's for the best.How has this been tested?
Type checks pass. No runtime changes.
Types of changes
New feature.
Checklist:
*.native.js
files for terms that need renaming or removal).