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

Migrating BlockPatternSetup to use updated Composite implementation #55425

Merged
merged 6 commits into from
Nov 20, 2023
Merged

Migrating BlockPatternSetup to use updated Composite implementation #55425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
72270c1
Migrating `BlockPatternSetup`
Oct 17, 2023
1461398
Merge branch 'trunk' into 55224/migrate-BlockPatternSetup
Nov 16, 2023
01ad516
Restoring much of the original code
Nov 16, 2023
d4cb67c
Adding scroll-margin to list items
Nov 16, 2023
8969e9d
Removing redundant CSS
Nov 16, 2023
ebad31e
Using `__experimentalIsFocusable`
Nov 16, 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
60 changes: 38 additions & 22 deletions packages/block-editor/src/components/block-pattern-setup/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ import { useDispatch } from '@wordpress/data';
import { cloneBlock } from '@wordpress/blocks';
import {
VisuallyHidden,
__unstableComposite as Composite,
__unstableUseCompositeState as useCompositeState,
__unstableCompositeItem as CompositeItem,
privateApis as componentsPrivateApis,
} from '@wordpress/components';

import { useState } from '@wordpress/element';
Expand All @@ -22,6 +20,13 @@ import BlockPreview from '../block-preview';
import SetupToolbar from './setup-toolbar';
import usePatternsSetup from './use-patterns-setup';
import { VIEWMODES } from './constants';
import { unlock } from '../../lock-unlock';

const {
CompositeV2: Composite,
CompositeItemV2: CompositeItem,
useCompositeStoreV2: useCompositeStore,
} = unlock( componentsPrivateApis );

const SetupContent = ( {
viewMode,
Expand All @@ -30,8 +35,9 @@ const SetupContent = ( {
onBlockPatternSelect,
showTitles,
} ) => {
const composite = useCompositeState();
const compositeStore = useCompositeStore();
const containerClass = 'block-editor-block-pattern-setup__container';

if ( viewMode === VIEWMODES.carousel ) {
const slideClass = new Map( [
[ activeSlide, 'active-slide' ],
Expand All @@ -41,23 +47,25 @@ const SetupContent = ( {
return (
<div className="block-editor-block-pattern-setup__carousel">
<div className={ containerClass }>
<ul className="carousel-container">
<div className="carousel-container">
{ patterns.map( ( pattern, index ) => (
<BlockPatternSlide
active={ index === activeSlide }
className={ slideClass.get( index ) || '' }
key={ pattern.name }
pattern={ pattern }
/>
) ) }
</ul>
</div>
</div>
</div>
);
}

return (
<div className="block-editor-block-pattern-setup__grid">
<Composite
{ ...composite }
store={ compositeStore }
role="listbox"
className={ containerClass }
aria-label={ __( 'Patterns list' ) }
Expand All @@ -67,7 +75,6 @@ const SetupContent = ( {
key={ pattern.name }
pattern={ pattern }
onSelect={ onBlockPatternSelect }
composite={ composite }
showTitles={ showTitles }
/>
) ) }
Expand All @@ -76,24 +83,27 @@ const SetupContent = ( {
);
};

function BlockPattern( { pattern, onSelect, composite, showTitles } ) {
function BlockPattern( { pattern, onSelect, showTitles } ) {
const baseClassName = 'block-editor-block-pattern-setup-list';
const { blocks, description, viewportWidth = 700 } = pattern;
const descriptionId = useInstanceId(
BlockPattern,
`${ baseClassName }__item-description`
);
return (
<div
className={ `${ baseClassName }__list-item` }
aria-label={ pattern.title }
aria-describedby={ pattern.description ? descriptionId : undefined }
>
<div className={ `${ baseClassName }__list-item` }>
<CompositeItem
render={
<div
aria-describedby={
description ? descriptionId : undefined
}
aria-label={ pattern.title }
className={ `${ baseClassName }__item` }
/>
}
id={ `${ baseClassName }__pattern__${ pattern.name }` }
role="option"
as="div"
{ ...composite }
className={ `${ baseClassName }__item` }
onClick={ () => onSelect( blocks ) }
>
<BlockPreview
Expand All @@ -115,14 +125,16 @@ function BlockPattern( { pattern, onSelect, composite, showTitles } ) {
);
}

function BlockPatternSlide( { className, pattern, minHeight } ) {
function BlockPatternSlide( { active, className, pattern, minHeight } ) {
const { blocks, title, description } = pattern;
const descriptionId = useInstanceId(
BlockPatternSlide,
'block-editor-block-pattern-setup-list__item-description'
);
return (
<li
<div
aria-hidden={ ! active }
role="img"
className={ `pattern-slide ${ className }` }
aria-label={ title }
aria-describedby={ description ? descriptionId : undefined }
Expand All @@ -133,7 +145,7 @@ function BlockPatternSlide( { className, pattern, minHeight } ) {
{ description }
</VisuallyHidden>
) }
</li>
</div>
);
}

Expand Down Expand Up @@ -178,10 +190,14 @@ const BlockPatternSetup = ( {
activeSlide={ activeSlide }
totalSlides={ patterns.length }
handleNext={ () => {
setActiveSlide( ( active ) => active + 1 );
setActiveSlide( ( active ) =>
Math.min( active + 1, patterns.length - 1 )
);
Comment on lines +193 to +195
Copy link
Contributor Author

@andrewhayward andrewhayward Oct 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Now that previous/next buttons are no longer disabled (see CarouselNavigation), this is necessary to keep the active slide within bounds.

} }
handlePrevious={ () => {
setActiveSlide( ( active ) => active - 1 );
setActiveSlide( ( active ) =>
Math.max( active - 1, 0 )
);
Comment on lines +198 to +200
Copy link
Contributor Author

@andrewhayward andrewhayward Oct 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Now that previous/next buttons are no longer disabled (see CarouselNavigation), this is necessary to keep the active slide within bounds.

} }
onBlockPatternSelect={ () => {
onPatternSelectCallback(
Expand Down
2 changes: 2 additions & 0 deletions packages/block-editor/src/components/block-pattern-setup/setup-toolbar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,14 @@ const CarouselNavigation = ( {
label={ __( 'Previous pattern' ) }
onClick={ handlePrevious }
disabled={ activeSlide === 0 }
__experimentalIsFocusable
/>
<Button
icon={ chevronRight }
label={ __( 'Next pattern' ) }
onClick={ handleNext }
disabled={ activeSlide === totalSlides - 1 }
__experimentalIsFocusable
/>
</div>
);
Expand Down
5 changes: 4 additions & 1 deletion packages/block-editor/src/components/block-pattern-setup/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
}

.block-editor-block-pattern-setup-list__item {
scroll-margin: 5px 0;

&:hover .block-editor-block-preview__container {
box-shadow: 0 0 0 2px var(--wp-admin-theme-color);
}
Expand All @@ -44,6 +46,7 @@
color: var(--wp-admin-theme-color);
}
}

.block-editor-block-pattern-setup-list__list-item {
break-inside: avoid-column;
margin-bottom: $grid-unit-30;
Expand Down Expand Up @@ -85,7 +88,7 @@
align-items: center;
justify-content: space-between;
border-top: 1px solid $gray-300;
align-self: flex-end;
align-self: stretch;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change seems to make the toolbar more stable, preventing it from jumping around as slide height varies.


.block-editor-block-pattern-setup__display-controls {
display: flex;
Expand Down
Loading