From 96a2ebdfc16d8cc667b64c1bcbb89bc1a8070747 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 21 Jun 2024 03:56:37 -0700 Subject: [PATCH 01/10] Rename yieldToMain to splitTask and export from @wordpress/interactivity (#62665) * Export yieldToMain from @wordpress/interactivity * Rename yieldToMain to splitTask Co-authored-by: tunetheweb * Update docs to note splitTask being exported --------- Co-authored-by: tunetheweb Co-authored-by: westonruter Co-authored-by: luisherranz Co-authored-by: cbravobernal --- .../interactivity-api/api-reference.md | 5 +++-- packages/interactivity/src/directives.tsx | 12 +++--------- packages/interactivity/src/index.ts | 1 + packages/interactivity/src/init.ts | 6 +++--- packages/interactivity/src/utils.ts | 2 +- 5 files changed, 11 insertions(+), 15 deletions(-) diff --git a/docs/reference-guides/interactivity-api/api-reference.md b/docs/reference-guides/interactivity-api/api-reference.md index f0a61677bd8290..5f11ab20e59bf2 100644 --- a/docs/reference-guides/interactivity-api/api-reference.md +++ b/docs/reference-guides/interactivity-api/api-reference.md @@ -813,7 +813,8 @@ const { state } = store("myPlugin", { As mentioned above with [`wp-on`](#wp-on), [`wp-on-window`](#wp-on-window), and [`wp-on-document`](#wp-on-document), an async action should be used whenever the `async` versions of the aforementioned directives cannot be used due to the action requiring synchronous access to the `event` object. Synchronous access is reqired whenever the action needs to call `event.preventDefault()`, `event.stopPropagation()`, or `event.stopImmediatePropagation()`. To ensure that the action code does not contribute to a long task, you may manually yield to the main thread after calling the synchronous event API. For example: ```js -function toMainThread() { +// Note: In WordPress 6.6 this splitTask function is exported by @wordpress/interactivity. +function splitTask() { return new Promise(resolve => { setTimeout(resolve, 0); }); @@ -823,7 +824,7 @@ store("myPlugin", { actions: { handleClick: function* (event) { event.preventDefault(); - yield toMainThread(); + yield splitTask(); doTheWork(); }, }, diff --git a/packages/interactivity/src/directives.tsx b/packages/interactivity/src/directives.tsx index a013d442166897..fa59965607946f 100644 --- a/packages/interactivity/src/directives.tsx +++ b/packages/interactivity/src/directives.tsx @@ -11,13 +11,7 @@ import { deepSignal, peek, type DeepSignal } from 'deepsignal'; /** * Internal dependencies */ -import { - useWatch, - useInit, - kebabToCamelCase, - warn, - yieldToMain, -} from './utils'; +import { useWatch, useInit, kebabToCamelCase, warn, splitTask } from './utils'; import type { DirectiveEntry } from './hooks'; import { directive, getScope, getEvaluate } from './hooks'; @@ -246,7 +240,7 @@ const getGlobalAsyncEventDirective = ( type: 'window' | 'document' ) => { const eventName = entry.suffix.split( '--', 1 )[ 0 ]; useInit( () => { const cb = async ( event: Event ) => { - await yieldToMain(); + await splitTask(); evaluate( entry, event ); }; const globalVar = type === 'window' ? window : document; @@ -361,7 +355,7 @@ export default () => { existingHandler( event ); } entries.forEach( async ( entry ) => { - await yieldToMain(); + await splitTask(); evaluate( entry, event ); } ); }; diff --git a/packages/interactivity/src/index.ts b/packages/interactivity/src/index.ts index 3c91e919d91bdc..a43534509bb5ac 100644 --- a/packages/interactivity/src/index.ts +++ b/packages/interactivity/src/index.ts @@ -25,6 +25,7 @@ export { useLayoutEffect, useCallback, useMemo, + splitTask, } from './utils'; export { useState, useRef } from 'preact/hooks'; diff --git a/packages/interactivity/src/init.ts b/packages/interactivity/src/init.ts index b3376a7510c13a..ddf6785d4dfdf4 100644 --- a/packages/interactivity/src/init.ts +++ b/packages/interactivity/src/init.ts @@ -6,7 +6,7 @@ import { hydrate, type ContainerNode, type ComponentChild } from 'preact'; * Internal dependencies */ import { toVdom, hydratedIslands } from './vdom'; -import { createRootFragment, yieldToMain } from './utils'; +import { createRootFragment, splitTask } from './utils'; import { directivePrefix } from './constants'; // Keep the same root fragment for each interactive region node. @@ -35,11 +35,11 @@ export const init = async () => { for ( const node of nodes ) { if ( ! hydratedIslands.has( node ) ) { - await yieldToMain(); + await splitTask(); const fragment = getRegionRootFragment( node ); const vdom = toVdom( node ); initialVdom.set( node, vdom ); - await yieldToMain(); + await splitTask(); hydrate( vdom, fragment ); } } diff --git a/packages/interactivity/src/utils.ts b/packages/interactivity/src/utils.ts index b4975228e3069b..3c880d43f17065 100644 --- a/packages/interactivity/src/utils.ts +++ b/packages/interactivity/src/utils.ts @@ -54,7 +54,7 @@ const afterNextFrame = ( callback: () => void ) => { * * @return Promise */ -export const yieldToMain = () => { +export const splitTask = () => { return new Promise( ( resolve ) => { // TODO: Use scheduler.yield() when available. setTimeout( resolve, 0 ); From 308b7ff4120ab822ca67e97c8ee3e8d97859bf09 Mon Sep 17 00:00:00 2001 From: Micha Krapp Date: Fri, 21 Jun 2024 14:35:33 +0200 Subject: [PATCH 02/10] fix link url (#62725) Co-authored-by: michakrapp Co-authored-by: juanmaguitar --- docs/getting-started/tutorial.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/getting-started/tutorial.md b/docs/getting-started/tutorial.md index 641ecad07ab9bc..aac507d4c573f4 100644 --- a/docs/getting-started/tutorial.md +++ b/docs/getting-started/tutorial.md @@ -33,7 +33,7 @@ If you don't have one or more of these items, the [Block Development Environment The first step in creating the Copyright Date Block is to scaffold the initial block structure using the [`@wordpress/create-block`](https://developer.wordpress.org/block-editor/reference-guides/packages/packages-create-block/) package.
- Review the Get started with create-block documentation for an introduction to using this package. + Review the Get started with create-block documentation for an introduction to using this package.
You can use `create-block` from just about any directory (folder) on your computer and then use `wp-env` to create a local WordPress development environment with your new block plugin installed and activated. From 368015281c536800bc4e8cf0730af657ee9891ec Mon Sep 17 00:00:00 2001 From: Artemio Morales Date: Fri, 21 Jun 2024 14:44:29 +0200 Subject: [PATCH 03/10] Query Loop block: Clarify explanation around query loop variation example (#62605) * Clarify what is needed to get the example working * Add additional clarifying text Co-authored-by: artemiomorales Co-authored-by: juanmaguitar --- .../extending-the-query-loop-block.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/docs/how-to-guides/block-tutorial/extending-the-query-loop-block.md b/docs/how-to-guides/block-tutorial/extending-the-query-loop-block.md index bf48c5db4e9bb9..d4134749891ca1 100644 --- a/docs/how-to-guides/block-tutorial/extending-the-query-loop-block.md +++ b/docs/how-to-guides/block-tutorial/extending-the-query-loop-block.md @@ -12,9 +12,14 @@ By registering your own block variation with some specific Query Loop block sett With the block variations API you can provide the default settings that make the most sense for your use-case. +In order to have a Query Loop variation properly working, we'll need to: +- Register the block variation for the `core/query` block with some default values +- Define a layout for the block variation +- Use the `namespace` attribute in the `isActive` block variation property + Let's go on a journey, for example, of setting up a variation for a plugin which registers a `book` [custom post type](https://developer.wordpress.org/plugins/post-types/). -### Offer sensible defaults +### 1. Offer sensible defaults Your first step would be to create a variation which will be set up in such a way to provide a block variation which will display by default a list of books instead of blog posts. The full variation code will look something like this: @@ -91,7 +96,9 @@ In this way, your block will show up just like any other block while the user is At this point, your custom variation will be virtually indistinguishable from a stand-alone block. Completely branded to your plugin, easy to discover and directly available to the user as a drop in. -### Customize your variation layout +However, your query loop variation won't work just yet — we still need to define a layout so that it can render properly. + +### 2. Customize your variation layout Please note that the Query Loop block supports `'block'` as a string in the `scope` property. In theory, that's to allow the variation to be picked up after inserting the block itself. Read more about the Block Variation Picker [here](https://github.com/WordPress/gutenberg/blob/HEAD/packages/block-editor/src/components/block-variation-picker/README.md). @@ -125,7 +132,7 @@ In order for a variation to be connected to another Query Loop variation we need For example, if we have a Query Loop variation exposed to the inserter(`scope: ['inserter']`) with the name `products`, we can connect a scoped `block` variation by setting its `namespace` attribute to `['products']`. If the user selects this variation after having clicked `Start blank`, the namespace attribute will be overridden by the main inserter variation. -### Making Gutenberg recognize your variation +### 3. Making Gutenberg recognize your variation There is one slight problem you might have realized after implementing this variation: while it is transparent to the user as they are inserting it, Gutenberg will still recognize the variation as a Query Loop block at its core and so, after its insertion, it will show up as a Query Loop block in the tree view of the editor, for instance. From 26b7a12f4beeec3fb13dad3ffc51958f25cb021b Mon Sep 17 00:00:00 2001 From: Ivan Ottinger Date: Fri, 21 Jun 2024 15:09:55 +0200 Subject: [PATCH 04/10] Set `.editor-post-publish-panel`'s top prop to `$admin-bar-height-big` (#62736) Co-authored-by: ivan-ottinger Co-authored-by: youknowriad --- packages/editor/src/components/post-publish-panel/style.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/editor/src/components/post-publish-panel/style.scss b/packages/editor/src/components/post-publish-panel/style.scss index ca5639aa969124..c319924086b0e4 100644 --- a/packages/editor/src/components/post-publish-panel/style.scss +++ b/packages/editor/src/components/post-publish-panel/style.scss @@ -196,7 +196,7 @@ position: fixed; z-index: z-index(".editor-post-publish-panel"); background: $white; - top: 0; + top: $admin-bar-height-big; bottom: 0; right: 0; left: 0; From b0a160b247b96a52c2fbccb4864459166fcefc8f Mon Sep 17 00:00:00 2001 From: Marcelo Serpa <81248+fullofcaffeine@users.noreply.github.com> Date: Fri, 21 Jun 2024 08:00:19 -0600 Subject: [PATCH 05/10] CustomSelectControlV2 legacy adapter: fix handling of options extra attributes (#62255) * Pass the an object with the expected interface to the `selectedItem` property of the `changeObject` that's passed to the `onChange` callback for the legacy adapter * Remove unrelated test for this part * Remove debug code * Add test to V1 to confirm it supports passing custom properties as part of an option * Update V2 legacy adapter test to test that the comppnent supports custom attributes as part of option and that they are not added to the DOM as attributes (same behavior as V1) * Type `LegacyOption` to support arbitrary attributes * Remove re-export for the adapter for now * Remove non-nullable operator in favour of an early return * Remove extra space * Set default false value via prop destructuring * Use a valid HTML attribute in CustomSelectControl v2 unit tests * Update unit tests * Remove stable export * Export v2 legacy adapter as private API * CHANGELOG * Allow v1 Storybook to log onChange args in the actions tab for easier debugging * Do not export legacy adapter as private API just yet * Shorter test name --------- Co-authored-by: fullofcaffeine Co-authored-by: ciampo Co-authored-by: tyxla --- packages/components/CHANGELOG.md | 5 +- .../legacy-component/index.tsx | 18 +++--- .../legacy-component/test/index.tsx | 62 +++++++++++++++++-- .../src/custom-select-control-v2/types.ts | 1 + .../stories/index.story.tsx | 35 ++++++++++- .../src/custom-select-control/test/index.js | 51 +++++++++++++++ 6 files changed, 153 insertions(+), 19 deletions(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index bc4c7da0734d1f..9a595827ffb0c8 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -4,11 +4,12 @@ ### Enhancements -- DropZone: rewrite animation without depending on framer-motion. ([#62044](https://github.com/WordPress/gutenberg/pull/62044)) +- `DropZone`: rewrite animation without depending on framer-motion. ([#62044](https://github.com/WordPress/gutenberg/pull/62044)) ### Internal -- CustomSelectControl: align unit tests for v1 and legacy v2 versions. ([#62706](https://github.com/WordPress/gutenberg/pull/62706)) +- `CustomSelectControl`: align unit tests for v1 and legacy v2 versions. ([#62706](https://github.com/WordPress/gutenberg/pull/62706)) +- `CustomSelectControl`: fix handling of extra option attributes in the `onChange` callbacks and when forwarding them to the option DOM elements. ([#62255](https://github.com/WordPress/gutenberg/pull/62255)) ## 28.1.0 (2024-06-15) diff --git a/packages/components/src/custom-select-control-v2/legacy-component/index.tsx b/packages/components/src/custom-select-control-v2/legacy-component/index.tsx index 43f102e6ee0493..089d55c9a0a06c 100644 --- a/packages/components/src/custom-select-control-v2/legacy-component/index.tsx +++ b/packages/components/src/custom-select-control-v2/legacy-component/index.tsx @@ -14,7 +14,7 @@ import * as Styled from '../styles'; function CustomSelectControl( props: LegacyCustomSelectProps ) { const { - __experimentalShowSelectedHint, + __experimentalShowSelectedHint = false, __next40pxDefaultSize = false, describedBy, options, @@ -27,7 +27,11 @@ function CustomSelectControl( props: LegacyCustomSelectProps ) { // Forward props + store from v2 implementation const store = Ariakit.useSelectStore( { async setValue( nextValue ) { - if ( ! onChange ) { + const nextOption = options.find( + ( item ) => item.name === nextValue + ); + + if ( ! onChange || ! nextOption ) { return; } @@ -42,10 +46,7 @@ function CustomSelectControl( props: LegacyCustomSelectProps ) { ), inputValue: '', isOpen: state.open, - selectedItem: { - name: nextValue as string, - key: nextValue as string, - }, + selectedItem: nextOption, type: '', }; onChange( changeObject ); @@ -53,7 +54,7 @@ function CustomSelectControl( props: LegacyCustomSelectProps ) { } ); const children = options.map( - ( { name, key, __experimentalHint, ...rest } ) => { + ( { name, key, __experimentalHint, style, className } ) => { const withHint = ( { name } @@ -68,7 +69,8 @@ function CustomSelectControl( props: LegacyCustomSelectProps ) { key={ key } value={ name } children={ __experimentalHint ? withHint : name } - { ...rest } + style={ style } + className={ className } /> ); } diff --git a/packages/components/src/custom-select-control-v2/legacy-component/test/index.tsx b/packages/components/src/custom-select-control-v2/legacy-component/test/index.tsx index 02680e76cdb81b..06af287a142367 100644 --- a/packages/components/src/custom-select-control-v2/legacy-component/test/index.tsx +++ b/packages/components/src/custom-select-control-v2/legacy-component/test/index.tsx @@ -46,6 +46,16 @@ const legacyProps = { name: 'aquamarine', style: customStyles, }, + { + key: 'color3', + name: 'tomato (with custom props)', + className: customClassName, + style: customStyles, + // try passing a valid HTML attribute + 'aria-label': 'test label', + // try adding a custom prop + customPropFoo: 'foo', + }, ], }; @@ -289,8 +299,7 @@ describe.each( [ expect.objectContaining( { inputValue: '', isOpen: false, - // TODO: key should be different — this is a known bug and will be fixed - selectedItem: { key: 'violets', name: 'violets' }, + selectedItem: { key: 'flower1', name: 'violets' }, type: '', } ) ); @@ -333,8 +342,7 @@ describe.each( [ 1, expect.objectContaining( { selectedItem: expect.objectContaining( { - // TODO: key should be different — this is a known bug and will be fixed - key: 'violets', + key: 'flower1', name: 'violets', } ), } ) @@ -347,14 +355,56 @@ describe.each( [ 2, expect.objectContaining( { selectedItem: expect.objectContaining( { - // TODO: key should be different — this is a known bug and will be fixed - key: 'poppy', + key: 'flower3', name: 'poppy', } ), } ) ); } ); + it( "Should pass arbitrary props to onChange's selectedItem, but apply only style and className to DOM elements", async () => { + const onChangeMock = jest.fn(); + + render( ); + + const currentSelectedItem = screen.getByRole( 'combobox', { + expanded: false, + } ); + + await click( currentSelectedItem ); + + const optionWithCustomAttributes = screen.getByRole( 'option', { + name: 'tomato (with custom props)', + } ); + + // Assert that the option element does not have the custom attributes + expect( optionWithCustomAttributes ).not.toHaveAttribute( + 'customPropFoo' + ); + expect( optionWithCustomAttributes ).not.toHaveAttribute( + 'aria-label' + ); + + await click( optionWithCustomAttributes ); + + // NOTE: legacy CustomSelectControl doesn't fire onChange + // on first render, and so at this point in time `onChangeMock` + // would be called only once. + expect( onChangeMock ).toHaveBeenCalledTimes( 2 ); + expect( onChangeMock ).toHaveBeenCalledWith( + expect.objectContaining( { + selectedItem: expect.objectContaining( { + key: 'color3', + name: 'tomato (with custom props)', + className: customClassName, + style: customStyles, + 'aria-label': 'test label', + customPropFoo: 'foo', + } ), + } ) + ); + } ); + describe( 'Keyboard behavior and accessibility', () => { // skip reason: legacy v2 doesn't currently implement this behavior it.skip( 'Captures the keypress event and does not let it propagate', async () => { diff --git a/packages/components/src/custom-select-control-v2/types.ts b/packages/components/src/custom-select-control-v2/types.ts index 5540a533c09d45..12b41ba54f4a20 100644 --- a/packages/components/src/custom-select-control-v2/types.ts +++ b/packages/components/src/custom-select-control-v2/types.ts @@ -79,6 +79,7 @@ type LegacyOption = { style?: React.CSSProperties; className?: string; __experimentalHint?: string; + [ key: string ]: any; }; /** diff --git a/packages/components/src/custom-select-control/stories/index.story.tsx b/packages/components/src/custom-select-control/stories/index.story.tsx index a27ad6b3894b6f..8ff9a023c5821b 100644 --- a/packages/components/src/custom-select-control/stories/index.story.tsx +++ b/packages/components/src/custom-select-control/stories/index.story.tsx @@ -3,6 +3,11 @@ */ import type { StoryFn } from '@storybook/react'; +/** + * WordPress dependencies + */ +import { useState } from '@wordpress/element'; + /** * Internal dependencies */ @@ -20,10 +25,34 @@ export default { type: 'radio', }, }, + onChange: { control: { type: null } }, + value: { control: { type: null } }, + }, + parameters: { + actions: { argTypesRegex: '^on.*' }, }, }; -export const Default: StoryFn = CustomSelectControl.bind( {} ); +const Template: StoryFn< typeof CustomSelectControl > = ( props ) => { + const [ value, setValue ] = useState( props.options[ 0 ] ); + + const onChange: React.ComponentProps< + typeof CustomSelectControl + >[ 'onChange' ] = ( changeObject: { selectedItem: any } ) => { + setValue( changeObject.selectedItem ); + props.onChange?.( changeObject ); + }; + + return ( + + ); +}; + +export const Default: StoryFn = Template.bind( {} ); Default.args = { label: 'Label', options: [ @@ -51,7 +80,7 @@ Default.args = { ], }; -export const WithLongLabels: StoryFn = CustomSelectControl.bind( {} ); +export const WithLongLabels: StoryFn = Template.bind( {} ); WithLongLabels.args = { ...Default.args, options: [ @@ -70,7 +99,7 @@ WithLongLabels.args = { ], }; -export const WithHints: StoryFn = CustomSelectControl.bind( {} ); +export const WithHints: StoryFn = Template.bind( {} ); WithHints.args = { ...Default.args, options: [ diff --git a/packages/components/src/custom-select-control/test/index.js b/packages/components/src/custom-select-control/test/index.js index 867571650ed4e7..2eb855c15b51f9 100644 --- a/packages/components/src/custom-select-control/test/index.js +++ b/packages/components/src/custom-select-control/test/index.js @@ -46,6 +46,16 @@ const props = { name: 'aquamarine', style: customStyles, }, + { + key: 'color3', + name: 'tomato (with custom props)', + className: customClassName, + style: customStyles, + // try passing a valid HTML attribute + 'aria-label': 'test label', + // try adding a custom prop + customPropFoo: 'foo', + }, ], }; @@ -346,6 +356,47 @@ describe.each( [ ); } ); + it( "Should pass arbitrary props to onChange's selectedItem, but apply only style and className to DOM elements", async () => { + const user = userEvent.setup(); + const onChangeMock = jest.fn(); + + render( ); + + const currentSelectedItem = screen.getByRole( 'button', { + expanded: false, + } ); + + await user.click( currentSelectedItem ); + + const optionWithCustomAttributes = screen.getByRole( 'option', { + name: 'tomato (with custom props)', + } ); + + // Assert that the option element does not have the custom attributes + expect( optionWithCustomAttributes ).not.toHaveAttribute( + 'customPropFoo' + ); + expect( optionWithCustomAttributes ).not.toHaveAttribute( + 'aria-label' + ); + + await user.click( optionWithCustomAttributes ); + + expect( onChangeMock ).toHaveBeenCalledTimes( 1 ); + expect( onChangeMock ).toHaveBeenCalledWith( + expect.objectContaining( { + selectedItem: expect.objectContaining( { + key: 'color3', + name: 'tomato (with custom props)', + className: customClassName, + style: customStyles, + 'aria-label': 'test label', + customPropFoo: 'foo', + } ), + } ) + ); + } ); + describe( 'Keyboard behavior and accessibility', () => { it( 'Captures the keypress event and does not let it propagate', async () => { const user = userEvent.setup(); From f8a8a28ea46e796ece68581df967330bf065b1c2 Mon Sep 17 00:00:00 2001 From: Jorge Costa Date: Fri, 21 Jun 2024 16:08:06 +0200 Subject: [PATCH 06/10] Adds comment on blocks resource referencing wp_block post type. (#62722) Co-authored-by: jorgefilipecosta Co-authored-by: carolinan --- packages/edit-site/src/components/add-new-pattern/index.js | 1 + packages/patterns/src/components/pattern-convert-button.js | 1 + .../reusable-blocks-menu-items/reusable-block-convert-button.js | 1 + 3 files changed, 3 insertions(+) diff --git a/packages/edit-site/src/components/add-new-pattern/index.js b/packages/edit-site/src/components/add-new-pattern/index.js index 02758cb0bc4861..1bb9a13c646e2c 100644 --- a/packages/edit-site/src/components/add-new-pattern/index.js +++ b/packages/edit-site/src/components/add-new-pattern/index.js @@ -55,6 +55,7 @@ export default function AddNewPattern() { ?.add_new_item, addNewTemplatePartLabel: getPostType( TEMPLATE_PART_POST_TYPE ) ?.labels?.add_new_item, + // Blocks refers to the wp_block post type, this checks the ability to create a post of that type. canCreatePattern: canUser( 'create', 'blocks' ), canCreateTemplatePart: canUser( 'create', 'template-parts' ), }; diff --git a/packages/patterns/src/components/pattern-convert-button.js b/packages/patterns/src/components/pattern-convert-button.js index dd7d15b8d33bc5..1db406601898b7 100644 --- a/packages/patterns/src/components/pattern-convert-button.js +++ b/packages/patterns/src/components/pattern-convert-button.js @@ -85,6 +85,7 @@ export default function PatternConvertButton( { hasBlockSupport( block.name, 'reusable', true ) ) && // Hide when current doesn't have permission to do that. + // Blocks refers to the wp_block post type, this checks the ability to create a post of that type. !! canUser( 'create', 'blocks' ); return _canConvert; diff --git a/packages/reusable-blocks/src/components/reusable-blocks-menu-items/reusable-block-convert-button.js b/packages/reusable-blocks/src/components/reusable-blocks-menu-items/reusable-block-convert-button.js index 1a7293b6a55cb0..8cc892b6a84e6e 100644 --- a/packages/reusable-blocks/src/components/reusable-blocks-menu-items/reusable-block-convert-button.js +++ b/packages/reusable-blocks/src/components/reusable-blocks-menu-items/reusable-block-convert-button.js @@ -92,6 +92,7 @@ export default function ReusableBlockConvertButton( { hasBlockSupport( block.name, 'reusable', true ) ) && // Hide when current doesn't have permission to do that. + // Blocks refers to the wp_block post type, this checks the ability to create a post of that type. !! canUser( 'create', 'blocks' ); return _canConvert; From 33bc0da421ea4de9f3aecaaf01d0bb9f23d726b9 Mon Sep 17 00:00:00 2001 From: Jorge Costa Date: Fri, 21 Jun 2024 16:39:17 +0200 Subject: [PATCH 07/10] Fix: the trash post action doesn't take into account user capabilities. (#62589) Co-authored-by: jorgefilipecosta Co-authored-by: ellatrix Co-authored-by: youknowriad Co-authored-by: ntsekouras --- .../src/components/post-actions/actions.js | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/packages/editor/src/components/post-actions/actions.js b/packages/editor/src/components/post-actions/actions.js index 5cb208e23e7867..0034e2b2664c6a 100644 --- a/packages/editor/src/components/post-actions/actions.js +++ b/packages/editor/src/components/post-actions/actions.js @@ -3,7 +3,7 @@ */ import { external, trash, backup } from '@wordpress/icons'; import { addQueryArgs } from '@wordpress/url'; -import { useDispatch, useSelect } from '@wordpress/data'; +import { useDispatch, useSelect, useRegistry } from '@wordpress/data'; import { decodeEntities } from '@wordpress/html-entities'; import { store as coreStore } from '@wordpress/core-data'; import { __, _n, sprintf, _x } from '@wordpress/i18n'; @@ -307,6 +307,37 @@ const trashPostAction = { }, }; +function useTrashPostAction( postType ) { + const registry = useRegistry(); + const { resource, cachedCanUserResolvers } = useSelect( + ( select ) => { + const { getPostType, getCachedResolvers } = select( coreStore ); + return { + resource: getPostType( postType )?.rest_base || '', + cachedCanUserResolvers: getCachedResolvers().canUser, + }; + }, + [ postType ] + ); + return useMemo( + () => ( { + ...trashPostAction, + isEligible( item ) { + return ( + trashPostAction.isEligible( item ) && + registry + .select( coreStore ) + .canUser( 'delete', resource, item.id ) + ); + }, + } ), + // We are making this use memo depend on cachedCanUserResolvers as a way to make the component using this hook re-render + // when user capabilities are resolved. This makes sure the isEligible function is re-evaluated. + // eslint-disable-next-line react-hooks/exhaustive-deps + [ registry, resource, cachedCanUserResolvers ] + ); +} + const permanentlyDeletePostAction = { id: 'permanently-delete', label: __( 'Permanently delete' ), @@ -1020,6 +1051,7 @@ export function usePostActions( { postType, onActionPerformed, context } ) { ); const duplicatePostAction = useDuplicatePostAction( postType ); + const trashPostActionForPostType = useTrashPostAction( postType ); const isTemplateOrTemplatePart = [ TEMPLATE_POST_TYPE, TEMPLATE_PART_POST_TYPE, @@ -1048,7 +1080,7 @@ export function usePostActions( { postType, onActionPerformed, context } ) { isTemplateOrTemplatePart ? resetTemplateAction : restorePostAction, isTemplateOrTemplatePart || isPattern ? deletePostAction - : trashPostAction, + : trashPostActionForPostType, ! isTemplateOrTemplatePart && permanentlyDeletePostAction, ...defaultActions, ].filter( Boolean ); @@ -1113,6 +1145,7 @@ export function usePostActions( { postType, onActionPerformed, context } ) { isPattern, postTypeObject?.viewable, duplicatePostAction, + trashPostActionForPostType, onActionPerformed, isLoaded, supportsRevisions, From 418d8b77f1580677493d37077bdaa1b23e6879ea Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Fri, 21 Jun 2024 18:25:08 +0200 Subject: [PATCH 08/10] CustomSelectControl V2: fix passing external value/defaultValue (#62733) * Legavy v2 adapter: set value and defaultValue explicitly * Update v1 and legacy v2 tests to align assertions on onChange calls * Update unit tests * CHANGELOG * Move external controlled update tests outside of describe each * Re-use option objects when it makes sense instead of hardcoded strings --- Co-authored-by: ciampo Co-authored-by: tyxla --- packages/components/CHANGELOG.md | 3 +- .../legacy-component/index.tsx | 6 + .../legacy-component/test/index.tsx | 124 +++++++++++++----- .../src/custom-select-control/test/index.js | 99 ++++++++++---- 4 files changed, 168 insertions(+), 64 deletions(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 9a595827ffb0c8..57599320404e4b 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -9,7 +9,8 @@ ### Internal - `CustomSelectControl`: align unit tests for v1 and legacy v2 versions. ([#62706](https://github.com/WordPress/gutenberg/pull/62706)) -- `CustomSelectControl`: fix handling of extra option attributes in the `onChange` callbacks and when forwarding them to the option DOM elements. ([#62255](https://github.com/WordPress/gutenberg/pull/62255)) +- `CustomSelectControlV2`: fix handling of extra option attributes in the `onChange` callbacks and when forwarding them to the option DOM elements. ([#62255](https://github.com/WordPress/gutenberg/pull/62255)) +- `CustomSelectControlV2`: fix setting initial value and reacting to external controlled updates. ([#62733](https://github.com/WordPress/gutenberg/pull/62733)) ## 28.1.0 (2024-06-15) diff --git a/packages/components/src/custom-select-control-v2/legacy-component/index.tsx b/packages/components/src/custom-select-control-v2/legacy-component/index.tsx index 089d55c9a0a06c..41be4f58d9147f 100644 --- a/packages/components/src/custom-select-control-v2/legacy-component/index.tsx +++ b/packages/components/src/custom-select-control-v2/legacy-component/index.tsx @@ -51,6 +51,12 @@ function CustomSelectControl( props: LegacyCustomSelectProps ) { }; onChange( changeObject ); }, + value: value?.name, + // Setting the first option as a default value when no value is provided + // is already done natively by the underlying Ariakit component, + // but doing this explicitly avoids the `onChange` callback from firing + // on initial render, thus making this implementation closer to the v1. + defaultValue: options[ 0 ]?.name, } ); const children = options.map( diff --git a/packages/components/src/custom-select-control-v2/legacy-component/test/index.tsx b/packages/components/src/custom-select-control-v2/legacy-component/test/index.tsx index 06af287a142367..f0b699617cd03c 100644 --- a/packages/components/src/custom-select-control-v2/legacy-component/test/index.tsx +++ b/packages/components/src/custom-select-control-v2/legacy-component/test/index.tsx @@ -64,7 +64,7 @@ const ControlledCustomSelectControl = ( { onChange: onChangeProp, ...restProps }: React.ComponentProps< typeof UncontrolledCustomSelectControl > ) => { - const [ value, setValue ] = useState( options[ 0 ] ); + const [ value, setValue ] = useState( restProps.value ?? options[ 0 ] ); const onChange: typeof onChangeProp = ( changeObject ) => { onChangeProp?.( changeObject ); @@ -83,12 +83,87 @@ const ControlledCustomSelectControl = ( { ); }; +it( 'Should apply external controlled updates', async () => { + const mockOnChange = jest.fn(); + const { rerender } = render( + + ); + + const currentSelectedItem = screen.getByRole( 'combobox', { + expanded: false, + } ); + + expect( currentSelectedItem ).toHaveTextContent( + legacyProps.options[ 0 ].name + ); + + expect( mockOnChange ).not.toHaveBeenCalled(); + + rerender( + + ); + + expect( currentSelectedItem ).toHaveTextContent( + legacyProps.options[ 1 ].name + ); + + // Necessary to wait for onChange to potentially fire + await sleep(); + + expect( mockOnChange ).not.toHaveBeenCalled(); +} ); + describe.each( [ [ 'Uncontrolled', UncontrolledCustomSelectControl ], [ 'Controlled', ControlledCustomSelectControl ], ] )( 'CustomSelectControl (%s)', ( ...modeAndComponent ) => { const [ , Component ] = modeAndComponent; + it( 'Should select the first option when no explicit initial value is passed without firing onChange', async () => { + const mockOnChange = jest.fn(); + render( ); + + expect( + screen.getByRole( 'combobox', { + expanded: false, + } ) + ).toHaveTextContent( legacyProps.options[ 0 ].name ); + + // Necessary to wait for onChange to potentially fire + await sleep(); + + expect( mockOnChange ).not.toHaveBeenCalled(); + } ); + + it( 'Should pick the initially selected option if the value prop is passed without firing onChange', async () => { + const mockOnChange = jest.fn(); + render( + + ); + + expect( + screen.getByRole( 'combobox', { + expanded: false, + } ) + ).toHaveTextContent( legacyProps.options[ 3 ].name ); + + // Necessary to wait for onChange to potentially fire + await sleep(); + + expect( mockOnChange ).not.toHaveBeenCalled(); + } ); + it( 'Should replace the initial selection when a new item is selected', async () => { render( ); @@ -292,33 +367,21 @@ describe.each( [ } ) ); - // NOTE: legacy CustomSelectControl doesn't fire onChange - // at this point in time. - expect( mockOnChange ).toHaveBeenNthCalledWith( - 1, - expect.objectContaining( { - inputValue: '', - isOpen: false, - selectedItem: { key: 'flower1', name: 'violets' }, - type: '', - } ) - ); - await click( screen.getByRole( 'option', { name: 'aquamarine', } ) ); - expect( mockOnChange ).toHaveBeenNthCalledWith( - 2, + expect( mockOnChange ).toHaveBeenCalledTimes( 1 ); + expect( mockOnChange ).toHaveBeenLastCalledWith( expect.objectContaining( { inputValue: '', isOpen: false, selectedItem: expect.objectContaining( { name: 'aquamarine', } ), - type: '', + type: expect.any( String ), } ) ); } ); @@ -336,23 +399,11 @@ describe.each( [ } ) ).toHaveFocus(); - // NOTE: legacy CustomSelectControl doesn't fire onChange - // at this point in time. - expect( mockOnChange ).toHaveBeenNthCalledWith( - 1, - expect.objectContaining( { - selectedItem: expect.objectContaining( { - key: 'flower1', - name: 'violets', - } ), - } ) - ); - await type( 'p' ); await press.Enter(); - expect( mockOnChange ).toHaveBeenNthCalledWith( - 2, + expect( mockOnChange ).toHaveBeenCalledTimes( 1 ); + expect( mockOnChange ).toHaveBeenLastCalledWith( expect.objectContaining( { selectedItem: expect.objectContaining( { key: 'flower3', @@ -387,10 +438,7 @@ describe.each( [ await click( optionWithCustomAttributes ); - // NOTE: legacy CustomSelectControl doesn't fire onChange - // on first render, and so at this point in time `onChangeMock` - // would be called only once. - expect( onChangeMock ).toHaveBeenCalledTimes( 2 ); + expect( onChangeMock ).toHaveBeenCalledTimes( 1 ); expect( onChangeMock ).toHaveBeenCalledWith( expect.objectContaining( { selectedItem: expect.objectContaining( { @@ -454,7 +502,9 @@ describe.each( [ await press.ArrowDown(); await press.Enter(); - expect( currentSelectedItem ).toHaveTextContent( 'crimson clover' ); + expect( currentSelectedItem ).toHaveTextContent( + legacyProps.options[ 1 ].name + ); } ); it( 'Should be able to type characters to select matching options', async () => { @@ -488,7 +538,9 @@ describe.each( [ await sleep(); await press.Tab(); expect( currentSelectedItem ).toHaveFocus(); - expect( currentSelectedItem ).toHaveTextContent( 'violets' ); + expect( currentSelectedItem ).toHaveTextContent( + legacyProps.options[ 0 ].name + ); // Ideally we would test a multi-character typeahead, but anything more than a single character is flaky await type( 'a' ); diff --git a/packages/components/src/custom-select-control/test/index.js b/packages/components/src/custom-select-control/test/index.js index 2eb855c15b51f9..4a19449ac80730 100644 --- a/packages/components/src/custom-select-control/test/index.js +++ b/packages/components/src/custom-select-control/test/index.js @@ -64,7 +64,7 @@ const ControlledCustomSelectControl = ( { onChange: onChangeProp, ...restProps } ) => { - const [ value, setValue ] = useState( options[ 0 ] ); + const [ value, setValue ] = useState( restProps.value ?? options[ 0 ] ); const onChange = ( changeObject ) => { onChangeProp?.( changeObject ); @@ -81,12 +81,72 @@ const ControlledCustomSelectControl = ( { ); }; +it( 'Should apply external controlled updates', async () => { + const mockOnChange = jest.fn(); + const { rerender } = render( + + ); + + const currentSelectedItem = screen.getByRole( 'button', { + expanded: false, + } ); + + expect( currentSelectedItem ).toHaveTextContent( props.options[ 0 ].name ); + + rerender( + + ); + + expect( currentSelectedItem ).toHaveTextContent( props.options[ 1 ].name ); + + expect( mockOnChange ).not.toHaveBeenCalled(); +} ); + describe.each( [ - [ 'uncontrolled', UncontrolledCustomSelectControl ], - [ 'controlled', ControlledCustomSelectControl ], + [ 'Uncontrolled', UncontrolledCustomSelectControl ], + [ 'Controlled', ControlledCustomSelectControl ], ] )( 'CustomSelectControl %s', ( ...modeAndComponent ) => { const [ , Component ] = modeAndComponent; + it( 'Should select the first option when no explicit initial value is passed without firing onChange', () => { + const mockOnChange = jest.fn(); + render( ); + + expect( + screen.getByRole( 'button', { + expanded: false, + } ) + ).toHaveTextContent( props.options[ 0 ].name ); + + expect( mockOnChange ).not.toHaveBeenCalled(); + } ); + + it( 'Should pick the initially selected option if the value prop is passed without firing onChange', async () => { + const mockOnChange = jest.fn(); + render( + + ); + + expect( + screen.getByRole( 'button', { + expanded: false, + } ) + ).toHaveTextContent( props.options[ 3 ].name ); + + expect( mockOnChange ).not.toHaveBeenCalled(); + } ); + it( 'Should replace the initial selection when a new item is selected', async () => { const user = userEvent.setup(); @@ -285,13 +345,7 @@ describe.each( [ const user = userEvent.setup(); const mockOnChange = jest.fn(); - render( - - ); + render( ); await user.click( screen.getByRole( 'button', { @@ -299,32 +353,21 @@ describe.each( [ } ) ); - // DIFFERENCE WITH V2: NOT CALLED - // expect( mockOnChange ).toHaveBeenNthCalledWith( - // 1, - // expect.objectContaining( { - // inputValue: '', - // isOpen: false, - // selectedItem: { key: 'flower1', name: 'violets' }, - // type: '', - // } ) - // ); - await user.click( screen.getByRole( 'option', { name: 'aquamarine', } ) ); - expect( mockOnChange ).toHaveBeenNthCalledWith( - 1, + expect( mockOnChange ).toHaveBeenCalledTimes( 1 ); + expect( mockOnChange ).toHaveBeenLastCalledWith( expect.objectContaining( { inputValue: '', isOpen: false, selectedItem: expect.objectContaining( { name: 'aquamarine', } ), - type: '__item_click__', + type: expect.any( String ), } ) ); } ); @@ -345,8 +388,8 @@ describe.each( [ await user.keyboard( 'p' ); await user.keyboard( '{enter}' ); - expect( mockOnChange ).toHaveBeenNthCalledWith( - 1, + expect( mockOnChange ).toHaveBeenCalledTimes( 1 ); + expect( mockOnChange ).toHaveBeenLastCalledWith( expect.objectContaining( { selectedItem: expect.objectContaining( { key: 'flower3', @@ -446,7 +489,9 @@ describe.each( [ await user.keyboard( '{arrowdown}' ); await user.keyboard( '{enter}' ); - expect( currentSelectedItem ).toHaveTextContent( 'crimson clover' ); + expect( currentSelectedItem ).toHaveTextContent( + props.options[ 1 ].name + ); } ); it( 'Should be able to type characters to select matching options', async () => { From 417a837eacc1346704a49d3cd453be6cf01a3243 Mon Sep 17 00:00:00 2001 From: Jorge Costa Date: Fri, 21 Jun 2024 18:48:01 +0200 Subject: [PATCH 09/10] Update: Reverse backport changes on post type REST API changes. (#62751) Co-authored-by: jorgefilipecosta Co-authored-by: ellatrix --- lib/rest-api.php | 16 ++++++++-------- .../src/components/add-new-page/index.js | 17 +++++++++-------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/lib/rest-api.php b/lib/rest-api.php index cd208caec38695..ea87f424637048 100644 --- a/lib/rest-api.php +++ b/lib/rest-api.php @@ -42,13 +42,13 @@ function gutenberg_register_wp_rest_post_types_controller_fields() { array( 'get_callback' => function ( $item ) { $post_type = get_post_type_object( $item['slug'] ); - if ( ! empty( $post_type ) && ! empty( $post_type->template ) ) { - return $post_type->template; + if ( ! empty( $post_type ) ) { + return $post_type->template ?? array(); } }, 'schema' => array( 'type' => 'array', - 'description' => __( 'The block template associated with the post type, if it exists.', 'gutenberg' ), + 'description' => __( 'The block template associated with the post type.', 'gutenberg' ), 'readonly' => true, 'context' => array( 'view', 'edit', 'embed' ), ), @@ -60,14 +60,14 @@ function gutenberg_register_wp_rest_post_types_controller_fields() { array( 'get_callback' => function ( $item ) { $post_type = get_post_type_object( $item['slug'] ); - if ( ! empty( $post_type ) && ! empty( $post_type->template_lock ) && false !== $post_type->template_lock ) { - return $post_type->template_lock; + if ( ! empty( $post_type ) ) { + return ! empty( $post_type->template_lock ) ? $post_type->template_lock : false; } }, 'schema' => array( - 'type' => 'string', - 'enum' => array( 'all', 'insert', 'contentOnly' ), - 'description' => __( 'The template_lock associated with the post type, if any.', 'gutenberg' ), + 'type' => array( 'string', 'boolean' ), + 'enum' => array( 'all', 'insert', 'contentOnly', false ), + 'description' => __( 'The template_lock associated with the post type, or false if none.', 'gutenberg' ), 'readonly' => true, 'context' => array( 'view', 'edit', 'embed' ), ), diff --git a/packages/edit-site/src/components/add-new-page/index.js b/packages/edit-site/src/components/add-new-page/index.js index 9a227faccebcd3..41f5bf9bf9a5f2 100644 --- a/packages/edit-site/src/components/add-new-page/index.js +++ b/packages/edit-site/src/components/add-new-page/index.js @@ -42,14 +42,15 @@ export default function AddNewPageModal( { onSave, onClose } ) { status: 'draft', title, slug: title || __( 'No title' ), - content: !! pagePostType.template - ? serialize( - synchronizeBlocksWithTemplate( - [], - pagePostType.template - ) - ) - : undefined, + content: + !! pagePostType.template && pagePostType.template.length + ? serialize( + synchronizeBlocksWithTemplate( + [], + pagePostType.template + ) + ) + : undefined, }, { throwOnError: true } ); From 950f5c8239b6e78e9051ec5e845bac5aa863c4cb Mon Sep 17 00:00:00 2001 From: Ella <4710635+ellatrix@users.noreply.github.com> Date: Fri, 21 Jun 2024 20:44:28 +0300 Subject: [PATCH 10/10] Raw handling: fix too aggressive list removal (#62622) Co-authored-by: ellatrix Co-authored-by: saulyz Co-authored-by: jorgefilipecosta --- packages/blocks/src/api/raw-handling/list-reducer.js | 3 --- .../blocks/src/api/raw-handling/test/list-reducer.js | 6 ------ .../fixtures/documents/mixed-content-out.html | 12 +++++++++++- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/blocks/src/api/raw-handling/list-reducer.js b/packages/blocks/src/api/raw-handling/list-reducer.js index b7fce3d805b83e..f771a8215636bd 100644 --- a/packages/blocks/src/api/raw-handling/list-reducer.js +++ b/packages/blocks/src/api/raw-handling/list-reducer.js @@ -53,9 +53,6 @@ export default function listReducer( node ) { if ( prevListItem ) { prevListItem.appendChild( list ); parentList.removeChild( parentListItem ); - } else { - parentList.parentNode.insertBefore( list, parentList ); - parentList.parentNode.removeChild( parentList ); } } diff --git a/packages/blocks/src/api/raw-handling/test/list-reducer.js b/packages/blocks/src/api/raw-handling/test/list-reducer.js index cb89e5c45e6a48..5c1692dd5ebf33 100644 --- a/packages/blocks/src/api/raw-handling/test/list-reducer.js +++ b/packages/blocks/src/api/raw-handling/test/list-reducer.js @@ -41,12 +41,6 @@ describe( 'listReducer', () => { expect( deepFilterHTML( input, [ listReducer ] ) ).toEqual( output ); } ); - it( 'should remove empty list wrappers', () => { - const input = '
  • \n
    • test
    \n
'; - const output = '
  • test
'; - expect( deepFilterHTML( input, [ listReducer ] ) ).toEqual( output ); - } ); - it( 'should not remove filled list wrappers', () => { const input = '
  • \ntest\n
    • test
    \n
'; expect( deepFilterHTML( input, [ listReducer ] ) ).toEqual( input ); diff --git a/test/integration/fixtures/documents/mixed-content-out.html b/test/integration/fixtures/documents/mixed-content-out.html index 9c317a3b8edbf2..fc285c38c21023 100644 --- a/test/integration/fixtures/documents/mixed-content-out.html +++ b/test/integration/fixtures/documents/mixed-content-out.html @@ -1,3 +1,13 @@

Some heading

- \ No newline at end of file + + + +
    +
  • + + + +
  • Content we need to preserve
    • +
+ \ No newline at end of file