Improve cover z-index solution #66249
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -164,7 +164,7 @@ const v8ToV11BlockAttributes = { | |
| }, | ||
| }; | ||
|
|
||
| const v12toV13BlockAttributes = { | ||
| ...v8ToV11BlockAttributes, | ||
| useFeaturedImage: { | ||
| type: 'boolean', | ||
|
|
@@ -176,6 +176,13 @@ const v12BlockAttributes = { | |
| }, | ||
| }; | ||
|
|
||
| const v14BlockAttributes = { | ||
| ...v12toV13BlockAttributes, | ||
| sizeSlug: { | ||
| type: 'string', | ||
| }, | ||
| }; | ||
|
|
||
| const v7toV11BlockSupports = { | ||
| anchor: true, | ||
| align: true, | ||
|
|
@@ -244,9 +251,162 @@ const v12BlockSupports = { | |
| }, | ||
| }; | ||
|
|
||
| // Deprecation for blocks that have z-index. | ||
| const v14 = { | ||
|
There was a problem hiding this comment. This deprecation will need a fixture to cover it. Here's an example for an earlier deprecation. There was a problem hiding this comment. Thank you for the suggestion and the example! There was a problem hiding this comment. I followed this documentation, and based on your example, I added the new fixture: 9ba72c0 (created the HTML and let the command generate the others) I'm still struggling a bit to fix the others. I let you know here when it's done. There was a problem hiding this comment. I fixed most of the issues (0ae6273), but in the fixture tests it's trying to add the Not sure if it's a bug with the fixture tests, or if adding a class to the wrapper element wouldn't be recommended. 🤔 I thought of not using the sibling class, as you suggested to fix this, but it would mean a few new classes instead of just one. Another approach would be adding an additional wrapper with a class, it would break a few styles (we have some using I'm out of ideas now. I need to think more. And I noticed that I also need to update some mobile snapshots. There was a problem hiding this comment. Thanks for the updates, I'll review those shortly. Regarding the sibling selectors though, why would we need additional classes? We should be able to rely on the order of the elements to have the correct sibling selector apply because the old and new versions moved the overlay element. When there aren't multiple elements there, we wouldn't need to juggle their z-index. I'm a little short on bandwidth at the moment to code something up but I could take a run at it later this week perhaps if you don't beat me to it. It could be worth pursuing this, even if it is ruled out as an option, as it would limit the number of blocks needing migration and avoid the slightly awkward class on the wrapper. There was a problem hiding this comment. Hey! 👋 Thank you for your suggestions! I followed your suggestion and applied it with this commit: 0e25fed. |
||
| attributes: v14BlockAttributes, | ||
| supports: v12BlockSupports, | ||
| save( { attributes } ) { | ||
| const { | ||
| backgroundType, | ||
| gradient, | ||
| contentPosition, | ||
| customGradient, | ||
| customOverlayColor, | ||
| dimRatio, | ||
| focalPoint, | ||
| useFeaturedImage, | ||
| hasParallax, | ||
| isDark, | ||
| isRepeated, | ||
| overlayColor, | ||
| url, | ||
| alt, | ||
| id, | ||
| minHeight: minHeightProp, | ||
| minHeightUnit, | ||
| tagName: Tag, | ||
| sizeSlug, | ||
| } = attributes; | ||
| const overlayColorClass = getColorClassName( | ||
| 'background-color', | ||
| overlayColor | ||
| ); | ||
| const gradientClass = __experimentalGetGradientClass( gradient ); | ||
| const minHeight = | ||
| minHeightProp && minHeightUnit | ||
| ? `${ minHeightProp }${ minHeightUnit }` | ||
| : minHeightProp; | ||
|
|
||
| const isImageBackground = IMAGE_BACKGROUND_TYPE === backgroundType; | ||
| const isVideoBackground = VIDEO_BACKGROUND_TYPE === backgroundType; | ||
|
|
||
| const isImgElement = ! ( hasParallax || isRepeated ); | ||
|
|
||
| const style = { | ||
| minHeight: minHeight || undefined, | ||
| }; | ||
|
|
||
| const bgStyle = { | ||
| backgroundColor: ! overlayColorClass | ||
| ? customOverlayColor | ||
| : undefined, | ||
| background: customGradient ? customGradient : undefined, | ||
| }; | ||
|
|
||
| const objectPosition = | ||
| // prettier-ignore | ||
| focalPoint && isImgElement | ||
| ? mediaPosition(focalPoint) | ||
| : undefined; | ||
|
|
||
| const backgroundImage = url ? `url(${ url })` : undefined; | ||
|
|
||
| const backgroundPosition = mediaPosition( focalPoint ); | ||
|
|
||
| const classes = clsx( | ||
| { | ||
| 'is-light': ! isDark, | ||
| 'has-parallax': hasParallax, | ||
| 'is-repeated': isRepeated, | ||
| 'has-custom-content-position': | ||
| ! isContentPositionCenter( contentPosition ), | ||
| }, | ||
| getPositionClassName( contentPosition ) | ||
| ); | ||
|
|
||
| const imgClasses = clsx( | ||
| 'wp-block-cover__image-background', | ||
| id ? `wp-image-${ id }` : null, | ||
| { | ||
| [ `size-${ sizeSlug }` ]: sizeSlug, | ||
| 'has-parallax': hasParallax, | ||
| 'is-repeated': isRepeated, | ||
| } | ||
| ); | ||
|
|
||
| const gradientValue = gradient || customGradient; | ||
|
|
||
| return ( | ||
| <Tag { ...useBlockProps.save( { className: classes, style } ) }> | ||
| <span | ||
| aria-hidden="true" | ||
| className={ clsx( | ||
| 'wp-block-cover__background', | ||
| overlayColorClass, | ||
| dimRatioToClass( dimRatio ), | ||
| { | ||
| 'has-background-dim': dimRatio !== undefined, | ||
| // For backwards compatibility. Former versions of the Cover Block applied | ||
| // `.wp-block-cover__gradient-background` in the presence of | ||
| // media, a gradient and a dim. | ||
| 'wp-block-cover__gradient-background': | ||
| url && gradientValue && dimRatio !== 0, | ||
| 'has-background-gradient': gradientValue, | ||
| [ gradientClass ]: gradientClass, | ||
| } | ||
| ) } | ||
| style={ bgStyle } | ||
| /> | ||
|
|
||
| { ! useFeaturedImage && | ||
| isImageBackground && | ||
| url && | ||
| ( isImgElement ? ( | ||
| <img | ||
| className={ imgClasses } | ||
| alt={ alt } | ||
| src={ url } | ||
| style={ { objectPosition } } | ||
| data-object-fit="cover" | ||
| data-object-position={ objectPosition } | ||
| /> | ||
| ) : ( | ||
| <div | ||
| role={ alt ? 'img' : undefined } | ||
| aria-label={ alt ? alt : undefined } | ||
| className={ imgClasses } | ||
| style={ { backgroundPosition, backgroundImage } } | ||
| /> | ||
| ) ) } | ||
| { isVideoBackground && url && ( | ||
| <video | ||
| className={ clsx( | ||
| 'wp-block-cover__video-background', | ||
| 'intrinsic-ignore' | ||
| ) } | ||
| autoPlay | ||
| muted | ||
| loop | ||
| playsInline | ||
| src={ url } | ||
| style={ { objectPosition } } | ||
| data-object-fit="cover" | ||
| data-object-position={ objectPosition } | ||
| /> | ||
| ) } | ||
| <div | ||
| { ...useInnerBlocksProps.save( { | ||
| className: 'wp-block-cover__inner-container', | ||
| } ) } | ||
| /> | ||
| </Tag> | ||
| ); | ||
| }, | ||
| }; | ||
|
|
||
| // Deprecation for blocks that does not have the aria-label when the image background is fixed or repeated. | ||
| const v13 = { | ||
| attributes: v12toV13BlockAttributes, | ||
| supports: v12BlockSupports, | ||
| save( { attributes } ) { | ||
| const { | ||
|
|
@@ -396,7 +556,7 @@ const v13 = { | |
|
|
||
| // Deprecation for blocks to prevent auto overlay color from overriding previously set values. | ||
| const v12 = { | ||
| attributes: v12toV13BlockAttributes, | ||
| supports: v12BlockSupports, | ||
| isEligible( attributes ) { | ||
| return ( | ||
|
|
@@ -1824,4 +1984,4 @@ const v1 = { | |
| }, | ||
| }; | ||
|
|
||
| export default [ v14, v13, v12, v11, v10, v9, v8, v7, v6, v5, v4, v3, v2, v1 ]; | ||
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 removing it again because the change discussed here was reverted.