Skip to content
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

ResponsiveWrapper: Convert to TypeScript #46480

Merged
merged 9 commits into from
Jan 6, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
- `useBaseField`: Convert to TypeScript ([#45712](https://github.com/WordPress/gutenberg/pull/45712)).
- `Dashicon`: Convert to TypeScript ([#45924](https://github.com/WordPress/gutenberg/pull/45924)).
- `PaletteEdit`: add follow up changelog for #45681 and tests [#46095](https://github.com/WordPress/gutenberg/pull/46095).
- `ResponsiveWrapper`: Convert to TypeScript ([#46480](https://github.com/WordPress/gutenberg/pull/46480)).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May need repositioning after the last package release

- `AlignmentMatrixControl`: Convert to TypeScript ([#46162](https://github.com/WordPress/gutenberg/pull/46162)).

### Documentation
Expand Down
29 changes: 29 additions & 0 deletions packages/components/src/responsive-wrapper/README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# ResponsiveWrapper

A wrapper component that maintains its aspect ratio when resized.

## Usage

```jsx
Expand All @@ -14,3 +16,30 @@ const MyResponsiveWrapper = () => (
</ResponsiveWrapper>
);
```

## Props

### `children`: `React.ReactElement`

The element to wrap.

- Required: Yes

### `isInline`: `boolean`

If true, the wrapper will be `span` instead of `div`.

- Required: No
- Default: `false`

### `naturalHeight`: `number`

The intrinsic height of the element to wrap. Will be used to determine the aspect ratio.

- Required: Yes

### `naturalWidth`: `number`

The intrinsic width of the element to wrap. Will be used to determine the aspect ratio.

- Required: Yes
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,41 @@ import classnames from 'classnames';
import { cloneElement, Children } from '@wordpress/element';
import { useResizeObserver } from '@wordpress/compose';

/**
* Internal dependencies
*/
import type { ResponsiveWrapperProps } from './types';

/**
* A wrapper component that maintains its aspect ratio when resized.
*
* ```jsx
* import { ResponsiveWrapper } from '@wordpress/components';
*
* const MyResponsiveWrapper = () => (
* <ResponsiveWrapper naturalWidth={ 2000 } naturalHeight={ 680 }>
* <img
* src="https://s.w.org/style/images/about/WordPress-logotype-standard.png"
* alt="WordPress"
* />
* </ResponsiveWrapper>
* );
* ```
*/
function ResponsiveWrapper( {
naturalWidth,
naturalHeight,
children,
isInline = false,
} ) {
}: ResponsiveWrapperProps ) {
const [ containerResizeListener, { width: containerWidth } ] =
useResizeObserver();
if ( Children.count( children ) !== 1 ) {
return null;
}
const imageStyle = {
paddingBottom:
naturalWidth < containerWidth
naturalWidth < ( containerWidth ?? 0 )
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TS was not happy because containerWidth can potentially be null. (Apparently useResizeObserver() will return null for this value until after the first render.)

My code change here just makes explicit what was happening implicitly — a null will be interpreted as 0 when compared with the < operator. This maintains the current behavior while assuring TS that it's ok.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent explanation, saved a bunch of time on my end and will make it clear for anyone looking into the reasons behind this code change. Thank you!

? naturalHeight
: ( naturalHeight / naturalWidth ) * 100 + '%',
};
Expand Down
35 changes: 35 additions & 0 deletions packages/components/src/responsive-wrapper/stories/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/**
* External dependencies
*/
import type { ComponentMeta, ComponentStory } from '@storybook/react';

/**
* Internal dependencies
*/
import ResponsiveWrapper from '..';

const meta: ComponentMeta< typeof ResponsiveWrapper > = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we hide children's control? It currently shows a react object, which is not really user-friendly to edit

component: ResponsiveWrapper,
title: 'Components/ResponsiveWrapper',
parameters: {
controls: { expanded: true },
docs: { source: { state: 'open' } },
},
};
export default meta;

const Template: ComponentStory< typeof ResponsiveWrapper > = ( args ) => (
<ResponsiveWrapper { ...args } />
);

export const Default = Template.bind( {} );
Default.args = {
naturalWidth: 2000,
naturalHeight: 680,
children: (
<img
src="https://s.w.org/style/images/about/WordPress-logotype-standard.png"
alt="WordPress"
/>
),
};
20 changes: 20 additions & 0 deletions packages/components/src/responsive-wrapper/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
export type ResponsiveWrapperProps = {
/**
* The intrinsic width of the element to wrap. Will be used to determine the aspect ratio.
*/
naturalWidth: number;
/**
* The intrinsic height of the element to wrap. Will be used to determine the aspect ratio.
*/
naturalHeight: number;
/**
* The element to wrap.
*/
children: React.ReactElement;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The choice of ReactElement seems correct. We can also broaden the type if necessary.

/**
* If true, the wrapper will be `span` instead of `div`.
*
* @default false
*/
isInline?: boolean;
};
1 change: 0 additions & 1 deletion packages/components/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
"src/palette-edit",
"src/panel",
"src/query-controls",
"src/responsive-wrapper",
"src/sandbox",
"src/toolbar",
"src/toolbar-button",
Expand Down