typescript(vx-responsive): re-write package in TypeScript #517
Conversation
|
@williaster Thanks for the review! Should be good now :) |
| }; | ||
|
|
||
| animationFrameID: number | null; | ||
| ro: ResizeObserver | undefined; |
There was a problem hiding this comment.
do we want to use more verbose name?
| const { parentWidth, parentHeight } = this.state; | ||
| return ( | ||
| <div | ||
| style={{ width: '100%', height: '100%' }} |
There was a problem hiding this comment.
Convert to constant.
Note: This 100% style is known to create a layout bug that leave some area of the screen empty when the children use less than 100% width or height, e.g. 50% width & 50% height. This is because this div always secure 100% width & height bounding box in the layout.
| window.removeEventListener('resize', this.handleResize, false); | ||
| } | ||
|
|
||
| resize(/** event */) { |
There was a problem hiding this comment.
use arrow function for binding?
| if (this.ro) this.ro.disconnect(); | ||
| } | ||
|
|
||
| resize({ width, height }: { width: number; height: number }) { |
There was a problem hiding this comment.
use arrow function for binding?
|
looks like we might need to add types for also, updated base branch to master. |
|
@williaster I bumped |
|
Hrmm, that's strange 🙈 Do you think it'd be fixable by changing the syntax to something like
instead of
|
|
@williaster that will work, thanks! Should be fixed now:) |
🚀 Enhancements
This PR builds off of #488 which added TypeScript build support, and converts the
@vx/responsivepackage to TypeScript.Closes #514.
Testing
@williaster @hshoff @schillerk @milesj @kristw