From a17dab5a03c2718679733fe88803c59e5ccabe66 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 28 Feb 2023 01:03:56 +0100 Subject: [PATCH 1/6] ResponsiveWrapper: use aspect-ratio + intrinsic width/height instead of padding-bottom hack --- .../src/responsive-wrapper/index.tsx | 35 ++++++++++--------- .../src/responsive-wrapper/style.scss | 16 +++------ 2 files changed, 23 insertions(+), 28 deletions(-) diff --git a/packages/components/src/responsive-wrapper/index.tsx b/packages/components/src/responsive-wrapper/index.tsx index b59731539f3a4..a8b2ef9ff438a 100644 --- a/packages/components/src/responsive-wrapper/index.tsx +++ b/packages/components/src/responsive-wrapper/index.tsx @@ -7,7 +7,6 @@ import classnames from 'classnames'; * WordPress dependencies */ import { cloneElement, Children } from '@wordpress/element'; -import { useResizeObserver } from '@wordpress/compose'; /** * Internal dependencies @@ -36,28 +35,30 @@ function ResponsiveWrapper( { children, isInline = false, }: ResponsiveWrapperProps ) { - const [ containerResizeListener, { width: containerWidth } ] = - useResizeObserver(); if ( Children.count( children ) !== 1 ) { return null; } - const imageStyle = { - paddingBottom: - naturalWidth < ( containerWidth ?? 0 ) - ? naturalHeight - : ( naturalHeight / naturalWidth ) * 100 + '%', - }; + const TagName = isInline ? 'span' : 'div'; + let aspectRatio; + if ( naturalWidth && naturalHeight ) { + aspectRatio = `${ naturalWidth } / ${ naturalHeight }`; + } + return ( - { containerResizeListener } - - { cloneElement( children, { - className: classnames( - 'components-responsive-wrapper__content', - children.props.className - ), - } ) } +
+ { cloneElement( children, { + className: classnames( + 'components-responsive-wrapper__content', + children.props.className + ), + style: { + ...children.props.style, + aspectRatio, + }, + } ) } +
); } diff --git a/packages/components/src/responsive-wrapper/style.scss b/packages/components/src/responsive-wrapper/style.scss index b6a556bf3425d..d32fed8cd4840 100644 --- a/packages/components/src/responsive-wrapper/style.scss +++ b/packages/components/src/responsive-wrapper/style.scss @@ -1,19 +1,13 @@ .components-responsive-wrapper { position: relative; max-width: 100%; - &, - & > span { - display: block; - } + display: flex; + align-items: center; + justify-content: center; } .components-responsive-wrapper__content { - position: absolute; - top: 0; - right: 0; - bottom: 0; - left: 0; + display: block; + max-width: 100%; width: 100%; - height: 100%; - margin: auto; } From c7a0422d9fb58709dfeca74307c0a17256224dd0 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 28 Feb 2023 01:04:15 +0100 Subject: [PATCH 2/6] Mark naturalHeight and naturalWidth props as optional --- packages/components/src/responsive-wrapper/README.md | 4 ++-- packages/components/src/responsive-wrapper/types.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/components/src/responsive-wrapper/README.md b/packages/components/src/responsive-wrapper/README.md index 5ccfc16c1b84d..ae230afa92026 100644 --- a/packages/components/src/responsive-wrapper/README.md +++ b/packages/components/src/responsive-wrapper/README.md @@ -36,10 +36,10 @@ If true, the wrapper will be `span` instead of `div`. The intrinsic height of the element to wrap. Will be used to determine the aspect ratio. -- Required: Yes +- Required: No ### `naturalWidth`: `number` The intrinsic width of the element to wrap. Will be used to determine the aspect ratio. -- Required: Yes \ No newline at end of file +- Required: No diff --git a/packages/components/src/responsive-wrapper/types.ts b/packages/components/src/responsive-wrapper/types.ts index 2f1e4a53aafbf..7f76ff8af5a55 100644 --- a/packages/components/src/responsive-wrapper/types.ts +++ b/packages/components/src/responsive-wrapper/types.ts @@ -2,11 +2,11 @@ export type ResponsiveWrapperProps = { /** * The intrinsic width of the element to wrap. Will be used to determine the aspect ratio. */ - naturalWidth: number; + naturalWidth?: number; /** * The intrinsic height of the element to wrap. Will be used to determine the aspect ratio. */ - naturalHeight: number; + naturalHeight?: number; /** * The element to wrap. */ From 5004b9ab50fdd4fa2b7c51259bab8ed32e442132 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 28 Feb 2023 01:04:25 +0100 Subject: [PATCH 3/6] Add SVG example story --- .../src/responsive-wrapper/stories/index.tsx | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/packages/components/src/responsive-wrapper/stories/index.tsx b/packages/components/src/responsive-wrapper/stories/index.tsx index 5f48b91d279b2..f8f06c30c0b00 100644 --- a/packages/components/src/responsive-wrapper/stories/index.tsx +++ b/packages/components/src/responsive-wrapper/stories/index.tsx @@ -36,3 +36,41 @@ Default.args = { /> ), }; + +/** + * Since SVG images don't have the equivalent of a natural width and height, + * it is possible that the `naturalWidth` and `naturalHeight` properties are not + * specified. In this case, the SVG simply keeps scaling up to fill its + * container, unless the `height` and `width` attributes are specified. + */ +export const WithSVG: ComponentStory< typeof ResponsiveWrapper > = + Template.bind( {} ); +WithSVG.args = { + children: ( + + + + + + + + + ), +}; From c8ef28d31413077768914bf7435576c22d037fee Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 28 Feb 2023 09:54:06 +0100 Subject: [PATCH 4/6] Add more information to README --- packages/components/src/responsive-wrapper/README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/components/src/responsive-wrapper/README.md b/packages/components/src/responsive-wrapper/README.md index ae230afa92026..31acf65932d4e 100644 --- a/packages/components/src/responsive-wrapper/README.md +++ b/packages/components/src/responsive-wrapper/README.md @@ -17,6 +17,12 @@ const MyResponsiveWrapper = () => ( ); ``` +### Usage with `SVG` elements + +When passing an `SVG` element as the ``'s child, make sure that it has the `viewbox` and the `preserveAspectRatio` set. + +When dealing with SVGs, it may not be possible to derive its `naturalWidth` and `naturalHeight` and therefore passing them as propertied to ``. In this case, the SVG simply keeps scaling up to fill its container, unless the `height` and `width` attributes are specified. + ## Props ### `children`: `React.ReactElement` From 3c21645066345aacfbb92fdf4ca0089a6ebdd110 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 28 Feb 2023 09:54:54 +0100 Subject: [PATCH 5/6] Update story description --- .../src/responsive-wrapper/stories/index.tsx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/components/src/responsive-wrapper/stories/index.tsx b/packages/components/src/responsive-wrapper/stories/index.tsx index f8f06c30c0b00..acca93afe56bc 100644 --- a/packages/components/src/responsive-wrapper/stories/index.tsx +++ b/packages/components/src/responsive-wrapper/stories/index.tsx @@ -38,10 +38,13 @@ Default.args = { }; /** - * Since SVG images don't have the equivalent of a natural width and height, - * it is possible that the `naturalWidth` and `naturalHeight` properties are not - * specified. In this case, the SVG simply keeps scaling up to fill its - * container, unless the `height` and `width` attributes are specified. + * When passing an `SVG` element as the ``'s child, make + * sure that it has the `viewbox` and the `preserveAspectRatio` set. + * + * When dealing with SVGs, it may not be possible to derive its `naturalWidth` + * and `naturalHeight` and therefore passing them as propertied to + * ``. In this case, the SVG simply keeps scaling up to fill + * its container, unless the `height` and `width` attributes are specified. */ export const WithSVG: ComponentStory< typeof ResponsiveWrapper > = Template.bind( {} ); From 182e178ecc48e913cb8efadc164289a44bacfb98 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 28 Feb 2023 09:57:24 +0100 Subject: [PATCH 6/6] CHANGELOG --- packages/components/CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 799703d001b61..854fb6f13cdc2 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -5,6 +5,10 @@ ### Enhancements - `FontSizePicker`: Allow custom units for custom font size control ([#48468](https://github.com/WordPress/gutenberg/pull/48468)). +### Bug Fix + +- `ResponsiveWrapper`: use `aspect-ratio` CSS prop, add support for `SVG` elements ([#48573](https://github.com/WordPress/gutenberg/pull/48573). + ### Internal - `Guide`: Convert to TypeScript ([#47493](https://github.com/WordPress/gutenberg/pull/47493)). @@ -17,7 +21,7 @@ ### Enhancements -- `ToolsPanel`: Separate reset all filter registration from items registration and support global resets ([#48123](https://github.com/WordPress/gutenberg/pull/48123#pullrequestreview-1308386926)). +- `ToolsPanel`: Separate reset all filter registration from items registration and support global resets ([#48123](https://github.com/WordPress/gutenberg/pull/48123)). ### Internal