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 StyleBook to use updated Composite implementation #55344

Merged
merged 6 commits into from
Nov 24, 2023
Merged

Migrating StyleBook to use updated Composite implementation #55344

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
abab9fc
Migrating `StyleBook`
Oct 13, 2023
4006205
Merge branch 'trunk' into 55224/migrate-StyleBook
Nov 16, 2023
5775677
Adding scroll margins
Nov 16, 2023
c1e4fa5
Fixing composite when context changes
Nov 17, 2023
c21998c
Removing `activeId` check in favour of `key` assignment
Nov 17, 2023
fea8b96
Hardcoding scroll margins
Nov 24, 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
83 changes: 48 additions & 35 deletions packages/edit-site/src/components/style-book/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,10 @@ import classnames from 'classnames';
* WordPress dependencies
*/
import {
__unstableComposite as Composite,
__unstableUseCompositeState as useCompositeState,
__unstableCompositeItem as CompositeItem,
Disabled,
TabPanel,
privateApis as componentsPrivateApis,
} from '@wordpress/components';

import { __, sprintf } from '@wordpress/i18n';
import {
getCategories,
Expand Down Expand Up @@ -43,6 +40,12 @@ const { ExperimentalBlockEditorProvider, useGlobalStyle } = unlock(
blockEditorPrivateApis
);

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

// The content area of the Style Book is rendered within an iframe so that global styles
// are applied to elements within the entire content area. To support elements that are
// not part of the block previews, such as headings and layout for the block previews,
Expand All @@ -66,6 +69,8 @@ const STYLE_BOOK_IFRAME_STYLES = `
padding: 16px;
width: 100%;
box-sizing: border-box;
scroll-margin-top: 32px;
scroll-margin-bottom: 32px;
}

.edit-site-style-book__example.is-selected {
Expand Down Expand Up @@ -332,19 +337,22 @@ const StyleBookBody = ( {
}
isSelected={ isSelected }
onSelect={ onSelect }
key={ category }
/>
</Iframe>
);
};

const Examples = memo(
( { className, examples, category, label, isSelected, onSelect } ) => {
const composite = useCompositeState( { orientation: 'vertical' } );
const compositeStore = useCompositeStore( { orientation: 'vertical' } );

return (
<Composite
{ ...composite }
store={ compositeStore }
className={ className }
aria-label={ label }
role="grid"
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the rationale for changing the role to grid? Does it make sense even if every row always contains only one cell?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The grid pattern can be used to group a set of interactive elements, such as links, buttons, or checkboxes. Since only one element in the entire grid is included in the tab sequence, grouping with a grid can dramatically reduce the number of tab stops on a page. 1

In theory it should make it clearer that the buttons (as rendered by Example below) should be navigated with arrow keys rather than tab. Most screen readers will announce the number of rows too, which means users get told how many buttons are in the collection

Footnotes

  1. W3: Layout Grid pattern ↩

Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocker for this PR, but — have you considered using CompositeRow (and potentially CompositeGroup) ?

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 did, but I didn't want to move this too far away from the original implementation.

>
{ examples
.filter( ( example ) =>
Expand All @@ -354,7 +362,6 @@ const Examples = memo(
<Example
key={ example.name }
id={ `example-${ example.name }` }
composite={ composite }
title={ example.title }
blocks={ example.blocks }
isSelected={ isSelected( example.name ) }
Expand All @@ -368,7 +375,7 @@ const Examples = memo(
}
);

const Example = ( { composite, id, title, blocks, isSelected, onClick } ) => {
const Example = ( { id, title, blocks, isSelected, onClick } ) => {
const originalSettings = useSelect(
( select ) => select( blockEditorStore ).getSettings(),
[]
Expand All @@ -385,35 +392,41 @@ const Example = ( { composite, id, title, blocks, isSelected, onClick } ) => {
);

return (
<CompositeItem
{ ...composite }
className={ classnames( 'edit-site-style-book__example', {
'is-selected': isSelected,
} ) }
id={ id }
aria-label={ sprintf(
// translators: %s: Title of a block, e.g. Heading.
__( 'Open %s styles in Styles panel' ),
title
) }
onClick={ onClick }
role="button"
as="div"
>
<span className="edit-site-style-book__example-title">
{ title }
</span>
<div className="edit-site-style-book__example-preview" aria-hidden>
<Disabled className="edit-site-style-book__example-preview__content">
<ExperimentalBlockEditorProvider
value={ renderedBlocks }
settings={ settings }
<div role="row">
<div role="gridcell">
<CompositeItem
className={ classnames( 'edit-site-style-book__example', {
'is-selected': isSelected,
} ) }
id={ id }
aria-label={ sprintf(
// translators: %s: Title of a block, e.g. Heading.
__( 'Open %s styles in Styles panel' ),
title
) }
render={ <div /> }
role="button"
onClick={ onClick }
>
<span className="edit-site-style-book__example-title">
{ title }
</span>
<div
className="edit-site-style-book__example-preview"
aria-hidden
>
<BlockList renderAppender={ false } />
</ExperimentalBlockEditorProvider>
</Disabled>
<Disabled className="edit-site-style-book__example-preview__content">
<ExperimentalBlockEditorProvider
value={ renderedBlocks }
settings={ settings }
>
<BlockList renderAppender={ false } />
</ExperimentalBlockEditorProvider>
</Disabled>
</div>
</CompositeItem>
</div>
</CompositeItem>
</div>
);
};

Expand Down
Loading