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

Add featured image to Media & Text block #51491

Merged
merged 32 commits into from
Mar 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
4289a75
Add featured image to Media & Text block
carolinan Jun 14, 2023
1ff701f
Try adding the featured image option to the toolbar (replace flow)
carolinan Jun 15, 2023
9505f7d
fix JS warning in templates where there is no featured image.
carolinan Jun 15, 2023
f1de86e
Add deprecation, update fixtures
carolinan Jun 16, 2023
c1f8969
Try adding index.php
carolinan Jun 21, 2023
e599a7d
Index.php: use the $content and only replace the image source and the…
carolinan Aug 11, 2023
b70c2fa
Update index.php
carolinan Aug 12, 2023
27d42bc
Update index.php
carolinan Aug 13, 2023
f12bd39
Add alt and class names to the image tag
carolinan Aug 14, 2023
02484dc
Fix white space issue
carolinan Aug 14, 2023
38bd0c7
Add a placeholder for the featured image
carolinan Aug 17, 2023
2d1ce2c
Remove the deprecation
carolinan Oct 3, 2023
ebf6555
Add condition with useFeaturedImage, remove alt text option from plac…
carolinan Oct 3, 2023
87fbd79
Set a minimum height for the placeholder image
carolinan Jan 3, 2024
d0e6eda
Merge branch 'trunk' into add/media-text-featured-image
carolinan Jan 10, 2024
759db59
Remove set_attribute 'alt' from index.php.
carolinan Jan 10, 2024
d7a553e
Merge branch 'trunk' into add/media-text-featured-image
carolinan Jan 12, 2024
3a47c7c
Update editor.scss
carolinan Jan 12, 2024
9755334
Merge branch 'trunk' into add/media-text-featured-image
carolinan Jan 18, 2024
665f068
Update packages/block-library/src/media-text/index.php
carolinan Jan 28, 2024
183fb7a
Merge branch 'trunk' into add/media-text-featured-image
carolinan Jan 29, 2024
c916464
Update packages/block-library/src/media-text/media-container.js
carolinan Jan 29, 2024
4ee5cf3
Merge branch 'trunk' into add/media-text-featured-image
carolinan Feb 1, 2024
7e3c6b1
Try removing the image mediaType and inserting the img tag in index.php.
carolinan Feb 1, 2024
ac0a606
CS fix
carolinan Feb 1, 2024
f89e7af
Merge branch 'trunk' into add/media-text-featured-image
carolinan Feb 6, 2024
1aa6703
Add condition to save.js.
carolinan Feb 6, 2024
96c3a18
Merge branch 'trunk' into add/media-text-featured-image
carolinan Feb 14, 2024
ffdc6f1
Merge branch 'trunk' into add/media-text-featured-image
carolinan Feb 22, 2024
4c19b22
Merge branch 'trunk' into add/media-text-featured-image
carolinan Mar 8, 2024
f4508d5
CS: Remove the truthy default value for withIllustration in media-tex…
carolinan Mar 8, 2024
04a6d07
Update the condition that outputs the img tag
carolinan Mar 8, 2024
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
2 changes: 1 addition & 1 deletion docs/reference-guides/core-blocks.md
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ Set media and words side-by-side for a richer layout. ([Source](https://github.c
- **Name:** core/media-text
- **Category:** media
- **Supports:** align (full, wide), anchor, color (background, gradients, heading, link, text), interactivity (clientNavigation), spacing (margin, padding), typography (fontSize, lineHeight), ~~html~~
- **Attributes:** align, allowedBlocks, focalPoint, href, imageFill, isStackedOnMobile, linkClass, linkDestination, linkTarget, mediaAlt, mediaId, mediaLink, mediaPosition, mediaSizeSlug, mediaType, mediaUrl, mediaWidth, rel, verticalAlignment
- **Attributes:** align, allowedBlocks, focalPoint, href, imageFill, isStackedOnMobile, linkClass, linkDestination, linkTarget, mediaAlt, mediaId, mediaLink, mediaPosition, mediaSizeSlug, mediaType, mediaUrl, mediaWidth, rel, useFeaturedImage, verticalAlignment

## Unsupported

Expand Down
2 changes: 1 addition & 1 deletion lib/blocks.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ function gutenberg_reregister_core_block_types() {
'html',
'list',
'list-item',
'media-text',
'missing',
'more',
'nextpage',
Expand Down Expand Up @@ -78,6 +77,7 @@ function gutenberg_reregister_core_block_types() {
'latest-comments.php' => 'core/latest-comments',
'latest-posts.php' => 'core/latest-posts',
'loginout.php' => 'core/loginout',
'media-text.php' => 'core/media-text',
'navigation.php' => 'core/navigation',
'navigation-link.php' => 'core/navigation-link',
'navigation-submenu.php' => 'core/navigation-submenu',
Expand Down
5 changes: 5 additions & 0 deletions packages/block-library/src/media-text/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,13 @@
},
"allowedBlocks": {
"type": "array"
},
"useFeaturedImage": {
"type": "boolean",
"default": false
}
},
"usesContext": [ "postId", "postType" ],
"supports": {
"anchor": true,
"align": [ "wide", "full" ],
Expand Down
99 changes: 80 additions & 19 deletions packages/block-library/src/media-text/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {
} from '@wordpress/components';
import { isBlobURL, getBlobTypeByURL } from '@wordpress/blob';
import { pullLeft, pullRight } from '@wordpress/icons';
import { store as coreStore } from '@wordpress/core-data';
import { useEntityProp, store as coreStore } from '@wordpress/core-data';

/**
* Internal dependencies
Expand Down Expand Up @@ -127,7 +127,12 @@ function attributesFromMedia( {
};
}

function MediaTextEdit( { attributes, isSelected, setAttributes } ) {
function MediaTextEdit( {
attributes,
isSelected,
setAttributes,
context: { postId, postType },
} ) {
const {
focalPoint,
href,
Expand All @@ -145,9 +150,42 @@ function MediaTextEdit( { attributes, isSelected, setAttributes } ) {
rel,
verticalAlignment,
allowedBlocks,
useFeaturedImage,
} = attributes;
const mediaSizeSlug = attributes.mediaSizeSlug || DEFAULT_MEDIA_SIZE_SLUG;

const [ featuredImage ] = useEntityProp(
'postType',
postType,
'featured_media',
postId
);

const featuredImageMedia = useSelect(
( select ) =>
featuredImage &&
select( coreStore ).getMedia( featuredImage, { context: 'view' } ),
[ featuredImage ]
);

const featuredImageURL = useFeaturedImage
? featuredImageMedia?.source_url
: '';
const featuredImageAlt = useFeaturedImage
? featuredImageMedia?.alt_text
: '';
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we check for featuredImageURL and featuredImageAlt as bool in many places can we set them to false if they have no value? IMO the way JS says false || '' is false is better if we default to testing for false? Is there any downside?

Alternatively we could make hasFeaturedImageAlt and hasFeaturedImageURL, respectively. set to !== '', then use these in bool checks later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried this, but it meant that the alt text was set to the string "false" instead of being empty when:

  • The featured image is not used.
  • The current media has no alt text of its own.

It shows the text "false" inside the alt text option in the editor, and in the attribute.

So this condition on line 316 also needs to be updated.
value={ mediaAlt || featuredImageAlt }

Copy link
Contributor Author

@carolinan carolinan Feb 1, 2024

Choose a reason for hiding this comment

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

@draganescu I think I did not fully understand what to do here.

Copy link
Contributor

Choose a reason for hiding this comment

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

It shows the text "false" inside the alt text option in the editor, and in the attribute.

It means we need the empty string, In which case it means we should not check for value={ mediaAlt || featuredImageAlt } later but instead for value={ mediaAlt || featuredImageAlt !== '' }. That's it :)


const toggleUseFeaturedImage = () => {
setAttributes( {
imageFill: false,
mediaType: 'image',
Copy link
Contributor

Choose a reason for hiding this comment

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

If we set mediaType, an img element with no attributes will be saved. This markup is invalid because the img element requires at least a src attribute.

media-text

However, I do know that the img tag needs to be present in the markup in order to dynamically assign the src attribute during dynamic rendering. I am wondering if there is any good approach to solve this problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

By setting mediaType to undefined, we will be able to remove the img tag that does not have attributes from the saved markup. Instead, I believe that it's possible to inject an img tag in the dynamic rendering when useFeaturedImage is true.

We should be able to use preg_replace to inject the img element as a string right after the opening tag of the figure element. This approach is also used in the Image block. In the future, if the HTML API becomes able to inject arbitrary elements, it will be possible to refactor it at that time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you.
I'm trying, but I can't get it to work, the image tag is only rended on the front. I must be missing a step, or there is something that we did not consider.
I will push the changes though so they can be tested.

mediaId: undefined,
mediaUrl: undefined,
mediaAlt: undefined,
useFeaturedImage: ! useFeaturedImage,
} );
};

const { imageSizes, image } = useSelect(
( select ) => {
const { getSettings } = select( blockEditorStore );
Expand Down Expand Up @@ -261,25 +299,30 @@ function MediaTextEdit( { attributes, isSelected, setAttributes } ) {
}
/>
) }
{ imageFill && mediaUrl && mediaType === 'image' && (
<FocalPointPicker
__nextHasNoMarginBottom
__next40pxDefaultSize
label={ __( 'Focal point' ) }
url={ mediaUrl }
value={ focalPoint }
onChange={ ( value ) =>
setAttributes( { focalPoint: value } )
}
onDragStart={ imperativeFocalPointPreview }
onDrag={ imperativeFocalPointPreview }
/>
) }
{ mediaType === 'image' && (
{ imageFill &&
( mediaUrl || featuredImageURL ) &&
mediaType === 'image' && (
<FocalPointPicker
__nextHasNoMarginBottom
label={ __( 'Focal point' ) }
url={
useFeaturedImage && featuredImageURL
? featuredImageURL
: mediaUrl
}
value={ focalPoint }
onChange={ ( value ) =>
setAttributes( { focalPoint: value } )
}
onDragStart={ imperativeFocalPointPreview }
onDrag={ imperativeFocalPointPreview }
/>
) }
{ mediaType === 'image' && ( mediaUrl || featuredImageURL ) && (
<TextareaControl
__nextHasNoMarginBottom
label={ __( 'Alternative text' ) }
value={ mediaAlt }
value={ mediaAlt || featuredImageAlt }
draganescu marked this conversation as resolved.
Show resolved Hide resolved
onChange={ onMediaAltChange }
help={
<>
Expand All @@ -303,6 +346,16 @@ function MediaTextEdit( { attributes, isSelected, setAttributes } ) {
) }
/>
) }
{ ( mediaUrl || featuredImageURL ) && (
<RangeControl
__nextHasNoMarginBottom
label={ __( 'Media width' ) }
value={ temporaryMediaWidth || mediaWidth }
onChange={ commitWidthChange }
min={ WIDTH_CONSTRAINT_PERCENTAGE }
max={ 100 - WIDTH_CONSTRAINT_PERCENTAGE }
/>
) }
</PanelBody>
);

Expand Down Expand Up @@ -353,7 +406,11 @@ function MediaTextEdit( { attributes, isSelected, setAttributes } ) {
onChangeUrl={ onSetHref }
linkDestination={ linkDestination }
mediaType={ mediaType }
mediaUrl={ image && image.source_url }
mediaUrl={
useFeaturedImage && featuredImageURL
? featuredImageURL
: image && image.source_url
}
mediaLink={ image && image.link }
linkTarget={ linkTarget }
linkClass={ linkClass }
Expand All @@ -370,6 +427,7 @@ function MediaTextEdit( { attributes, isSelected, setAttributes } ) {
commitWidthChange={ commitWidthChange }
ref={ refMediaContainer }
enableResize={ blockEditingMode === 'default' }
toggleUseFeaturedImage={ toggleUseFeaturedImage }
{ ...{
focalPoint,
imageFill,
Expand All @@ -381,6 +439,9 @@ function MediaTextEdit( { attributes, isSelected, setAttributes } ) {
mediaType,
mediaUrl,
mediaWidth,
useFeaturedImage,
featuredImageURL,
featuredImageAlt,
} }
/>
{ mediaPosition !== 'right' && <div { ...innerBlocksProps } /> }
Expand Down
8 changes: 7 additions & 1 deletion packages/block-library/src/media-text/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,17 @@
width: 100% !important;
}

.wp-block-media-text.is-image-fill .editor-media-container__resizer {
.wp-block-media-text.is-image-fill .editor-media-container__resizer,
.wp-block-media-text.is-image-fill .components-placeholder.has-illustration {
// The resizer sets an inline height but for the image fill we set it to full height.
height: 100% !important;
}

.wp-block-media-text > .block-editor-block-list__layout > .block-editor-block-list__block {
max-width: unset;
}

/* Make the featured image placeholder the same height as the media selection area. */
.wp-block-media-text--placeholder-image {
min-height: 205px;
carolinan marked this conversation as resolved.
Show resolved Hide resolved
}
66 changes: 66 additions & 0 deletions packages/block-library/src/media-text/index.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<?php
/**
* Server-side rendering of the `core/media-text` block.
*
* @package WordPress
*/

/**
* Renders the `core/media-text` block on server.
*
* @param array $attributes The block attributes.
* @param string $content The block rendered content.
*
* @return string Returns the Media & Text block markup, if useFeaturedImage is true.
*/
function render_block_core_media_text( $attributes, $content ) {
if ( false === $attributes['useFeaturedImage'] ) {
return $content;
}

if ( in_the_loop() ) {
update_post_thumbnail_cache();
}

$current_featured_image = get_the_post_thumbnail_url();
if ( ! $current_featured_image ) {
return $content;
}

$image_tag = '<figure class="wp-block-media-text__media"><img>';
$content = preg_replace( '/<figure\s+class="wp-block-media-text__media">/', $image_tag, $content );

$processor = new WP_HTML_Tag_Processor( $content );
if ( isset( $attributes['imageFill'] ) && $attributes['imageFill'] ) {
$position = '50% 50%';
if ( isset( $attributes['focalPoint'] ) ) {
$position = round( $attributes['focalPoint']['x'] * 100 ) . '% ' . round( $attributes['focalPoint']['y'] * 100 ) . '%';
}
$processor->next_tag( 'figure' );
$processor->set_attribute( 'style', 'background-image:url(' . esc_url( $current_featured_image ) . ');background-position:' . $position . ';' );
}
$processor->next_tag( 'img' );
$media_size_slug = 'full';
if ( isset( $attributes['mediaSizeSlug'] ) ) {
$media_size_slug = $attributes['mediaSizeSlug'];
}
$processor->set_attribute( 'src', esc_url( $current_featured_image ) );
$processor->set_attribute( 'class', 'wp-image-' . get_post_thumbnail_id() . ' size-' . $media_size_slug );

$content = $processor->get_updated_html();

return $content;
}

/**
* Registers the `core/media-text` block renderer on server.
*/
function register_block_core_media_text() {
register_block_type_from_metadata(
__DIR__ . '/media-text',
array(
'render_callback' => 'render_block_core_media_text',
)
);
}
add_action( 'init', 'register_block_core_media_text' );
Loading
Loading