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.

Sign up for GitHub

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

Jump to bottom

List View: Add media previews to list view for gallery and image blocks #53381

Merged
merged 5 commits into from
Aug 10, 2023
Merged

List View: Add media previews to list view for gallery and image blocks #53381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8bcde10
List View: Add images
andrewserong Aug 7, 2023
291a6fe
Add some styling and a max number of images
andrewserong Aug 8, 2023
94d746f
Tweak styling
andrewserong Aug 9, 2023
334fa25
Add image count
andrewserong Aug 9, 2023
39819e2
Reduce to 3 images, remove image count
andrewserong Aug 10, 2023
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
19 changes: 19 additions & 0 deletions packages/block-editor/src/components/list-view/block-select-button.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import useBlockDisplayTitle from '../block-title/use-block-display-title';
import ListViewExpander from './expander';
import { useBlockLock } from '../block-lock';
import { store as blockEditorStore } from '../../store';
import useListViewImages from './use-list-view-images';

function ListViewBlockSelectButton(
{
Expand Down Expand Up @@ -63,6 +64,7 @@ function ListViewBlockSelectButton(
const { removeBlocks } = useDispatch( blockEditorStore );
const isMatch = useShortcutEventMatch();
const isSticky = blockInformation?.positionType === 'sticky';
const images = useListViewImages( { clientId, isExpanded } );

const positionLabel = blockInformation?.positionLabel
? sprintf(
Expand Down Expand Up @@ -184,6 +186,23 @@ function ListViewBlockSelectButton(
</span>
</Tooltip>
) }
{ images.length ? (
<span
className="block-editor-list-view-block-select-button__images"
aria-hidden
>
{ images.map( ( image, index ) => (
<span
className="block-editor-list-view-block-select-button__image"
ramonjd marked this conversation as resolved.
Show resolved Hide resolved
key={ `img-${ image.url }` }
style={ {
backgroundImage: `url(${ image.url })`,
zIndex: images.length - index, // Ensure the first image is on top, and subsequent images are behind.
} }
/>
) ) }
</span>
) : null }
{ isLocked && (
<span className="block-editor-list-view-block-select-button__lock">
<Icon icon={ lock } />
Expand Down
25 changes: 25 additions & 0 deletions packages/block-editor/src/components/list-view/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,31 @@
.block-editor-list-view-block-select-button__sticky {
line-height: 0;
}

.block-editor-list-view-block-select-button__images {
display: flex;
}

.block-editor-list-view-block-select-button__image {
background-size: cover;
width: 20px;
height: 20px;
border-radius: $radius-block-ui;

&:not(:only-child) {
box-shadow: 0 0 0 $radius-block-ui $white;
}

&:not(:first-child) {
margin-left: -5px;
}
}

&.is-selected .block-editor-list-view-block-select-button__image {
&:not(:only-child) {
box-shadow: 0 0 0 $radius-block-ui var(--wp-admin-theme-color);
}
}
}

.block-editor-list-view-block__contents-cell,
Expand Down
79 changes: 79 additions & 0 deletions packages/block-editor/src/components/list-view/use-list-view-images.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/**
* WordPress dependencies
*/
import { useMemo } from '@wordpress/element';
import { useSelect } from '@wordpress/data';

/**
* Internal dependencies
*/
import { store as blockEditorStore } from '../../store';

// Maximum number of images to display in a list view row.
const MAX_IMAGES = 3;

function getImageUrl( block ) {
Copy link
Member

Choose a reason for hiding this comment

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

Not for this PR, but it's be good to think through an API that goes beyond hard-coding core blocks and builds similarly to the custom label support. For example, it might be a side effect of attribute types, so that a Cover block can show an image if one is set as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, thanks for taking an early look! Yes, I've just hard-coded things in at the moment while I hack around, but it'd be preferable in the longer-term to have a consistent API for it.

I imagine we'd likely keep things fairly hard-coded for the first round, but thinking through an API could be handy so that custom blocks / plugins can have image previews, too.

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, one step at a time :)

Copy link
Member

Choose a reason for hiding this comment

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

Maybe in addition to the save and edit functions there could be a list function or something that returns JSX that appears in the List View. This could replace __experimentalLabel.

list( { attributes } ) {
	return (
		<Row>
			__( 'Image' )
			<ImagePreview attributes={ attributes } />
		</Row>
	);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hrm, interesting idea! One thing we'd probably want to be careful about is that it's quite a restricted area visually, so we might not want custom blocks to arbitrarily add elements, but rather provide data that the UI already knows how to display without anything overflowing or breaking out of the available space? There isn't very much real estate available, and the text truncation, etc for __experimentalLabel is handled automatically by the list view rather than individual blocks having to worry about it.

Copy link
Member

Choose a reason for hiding this comment

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

It's a similar problem to BlockToolbar. In theory you can put anything inside a <BlockControls> and it will appear in the toolbar but we trust that extenders use the components we provide that are designed for this purpose e.g. ToolbarButton.

In fact the problems are so similar that maybe instead of a list() function we just have ListViewControls which is rendered by edit() 😀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact the problems are so similar that maybe instead of a list() function we just have ListViewControls which is rendered by edit() 😀

Yeah, something like that could work. Though I'd imagine that would likely be in addition to a label function? __experimentalLabel works for other contexts than just the list view, since it winds up being called internally by useBlockDisplayTitle.

In any case, definitely worth hacking around with in follow-ups to see if it's more ergonomic for the block code to decide what's rendered, or for the block code to provide data that the list view renders in an opinionated way.

if ( block.name !== 'core/image' ) {
return;
}

if ( block.attributes?.url ) {
return { url: block.attributes.url, alt: block.attributes.alt };
}
}

function getImagesFromGallery( block ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to expand this to other container blocks that may have images, such as Group or Column? (Not necessarily as part of this PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes — I was thinking we'd start with Gallery + Image for now, and look at other blocks such as Cover, etc in a follow-up where we can look at how an API for it might work, as in the other discussion above.

I like the idea of Group, Column, or other container blocks also showing what's in them, that's a cool idea! And if the API discussion winds up being a bit complex, then it'll be a simple follow-up to add hard-coded rules to allow Group, Column, etc in, too.

Copy link
Member

Choose a reason for hiding this comment

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

We can try it, but it may be more distracting than necessary to have images also visible in parent group/columns blocks, and not just image/media related blocks.

Copy link
Member

Choose a reason for hiding this comment

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

I'd keep it just for galleries and blocks that can have an image as an attribute (Cover, Media and Text).

Copy link
Member

Choose a reason for hiding this comment

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

Wrote a follow-up issue to add support for other image-attribute-based blocks: #53684

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, that sounds good to me 👍

if ( block.name !== 'core/gallery' || ! block.innerBlocks ) {
ramonjd marked this conversation as resolved.
Show resolved Hide resolved
return [];
}

const images = [];

for ( const innerBlock of block.innerBlocks ) {
const img = getImageUrl( innerBlock );
if ( img ) {
images.push( img );
}
if ( images.length >= MAX_IMAGES ) {
return images;
}
}

return images;
}

function getImagesFromBlock( block, isExpanded ) {
const img = getImageUrl( block );
if ( img ) {
return [ img ];
}
return isExpanded ? [] : getImagesFromGallery( block );
}

/**
* Get a block's preview images for display within a list view row.
*
* TODO: Currently only supports images from the core/image and core/gallery
* blocks. This should be expanded to support other blocks that have images,
* potentially via an API that blocks can opt into / provide their own logic.
*
* @param {Object} props Hook properties.
* @param {string} props.clientId The block's clientId.
* @param {boolean} props.isExpanded Whether or not the block is expanded in the list view.
* @return {Array} Images.
*/
export default function useListViewImages( { clientId, isExpanded } ) {
const { block } = useSelect(
( select ) => {
const _block = select( blockEditorStore ).getBlock( clientId );
return { block: _block };
},
[ clientId ]
);

const images = useMemo( () => {
return getImagesFromBlock( block, isExpanded );
}, [ block, isExpanded ] );

return images;
}
Loading