From a006a4b6a4e4db39c422c9366a3f2a95433fac6b Mon Sep 17 00:00:00 2001 From: Benni <109149472+at-benni@users.noreply.github.com> Date: Thu, 1 Feb 2024 10:13:01 +0100 Subject: [PATCH 001/273] Update versions-in-wordpress.md (#58545) --- docs/contributors/versions-in-wordpress.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/contributors/versions-in-wordpress.md b/docs/contributors/versions-in-wordpress.md index ce9eb458e7372..a385c1ced9e43 100644 --- a/docs/contributors/versions-in-wordpress.md +++ b/docs/contributors/versions-in-wordpress.md @@ -6,6 +6,7 @@ If anything looks incorrect here, please bring it up in #core-editor in [WordPre | Gutenberg Versions | WordPress Version | | ------------------ | ----------------- | +| 16.2-16.7 | 6.4.3 | | 16.2-16.7 | 6.4.2 | | 16.2-16.7 | 6.4.1 | | 16.2-16.7 | 6.4 | From ece4d7d16b5964fa92336e3b79631e76ee88ddd4 Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Thu, 1 Feb 2024 11:38:02 +0100 Subject: [PATCH 002/273] Enable dynamic import transform for React Native tests (#58546) --- test/native/babel.config.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/native/babel.config.js b/test/native/babel.config.js index 582e6f5dc6f65..d1a840a697197 100644 --- a/test/native/babel.config.js +++ b/test/native/babel.config.js @@ -13,6 +13,7 @@ module.exports = ( api ) => { ], 'react-native-reanimated/plugin', '@babel/plugin-proposal-export-namespace-from', + '@babel/plugin-transform-dynamic-import', ], overrides: [ { From afa8b5c8b222a8c8d452e84d8b5ffc42c6027b92 Mon Sep 17 00:00:00 2001 From: Aki Hamano <54422211+t-hamano@users.noreply.github.com> Date: Thu, 1 Feb 2024 20:38:32 +0900 Subject: [PATCH 003/273] Font Library: Use Button's API to disable footer buttons (#58529) --- .../global-styles/font-library-modal/font-collection.js | 7 +++---- .../global-styles/font-library-modal/installed-fonts.js | 7 +++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/font-collection.js b/packages/edit-site/src/components/global-styles/font-library-modal/font-collection.js index 9735ce1c35a4d..4a14ee245694b 100644 --- a/packages/edit-site/src/components/global-styles/font-library-modal/font-collection.js +++ b/packages/edit-site/src/components/global-styles/font-library-modal/font-collection.js @@ -298,11 +298,10 @@ function Footer( { handleInstall, isDisabled } ) { diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/installed-fonts.js b/packages/edit-site/src/components/global-styles/font-library-modal/installed-fonts.js index cbe9927187e82..73ff17f25b9a6 100644 --- a/packages/edit-site/src/components/global-styles/font-library-modal/installed-fonts.js +++ b/packages/edit-site/src/components/global-styles/font-library-modal/installed-fonts.js @@ -186,11 +186,10 @@ function Footer( { shouldDisplayDeleteButton, handleUninstallClick } ) { ) } From 0e7576775bee6e455c950e4335b805d2707de115 Mon Sep 17 00:00:00 2001 From: Ella <4710635+ellatrix@users.noreply.github.com> Date: Thu, 1 Feb 2024 14:10:48 +0200 Subject: [PATCH 004/273] Writing flow: don't prepare for multi selection when dragging inwards (#58552) --- .../src/components/writing-flow/use-drag-selection.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/writing-flow/use-drag-selection.js b/packages/block-editor/src/components/writing-flow/use-drag-selection.js index 8d8791afe5f09..d8b113b5728d3 100644 --- a/packages/block-editor/src/components/writing-flow/use-drag-selection.js +++ b/packages/block-editor/src/components/writing-flow/use-drag-selection.js @@ -75,7 +75,13 @@ export default function useDragSelection() { } ); } - function onMouseLeave( { buttons, target } ) { + function onMouseLeave( { buttons, target, relatedTarget } ) { + // If we're moving into a child element, ignore. We're tracking + // the mouse leaving the element to a parent, no a child. + if ( target.contains( relatedTarget ) ) { + return; + } + // Avoid triggering a multi-selection if the user is already // dragging blocks. if ( isDraggingBlocks() ) { From 180ae59c81baab959eb2923e4e36204a6e77bea1 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Thu, 1 Feb 2024 17:09:42 +0400 Subject: [PATCH 005/273] Block Library: Rename variation build methods (#58538) --- packages/block-library/src/navigation-link/index.php | 5 ++--- packages/block-library/src/post-terms/index.php | 4 ++-- phpcs.xml.dist | 2 -- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/block-library/src/navigation-link/index.php b/packages/block-library/src/navigation-link/index.php index 749bae7aec64e..9a1421a663fb7 100644 --- a/packages/block-library/src/navigation-link/index.php +++ b/packages/block-library/src/navigation-link/index.php @@ -375,12 +375,11 @@ function block_core_navigation_link_unregister_variation( $name ) { } /** - * Register the navigation link block. * Returns an array of variations for the navigation link block. * * @return array */ -function build_navigation_link_block_variations() { +function block_core_navigation_link_build_variations() { // This will only handle post types and taxonomies registered until this point (init on priority 9). // See action hooks below for other post types and taxonomies. // See https://github.com/WordPress/gutenberg/issues/53826 for details. @@ -429,7 +428,7 @@ function register_block_core_navigation_link() { __DIR__ . '/navigation-link', array( 'render_callback' => 'render_block_core_navigation_link', - 'variation_callback' => 'build_navigation_link_block_variations', + 'variation_callback' => 'block_core_navigation_link_build_variations', ) ); } diff --git a/packages/block-library/src/post-terms/index.php b/packages/block-library/src/post-terms/index.php index 44291038383d4..c919db9cda2e4 100644 --- a/packages/block-library/src/post-terms/index.php +++ b/packages/block-library/src/post-terms/index.php @@ -63,7 +63,7 @@ function render_block_core_post_terms( $attributes, $content, $block ) { * * @return array The available variations for the block. */ -function build_post_term_block_variations() { +function block_core_post_terms_build_variations() { $taxonomies = get_taxonomies( array( 'publicly_queryable' => true, @@ -116,7 +116,7 @@ function register_block_core_post_terms() { __DIR__ . '/post-terms', array( 'render_callback' => 'render_block_core_post_terms', - 'variation_callback' => 'build_post_term_block_variations', + 'variation_callback' => 'block_core_post_terms_build_variations', ) ); } diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 3c45d416470bd..882eaa06b15e0 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -122,8 +122,6 @@ - - From 73a6abc3502fa33443c0ac6695140064e8268e76 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Thu, 1 Feb 2024 18:24:00 +0400 Subject: [PATCH 006/273] Components: Allow limiting the number of maximum visible Snackbars (#58559) * Components: Allow limiting the number of maximum visible Snackbars * Add changelong entry --- packages/components/CHANGELOG.md | 1 + packages/components/src/snackbar/list.tsx | 4 +++- packages/components/src/snackbar/types.ts | 1 + packages/edit-widgets/src/components/notices/index.js | 1 + packages/editor/src/components/editor-snackbars/index.js | 1 + 5 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 6080498d5e92b..628466b1583a3 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -5,6 +5,7 @@ ### Enhancements - `ConfirmDialog`: Add `__next40pxDefaultSize` to buttons ([#58421](https://github.com/WordPress/gutenberg/pull/58421)). +- `SnackbarList`: Allow limiting the number of maximum visible Snackbars ([#58559](https://github.com/WordPress/gutenberg/pull/58559)). ### Bug Fix diff --git a/packages/components/src/snackbar/list.tsx b/packages/components/src/snackbar/list.tsx index e423cf6c3a7f7..fc9f0ec580d19 100644 --- a/packages/components/src/snackbar/list.tsx +++ b/packages/components/src/snackbar/list.tsx @@ -62,12 +62,14 @@ const SNACKBAR_VARIANTS = { */ export function SnackbarList( { notices, + maxVisible, className, children, onRemove, }: WordPressComponentProps< SnackbarListProps, 'div' > ) { const listRef = useRef< HTMLDivElement | null >( null ); const isReducedMotion = useReducedMotion(); + const visibleNotices = maxVisible ? notices.slice( -maxVisible ) : notices; className = classnames( 'components-snackbar-list', className ); const removeNotice = ( notice: SnackbarListProps[ 'notices' ][ number ] ) => () => @@ -76,7 +78,7 @@ export function SnackbarList( {
{ children } - { notices.map( ( notice ) => { + { visibleNotices.map( ( notice ) => { const { content, ...restNotice } = notice; return ( diff --git a/packages/components/src/snackbar/types.ts b/packages/components/src/snackbar/types.ts index 539c4c3ebdf65..ecd7abff6d56a 100644 --- a/packages/components/src/snackbar/types.ts +++ b/packages/components/src/snackbar/types.ts @@ -38,6 +38,7 @@ export type SnackbarListProps = { content: string; } >; + maxVisible?: number; onRemove: ( id: string ) => void; children?: NoticeChildren | Array< NoticeChildren >; }; diff --git a/packages/edit-widgets/src/components/notices/index.js b/packages/edit-widgets/src/components/notices/index.js index bb02274e92871..b595d810dfc29 100644 --- a/packages/edit-widgets/src/components/notices/index.js +++ b/packages/edit-widgets/src/components/notices/index.js @@ -36,6 +36,7 @@ function Notices() { /> diff --git a/packages/editor/src/components/editor-snackbars/index.js b/packages/editor/src/components/editor-snackbars/index.js index 57f0fe4bf88af..bfdbb504cf8e9 100644 --- a/packages/editor/src/components/editor-snackbars/index.js +++ b/packages/editor/src/components/editor-snackbars/index.js @@ -18,6 +18,7 @@ export default function EditorSnackbars() { return ( From c3358a5a88ce6ec07e18beea58608ff8cb233a16 Mon Sep 17 00:00:00 2001 From: Ella <4710635+ellatrix@users.noreply.github.com> Date: Thu, 1 Feb 2024 16:33:32 +0200 Subject: [PATCH 007/273] Block editor: reduce appender sync! subscriptions (#58556) --- .../components/block-list-appender/index.js | 81 ++----------------- .../src/components/block-list/index.js | 74 +++++++++++------ 2 files changed, 56 insertions(+), 99 deletions(-) diff --git a/packages/block-editor/src/components/block-list-appender/index.js b/packages/block-editor/src/components/block-list-appender/index.js index 68f36f7dd2505..928f9fc29d170 100644 --- a/packages/block-editor/src/components/block-list-appender/index.js +++ b/packages/block-editor/src/components/block-list-appender/index.js @@ -39,77 +39,12 @@ function DefaultAppender( { rootClientId } ) { ); } -function useAppender( rootClientId, CustomAppender ) { - const isVisible = useSelect( - ( select ) => { - const { - getTemplateLock, - getSelectedBlockClientId, - __unstableGetEditorMode, - getBlockEditingMode, - } = select( blockEditorStore ); - - if ( ! CustomAppender ) { - const selectedBlockClientId = getSelectedBlockClientId(); - const isParentSelected = - rootClientId === selectedBlockClientId || - ( ! rootClientId && ! selectedBlockClientId ); - if ( ! isParentSelected ) { - return false; - } - } - - if ( - getTemplateLock( rootClientId ) || - getBlockEditingMode( rootClientId ) === 'disabled' || - __unstableGetEditorMode() === 'zoom-out' - ) { - return false; - } - - return true; - }, - [ rootClientId, CustomAppender ] - ); - - if ( ! isVisible ) { - return null; - } - - return CustomAppender ? ( - - ) : ( - - ); -} - -function BlockListAppender( { +export default function BlockListAppender( { rootClientId, - renderAppender, + CustomAppender, className, tagName: TagName = 'div', } ) { - if ( renderAppender === false ) { - return null; - } - - return ( - - ); -} - -function BlockListAppenderInner( { - rootClientId, - renderAppender, - className, - tagName: TagName, -} ) { - const appender = useAppender( rootClientId, renderAppender ); const isDragOver = useSelect( ( select ) => { const { @@ -130,10 +65,6 @@ function BlockListAppenderInner( { [ rootClientId ] ); - if ( ! appender ) { - return null; - } - return ( - { appender } + { CustomAppender ? ( + + ) : ( + + ) } ); } - -export default BlockListAppender; diff --git a/packages/block-editor/src/components/block-list/index.js b/packages/block-editor/src/components/block-list/index.js index 09a4712a3dd6d..a966c2850289d 100644 --- a/packages/block-editor/src/components/block-list/index.js +++ b/packages/block-editor/src/components/block-list/index.js @@ -151,29 +151,51 @@ export default function BlockList( settings ) { function Items( { placeholder, rootClientId, - renderAppender, + renderAppender: CustomAppender, __experimentalAppenderTagName, layout = defaultLayout, } ) { - const { order, selectedBlocks, visibleBlocks, temporarilyEditingAsBlocks } = - useSelect( - ( select ) => { - const { - getBlockOrder, - getSelectedBlockClientIds, - __unstableGetVisibleBlocks, - __unstableGetTemporarilyEditingAsBlocks, - } = select( blockEditorStore ); - return { - order: getBlockOrder( rootClientId ), - selectedBlocks: getSelectedBlockClientIds(), - visibleBlocks: __unstableGetVisibleBlocks(), - temporarilyEditingAsBlocks: - __unstableGetTemporarilyEditingAsBlocks(), - }; - }, - [ rootClientId ] - ); + // Avoid passing CustomAppender to useSelect because it could be a new + // function on every render. + const hasAppender = CustomAppender !== false; + const hasCustomAppender = !! CustomAppender; + const { + order, + selectedBlocks, + visibleBlocks, + temporarilyEditingAsBlocks, + shouldRenderAppender, + } = useSelect( + ( select ) => { + const { + getBlockOrder, + getSelectedBlockClientId, + getSelectedBlockClientIds, + __unstableGetVisibleBlocks, + __unstableGetTemporarilyEditingAsBlocks, + getTemplateLock, + getBlockEditingMode, + __unstableGetEditorMode, + } = select( blockEditorStore ); + const selectedBlockClientId = getSelectedBlockClientId(); + return { + order: getBlockOrder( rootClientId ), + selectedBlocks: getSelectedBlockClientIds(), + visibleBlocks: __unstableGetVisibleBlocks(), + temporarilyEditingAsBlocks: + __unstableGetTemporarilyEditingAsBlocks(), + shouldRenderAppender: + hasAppender && + ( hasCustomAppender + ? ! getTemplateLock( rootClientId ) && + getBlockEditingMode( rootClientId ) !== 'disabled' && + __unstableGetEditorMode() !== 'zoom-out' + : rootClientId === selectedBlockClientId || + ( ! rootClientId && ! selectedBlockClientId ) ), + }; + }, + [ rootClientId, hasAppender, hasCustomAppender ] + ); return ( @@ -199,11 +221,13 @@ function Items( { clientId={ temporarilyEditingAsBlocks } /> ) } - + { shouldRenderAppender && ( + + ) } ); } From e056237992890d6e22653d986374220b9a27a2dc Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Thu, 1 Feb 2024 15:45:04 +0100 Subject: [PATCH 008/273] RichTextData: use a private property (#58557) * RichTextData: use a private property * check-build-type-declaration-files: pass target and moduleResolution options to tsc --- .../check-build-type-declaration-files.js | 15 ++++++---- packages/rich-text/README.md | 4 +-- packages/rich-text/src/create.js | 28 +++++++------------ 3 files changed, 21 insertions(+), 26 deletions(-) diff --git a/bin/packages/check-build-type-declaration-files.js b/bin/packages/check-build-type-declaration-files.js index fff0b51e32fc2..74c7cfa5e3c2a 100644 --- a/bin/packages/check-build-type-declaration-files.js +++ b/bin/packages/check-build-type-declaration-files.js @@ -69,13 +69,16 @@ async function getDecFile( packagePath ) { async function typecheckDeclarations( file ) { return new Promise( ( resolve, reject ) => { - exec( `npx tsc --noEmit ${ file }`, ( error, stdout, stderr ) => { - if ( error ) { - reject( { file, error, stderr, stdout } ); - } else { - resolve( { file, stdout } ); + exec( + `npx tsc --target esnext --moduleResolution node --noEmit ${ file }`, + ( error, stdout, stderr ) => { + if ( error ) { + reject( { file, error, stderr, stdout } ); + } else { + resolve( { file, stdout } ); + } } - } ); + ); } ); } diff --git a/packages/rich-text/README.md b/packages/rich-text/README.md index 90fd15e1c905c..baf6da7ab6cab 100644 --- a/packages/rich-text/README.md +++ b/packages/rich-text/README.md @@ -359,8 +359,8 @@ _Returns_ The RichTextData class is used to instantiate a wrapper around rich text values, with methods that can be used to transform or manipulate the data. -- Create an emtpy instance: `new RichTextData()`. -- Create one from an html string: `RichTextData.fromHTMLString( +- Create an empty instance: `new RichTextData()`. +- Create one from an HTML string: `RichTextData.fromHTMLString( 'hello' )`. - Create one from a wrapper HTMLElement: `RichTextData.fromHTMLElement( document.querySelector( 'p' ) )`. diff --git a/packages/rich-text/src/create.js b/packages/rich-text/src/create.js index 7c0989a11dc4a..aa7911bc9083b 100644 --- a/packages/rich-text/src/create.js +++ b/packages/rich-text/src/create.js @@ -98,15 +98,12 @@ function toFormat( { tagName, attributes } ) { }; } -// Ideally we use a private property. -const RichTextInternalData = Symbol( 'RichTextInternalData' ); - /** * The RichTextData class is used to instantiate a wrapper around rich text * values, with methods that can be used to transform or manipulate the data. * - * - Create an emtpy instance: `new RichTextData()`. - * - Create one from an html string: `RichTextData.fromHTMLString( + * - Create an empty instance: `new RichTextData()`. + * - Create one from an HTML string: `RichTextData.fromHTMLString( * 'hello' )`. * - Create one from a wrapper HTMLElement: `RichTextData.fromHTMLElement( * document.querySelector( 'p' ) )`. @@ -117,6 +114,8 @@ const RichTextInternalData = Symbol( 'RichTextInternalData' ); * @todo Add methods to manipulate the data, such as applyFormat, slice etc. */ export class RichTextData { + #value; + static empty() { return new RichTextData(); } @@ -138,22 +137,15 @@ export class RichTextData { return richTextData; } constructor( init = createEmptyValue() ) { - // Setting text, formats, and replacements as enumerable properties - // unfortunately visualises these in the e2e tests. As long as the class - // instance doesn't have any enumerable properties, it will be - // visualised as a string. - Object.defineProperty( this, RichTextInternalData, { value: init } ); + this.#value = init; } toPlainText() { - return getTextContent( this[ RichTextInternalData ] ); + return getTextContent( this.#value ); } // We could expose `toHTMLElement` at some point as well, but we'd only use // it internally. toHTMLString() { - return ( - this.originalHTML || - toHTMLString( { value: this[ RichTextInternalData ] } ) - ); + return this.originalHTML || toHTMLString( { value: this.#value } ); } valueOf() { return this.toHTMLString(); @@ -168,13 +160,13 @@ export class RichTextData { return this.text.length; } get formats() { - return this[ RichTextInternalData ].formats; + return this.#value.formats; } get replacements() { - return this[ RichTextInternalData ].replacements; + return this.#value.replacements; } get text() { - return this[ RichTextInternalData ].text; + return this.#value.text; } } From 2a6b7c146d821d51ade78295dd1c1bdffcc8ced4 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Thu, 1 Feb 2024 18:56:29 +0400 Subject: [PATCH 009/273] Block Switcher: Don't use the 'useBlockDisplayInformation' hook (#58562) --- .../src/components/block-switcher/index.js | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/block-editor/src/components/block-switcher/index.js b/packages/block-editor/src/components/block-switcher/index.js index f73a5de543bd4..8a8d1908f3854 100644 --- a/packages/block-editor/src/components/block-switcher/index.js +++ b/packages/block-editor/src/components/block-switcher/index.js @@ -21,7 +21,6 @@ import { copy } from '@wordpress/icons'; * Internal dependencies */ import { store as blockEditorStore } from '../../store'; -import useBlockDisplayInformation from '../use-block-display-information'; import BlockIcon from '../block-icon'; import BlockTransformationsMenu from './block-transformations-menu'; import { useBlockVariationTransforms } from './block-variation-transformations'; @@ -162,7 +161,6 @@ function BlockSwitcherDropdownMenuContents( { } export const BlockSwitcher = ( { clientIds } ) => { - const blockInformation = useBlockDisplayInformation( clientIds?.[ 0 ] ); const { canRemove, hasBlockStyles, @@ -175,9 +173,11 @@ export const BlockSwitcher = ( { clientIds } ) => { const { getBlockRootClientId, getBlocksByClientId, + getBlockAttributes, canRemoveBlocks, } = select( blockEditorStore ); - const { getBlockStyles, getBlockType } = select( blocksStore ); + const { getBlockStyles, getBlockType, getActiveBlockVariation } = + select( blocksStore ); const _blocks = getBlocksByClientId( clientIds ); if ( ! _blocks.length || _blocks.some( ( block ) => ! block ) ) { return { invalidBlocks: true }; @@ -185,18 +185,24 @@ export const BlockSwitcher = ( { clientIds } ) => { const rootClientId = getBlockRootClientId( clientIds ); const [ { name: firstBlockName } ] = _blocks; const _isSingleBlockSelected = _blocks.length === 1; + const blockType = getBlockType( firstBlockName ); + let _icon; if ( _isSingleBlockSelected ) { - _icon = blockInformation?.icon; // Take into account active block variations. + const match = getActiveBlockVariation( + firstBlockName, + getBlockAttributes( clientIds[ 0 ] ) + ); + // Take into account active block variations. + _icon = match?.icon || blockType.icon; } else { const isSelectionOfSameType = new Set( _blocks.map( ( { name } ) => name ) ).size === 1; // When selection consists of blocks of multiple types, display an // appropriate icon to communicate the non-uniformity. - _icon = isSelectionOfSameType - ? getBlockType( firstBlockName )?.icon - : copy; + _icon = isSelectionOfSameType ? blockType.icon : copy; } + return { canRemove: canRemoveBlocks( clientIds, rootClientId ), hasBlockStyles: @@ -209,7 +215,7 @@ export const BlockSwitcher = ( { clientIds } ) => { _isSingleBlockSelected && isTemplatePart( _blocks[ 0 ] ), }; }, - [ clientIds, blockInformation?.icon ] + [ clientIds ] ); const blockTitle = useBlockDisplayTitle( { clientId: clientIds?.[ 0 ], From 726c5c62761003b92fea1d696fd4e3b62d27417f Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Thu, 1 Feb 2024 16:16:11 +0100 Subject: [PATCH 010/273] Ensure deleteAllUsers does not delete current user (#58320) --- .../e2e-test-utils-playwright/src/request-utils/users.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/e2e-test-utils-playwright/src/request-utils/users.ts b/packages/e2e-test-utils-playwright/src/request-utils/users.ts index 0997a7f51b5cb..11df5f37468ca 100644 --- a/packages/e2e-test-utils-playwright/src/request-utils/users.ts +++ b/packages/e2e-test-utils-playwright/src/request-utils/users.ts @@ -5,6 +5,7 @@ import type { RequestUtils } from './index'; export interface User { id: number; + name: string; email: string; } @@ -110,8 +111,11 @@ async function deleteAllUsers( this: RequestUtils ) { // The users endpoint doesn't support batch request yet. const responses = await Promise.all( users - // Do not delete root user. - .filter( ( user: User ) => user.id !== 1 ) + // Do not delete neither root user nor the current user. + .filter( + ( user: User ) => + user.id !== 1 && user.name !== this.user.username + ) .map( ( user: User ) => deleteUser.bind( this )( user.id ) ) ); From 89a15ccf9a99b77a04a3b5008a4f6d62c3784c7e Mon Sep 17 00:00:00 2001 From: Carlos Bravo <37012961+c4rl0sbr4v0@users.noreply.github.com> Date: Thu, 1 Feb 2024 17:35:21 +0100 Subject: [PATCH 011/273] Script Modules API: Add import map polyfill (#58263) * Add import map polyfill * Update issue number * Update WP folder for the polyfill file * Update the function with dom injection * Load it on the footer * Remove unwanted comma * Update WP Core folder to be like other polyfills * Do not load polyfill if there are no modules * Add an id to the script * Use WP default functions * Use load id * Use stable version * Sync function with Core --- .../wordpress-6.5/class-wp-script-modules.php | 27 +++++++++++++++++++ lib/compat/wordpress-6.5/scripts-modules.php | 11 ++++---- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/lib/compat/wordpress-6.5/class-wp-script-modules.php b/lib/compat/wordpress-6.5/class-wp-script-modules.php index f6a2a348f92ef..d7eaff177823c 100644 --- a/lib/compat/wordpress-6.5/class-wp-script-modules.php +++ b/lib/compat/wordpress-6.5/class-wp-script-modules.php @@ -212,10 +212,37 @@ public function print_script_module_preloads() { * Prints the import map using a script tag with a type="importmap" attribute. * * @since 6.5.0 + * + * @global WP_Scripts $wp_scripts The WP_Scripts object for printing the polyfill. */ public function print_import_map() { $import_map = $this->get_import_map(); if ( ! empty( $import_map['imports'] ) ) { + global $wp_scripts; + if ( isset( $wp_scripts ) ) { + /* + * In Core, the polyfill is registered with a different approach. + * See: https://github.com/WordPress/wordpress-develop/blob/4b23ba81ddb067110e41d05550de7f2a4f09dad3/src/wp-includes/script-loader.php#L99 + */ + wp_register_script( + 'wp-polyfill-importmap', + gutenberg_url( '/build/modules/importmap-polyfill.min.js' ), + array(), + '1.8.2', + true + ); + wp_print_inline_script_tag( + wp_get_script_polyfill( + $wp_scripts, + array( + 'HTMLScriptElement.supports && HTMLScriptElement.supports("importmap")' => 'wp-polyfill-importmap', + ) + ), + array( + 'id' => 'wp-load-polyfill-importmap', + ) + ); + } wp_print_inline_script_tag( wp_json_encode( $import_map, JSON_HEX_TAG | JSON_HEX_AMP ), array( diff --git a/lib/compat/wordpress-6.5/scripts-modules.php b/lib/compat/wordpress-6.5/scripts-modules.php index ba329b255b196..740a16581ce7f 100644 --- a/lib/compat/wordpress-6.5/scripts-modules.php +++ b/lib/compat/wordpress-6.5/scripts-modules.php @@ -20,13 +20,14 @@ * @return WP_Script_Modules The main WP_Script_Modules instance. */ function wp_script_modules(): WP_Script_Modules { - static $instance = null; - if ( is_null( $instance ) ) { - $instance = new WP_Script_Modules(); - $instance->add_hooks(); + global $wp_script_modules; + + if ( ! ( $wp_script_modules instanceof WP_Script_Modules ) ) { + $wp_script_modules = new WP_Script_Modules(); } - return $instance; + return $wp_script_modules; } + wp_script_modules()->add_hooks(); } if ( ! function_exists( 'wp_register_script_module' ) ) { From eae9748979ed20bf075f723a74896cc0fd96160b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20K=C3=A4gy?= Date: Thu, 1 Feb 2024 19:11:22 +0100 Subject: [PATCH 012/273] Fix: remove mention of weekly meeting from first time contributor PR label (#58547) --- .../lib/tasks/first-time-contributor-label/index.js | 3 +-- .../lib/tasks/first-time-contributor-label/test/index.js | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/project-management-automation/lib/tasks/first-time-contributor-label/index.js b/packages/project-management-automation/lib/tasks/first-time-contributor-label/index.js index 396a125dccee5..9c535fbbca722 100644 --- a/packages/project-management-automation/lib/tasks/first-time-contributor-label/index.js +++ b/packages/project-management-automation/lib/tasks/first-time-contributor-label/index.js @@ -65,8 +65,7 @@ async function firstTimeContributorLabel( payload, octokit ) { body: ':wave: Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @' + author + - "! In case you missed it, we'd love to have you join us in our [Slack community](https://make.wordpress.org/chat/), " + - 'where we hold [regularly weekly meetings](https://make.wordpress.org/core/tag/core-editor-summary/) open to anyone to coordinate with each other.\n\n' + + "! In case you missed it, we'd love to have you join us in our [Slack community](https://make.wordpress.org/chat/).\n\n" + 'If you want to learn more about WordPress development in general, check out the [Core Handbook](https://make.wordpress.org/core/handbook/) full of helpful information.', } ); } diff --git a/packages/project-management-automation/lib/tasks/first-time-contributor-label/test/index.js b/packages/project-management-automation/lib/tasks/first-time-contributor-label/test/index.js index 0450cb1f59b9f..0d0aa374a95a0 100644 --- a/packages/project-management-automation/lib/tasks/first-time-contributor-label/test/index.js +++ b/packages/project-management-automation/lib/tasks/first-time-contributor-label/test/index.js @@ -95,8 +95,7 @@ describe( 'firstTimeContributorLabel', () => { const expectedComment = ':wave: Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @ghost' + - "! In case you missed it, we'd love to have you join us in our [Slack community](https://make.wordpress.org/chat/)," + - ' where we hold [regularly weekly meetings](https://make.wordpress.org/core/tag/core-editor-summary/) open to anyone to coordinate with each other.\n\n' + + "! In case you missed it, we'd love to have you join us in our [Slack community](https://make.wordpress.org/chat/).\n\n" + 'If you want to learn more about WordPress development in general, check out the [Core Handbook](https://make.wordpress.org/core/handbook/) full of helpful information.'; await firstTimeContributorLabel( payload, octokit ); From 84435927fe39a5fdb34e2f1106cb5917f053dd3a Mon Sep 17 00:00:00 2001 From: Chad Chadbourne <13856531+chad1008@users.noreply.github.com> Date: Thu, 1 Feb 2024 13:43:45 -0500 Subject: [PATCH 013/273] Components: Tabs: Improve Controlled Mode Focus Handling (#57696) * limit focus update to currently selected tab * update unit tests * changelog * changelog. no, really this time. * implement feedback * implement feedback * remove testing changes in storybook * remove focus updating * update tests * update changelog * Rewrite hook to update the active element, this time using ariakit's state to track the active ID * Make sure that keyboard focus goes to the selected tab when tabbing back into the tablist * Update tests * Optimize hook dependencies, no need to rely on `selectedId` to get the active element --------- Co-authored-by: Marco Ciampini --- packages/components/CHANGELOG.md | 1 + packages/components/src/tabs/index.tsx | 42 +++-- packages/components/src/tabs/tablist.tsx | 19 +++ packages/components/src/tabs/test/index.tsx | 154 ++++++++++++------ .../various/keyboard-navigable-blocks.spec.js | 2 +- 5 files changed, 151 insertions(+), 67 deletions(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 628466b1583a3..20b64d798f403 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -18,6 +18,7 @@ - `Guide`, `Modal`: Restore accent color themability ([#58098](https://github.com/WordPress/gutenberg/pull/58098)). - `DropdownMenuV2`: Restore accent color themability ([#58130](https://github.com/WordPress/gutenberg/pull/58130)). +- `Tabs`: improve controlled mode focus handling and keyboard navigation ([#57696](https://github.com/WordPress/gutenberg/pull/57696)). - Expand theming support in the `COLORS` variable object ([#58097](https://github.com/WordPress/gutenberg/pull/58097)). ## 25.16.0 (2024-01-24) diff --git a/packages/components/src/tabs/index.tsx b/packages/components/src/tabs/index.tsx index 7f738cb9f08a9..e8e9bff76b3e9 100644 --- a/packages/components/src/tabs/index.tsx +++ b/packages/components/src/tabs/index.tsx @@ -8,7 +8,12 @@ import * as Ariakit from '@ariakit/react'; * WordPress dependencies */ import { useInstanceId } from '@wordpress/compose'; -import { useLayoutEffect, useMemo, useRef } from '@wordpress/element'; +import { + useEffect, + useLayoutEffect, + useMemo, + useRef, +} from '@wordpress/element'; /** * Internal dependencies @@ -44,8 +49,8 @@ function Tabs( { const isControlled = selectedTabId !== undefined; - const { items, selectedId } = store.useState(); - const { setSelectedId, move } = store; + const { items, selectedId, activeId } = store.useState(); + const { setSelectedId, setActiveId } = store; // Keep track of whether tabs have been populated. This is used to prevent // certain effects from firing too early while tab data and relevant @@ -154,26 +159,29 @@ function Tabs( { setSelectedId, ] ); - // In controlled mode, make sure browser focus follows the selected tab if - // the selection is changed while a tab is already being focused. - useLayoutEffect( () => { - if ( ! isControlled || ! selectOnMove ) { + useEffect( () => { + if ( ! isControlled ) { return; } - const currentItem = items.find( ( item ) => item.id === selectedId ); - const activeElement = currentItem?.element?.ownerDocument.activeElement; - const tabsHasFocus = items.some( ( item ) => { - return activeElement && activeElement === item.element; - } ); + + const focusedElement = + items?.[ 0 ]?.element?.ownerDocument.activeElement; if ( - activeElement && - tabsHasFocus && - selectedId !== activeElement.id + ! focusedElement || + ! items.some( ( item ) => focusedElement === item.element ) ) { - move( selectedId ); + return; // Return early if no tabs are focused. + } + + // If, after ariakit re-computes the active tab, that tab doesn't match + // the currently focused tab, then we force an update to ariakit to avoid + // any mismatches, especially when navigating to previous/next tab with + // arrow keys. + if ( activeId !== focusedElement.id ) { + setActiveId( focusedElement.id ); } - }, [ isControlled, items, move, selectOnMove, selectedId ] ); + }, [ activeId, isControlled, items, setActiveId ] ); const contextValue = useMemo( () => ( { diff --git a/packages/components/src/tabs/tablist.tsx b/packages/components/src/tabs/tablist.tsx index 7a53115910796..afa2d8283e6d6 100644 --- a/packages/components/src/tabs/tablist.tsx +++ b/packages/components/src/tabs/tablist.tsx @@ -28,11 +28,30 @@ export const TabList = forwardRef< return null; } const { store } = context; + + const { selectedId, activeId, selectOnMove } = store.useState(); + const { setActiveId } = store; + + const onBlur = () => { + if ( ! selectOnMove ) { + return; + } + + // When automatic tab selection is on, make sure that the active tab is up + // to date with the selected tab when leaving the tablist. This makes sure + // that the selected tab will receive keyboard focus when tabbing back into + // the tablist. + if ( selectedId !== activeId ) { + setActiveId( selectedId ); + } + }; + return ( } + onBlur={ onBlur } { ...otherProps } > { children } diff --git a/packages/components/src/tabs/test/index.tsx b/packages/components/src/tabs/test/index.tsx index 8c2c2d0fd2fa2..ca9a01928e599 100644 --- a/packages/components/src/tabs/test/index.tsx +++ b/packages/components/src/tabs/test/index.tsx @@ -1172,6 +1172,111 @@ describe( 'Tabs', () => { ).not.toBeInTheDocument(); } ); } ); + describe( 'When `selectedId` is changed by the controlling component', () => { + describe.each( [ true, false ] )( + 'and `selectOnMove` is %s', + ( selectOnMove ) => { + it( 'should continue to handle arrow key navigation properly', async () => { + const { rerender } = render( + + ); + + // Tab key should focus the currently selected tab, which is Beta. + await press.Tab(); + expect( await getSelectedTab() ).toHaveTextContent( + 'Beta' + ); + expect( await getSelectedTab() ).toHaveFocus(); + + rerender( + + ); + + // When the selected tab is changed, it should not automatically receive focus. + expect( await getSelectedTab() ).toHaveTextContent( + 'Gamma' + ); + expect( + screen.getByRole( 'tab', { name: 'Beta' } ) + ).toHaveFocus(); + + // Arrow keys should move focus to the next tab, which is Gamma + await press.ArrowRight(); + expect( + screen.getByRole( 'tab', { name: 'Gamma' } ) + ).toHaveFocus(); + } ); + + it( 'should focus the correct tab when tabbing out and back into the tablist', async () => { + const { rerender } = render( + <> + + + + ); + + // Tab key should focus the currently selected tab, which is Beta. + await press.Tab(); + await press.Tab(); + expect( await getSelectedTab() ).toHaveTextContent( + 'Beta' + ); + expect( await getSelectedTab() ).toHaveFocus(); + + rerender( + <> + + + + ); + + // When the selected tab is changed, it should not automatically receive focus. + expect( await getSelectedTab() ).toHaveTextContent( + 'Gamma' + ); + expect( + screen.getByRole( 'tab', { name: 'Beta' } ) + ).toHaveFocus(); + + // Press shift+tab, move focus to the button before Tabs + await press.ShiftTab(); + expect( + screen.getByRole( 'button', { name: 'Focus me' } ) + ).toHaveFocus(); + + // Press tab, move focus back to the tablist + await press.Tab(); + + const betaTab = screen.getByRole( 'tab', { + name: 'Beta', + } ); + const gammaTab = screen.getByRole( 'tab', { + name: 'Gamma', + } ); + + expect( + selectOnMove ? gammaTab : betaTab + ).toHaveFocus(); + } ); + } + ); + } ); describe( 'When `selectOnMove` is `true`', () => { it( 'should automatically select a newly focused tab', async () => { @@ -1188,24 +1293,6 @@ describe( 'Tabs', () => { expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); expect( await getSelectedTab() ).toHaveFocus(); } ); - it( 'should automatically update focus when the selected tab is changed by the controlling component', async () => { - const { rerender } = render( - - ); - - // Tab key should focus the currently selected tab, which is Beta. - await press.Tab(); - expect( await getSelectedTab() ).toHaveTextContent( 'Beta' ); - expect( await getSelectedTab() ).toHaveFocus(); - - rerender( - - ); - - // When the selected tab is changed, it should automatically receive focus. - expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); - expect( await getSelectedTab() ).toHaveFocus(); - } ); } ); describe( 'When `selectOnMove` is `false`', () => { it( 'should apply focus without automatically changing the selected tab', async () => { @@ -1247,37 +1334,6 @@ describe( 'Tabs', () => { await press.Enter(); expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); } ); - it( 'should not automatically update focus when the selected tab is changed by the controlling component', async () => { - const { rerender } = render( - - ); - - expect( await getSelectedTab() ).toHaveTextContent( 'Beta' ); - - // Tab key should focus the currently selected tab, which is Beta. - await press.Tab(); - await waitFor( async () => - expect( await getSelectedTab() ).toHaveFocus() - ); - - rerender( - - ); - - // When the selected tab is changed, it should not automatically receive focus. - expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); - expect( - screen.getByRole( 'tab', { name: 'Beta' } ) - ).toHaveFocus(); - } ); } ); } ); it( 'should associate each `Tab` with the correct `TabPanel`, even if they are not rendered in the same order', async () => { diff --git a/test/e2e/specs/editor/various/keyboard-navigable-blocks.spec.js b/test/e2e/specs/editor/various/keyboard-navigable-blocks.spec.js index 84536c88227ce..6b55f68897cbd 100644 --- a/test/e2e/specs/editor/various/keyboard-navigable-blocks.spec.js +++ b/test/e2e/specs/editor/various/keyboard-navigable-blocks.spec.js @@ -75,7 +75,7 @@ test.describe( 'Order of block keyboard navigation', () => { ); await page.keyboard.press( 'Tab' ); - await KeyboardNavigableBlocks.expectLabelToHaveFocus( 'Post' ); + await KeyboardNavigableBlocks.expectLabelToHaveFocus( 'Block' ); } ); test( 'allows tabbing in navigation mode if no block is selected (reverse)', async ( { From 91450bd78b2b40d3c89b9b334a11f0df1b44fd5f Mon Sep 17 00:00:00 2001 From: Jonathan Desrosiers Date: Thu, 1 Feb 2024 15:00:28 -0500 Subject: [PATCH 014/273] Add Props Bot workflow. (#58576) Co-authored-by: desrosj Co-authored-by: gziolo --- .github/workflows/props-bot.yml | 86 +++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 .github/workflows/props-bot.yml diff --git a/.github/workflows/props-bot.yml b/.github/workflows/props-bot.yml new file mode 100644 index 0000000000000..c0dacd923417a --- /dev/null +++ b/.github/workflows/props-bot.yml @@ -0,0 +1,86 @@ +name: Props Bot + +on: + # This event runs anytime a PR is (re)opened, updated, or labeled. + # GitHub does not allow filtering the `labeled` event by a specific label. + # However, the logic below will short-circuit the workflow when the `props-bot` label is not the one being added. + # Note: The pull_request_target event is uesed instead of pull_request because this workflow needs permission to comment + # on the pull request. Because this event grants extra permissions to `GITHUB_TOKEN`, any code changes within the PR + # should be considered untrusted. See https://securitylab.github.com/research/github-actions-preventing-pwn-requests/. + pull_request_target: + types: + - opened + - synchronize + - reopened + - labeled + # This event runs anytime a comment is added or deleted. + # You cannot filter this event for PR comments only. + # However, the logic below does short-circuit the workflow for issues. + issue_comment: + type: + - created + - deleted + # This event will run everytime a new PR review is initially submitted. + pull_request_review: + types: + - submitted + # This event runs anytime a PR review comment is created or deleted. + pull_request_review_comment: + types: + - created + - deleted + +# Cancels all previous workflow runs for pull requests that have not completed. +concurrency: + # The concurrency group contains the workflow name and the branch name for pull requests + # or the commit hash for any other events. + group: ${{ github.workflow }}-${{ contains( fromJSON( '["pull_request_target", "pull_request_review", "pull_request_review_comment"]' ), github.event_name ) && github.head_ref || github.sha }} + cancel-in-progress: true + +# Disable permissions for all available scopes by default. +# Any needed permissions should be configured at the job level. +permissions: {} + +jobs: + # Compiles a list of props for a pull request. + # + # Performs the following steps: + # - Collects a list of contributor props and leaves a comment. + # - Removes the props-bot label, if necessary. + props-bot: + name: Generate a list of props + runs-on: ubuntu-latest + permissions: + # The action needs permission `write` permission for PRs in order to add a comment. + pull-requests: write + contents: read + timeout-minutes: 20 + # The job should only run if: + # + # - A pull request review is created or commented on. + # - An issue comment is added to a pull request. + # - A pull request is opened, synchronized, or reopened. + # - The `props-bot` label is added to the pull request. + if: | + contains( fromJSON( '["pull_request_review", "pull_request_review_comment"]' ), github.event_name ) || + ( github.event_name == 'issue_comment' && github.event.issue.pull_request ) || + github.event_name == 'pull_request_target' && github.event.action != 'labeled' || + 'props-bot' == github.event.label.name + + steps: + - name: Gather a list of contributors + uses: WordPress/props-bot-action@trunk + + - name: Remove the props-bot label + uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 + if: ${{ github.event.action == 'labeled' && 'props-bot' == github.event.label.name }} + with: + retries: 2 + retry-exempt-status-codes: 418 + script: | + github.rest.issues.removeLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: '${{ github.event.number }}', + name: 'props-bot' + }); From 6d75b0f4e0060ac2143d78ee2fc55ee4478b5fac Mon Sep 17 00:00:00 2001 From: Matias Benedetto Date: Thu, 1 Feb 2024 20:09:49 -0300 Subject: [PATCH 015/273] Font Library: simplify font collection schema (#58574) Co-authored-by: matiasbenedetto Co-authored-by: creativecoder --- schemas/json/font-collection.json | 135 ++++++++++++++++++++++++------ 1 file changed, 109 insertions(+), 26 deletions(-) diff --git a/schemas/json/font-collection.json b/schemas/json/font-collection.json index 922569b47d940..41c4c6202fb89 100644 --- a/schemas/json/font-collection.json +++ b/schemas/json/font-collection.json @@ -2,6 +2,99 @@ "title": "JSON schema for WordPress Font Collections", "$schema": "http://json-schema.org/draft-04/schema#", "type": "object", + "definitions": { + "fontFace": { + "description": "Font face theme.json settings, with added preview property.", + "type": "object", + "properties": { + "preview": { + "description": "URL to a preview image of the font.", + "type": "string" + }, + "fontFamily": { + "description": "CSS font-family value.", + "type": "string", + "default": "" + }, + "fontStyle": { + "description": "CSS font-style value.", + "type": "string", + "default": "normal" + }, + "fontWeight": { + "description": "List of available font weights, separated by a space.", + "default": "400", + "oneOf": [ + { + "type": "string" + }, + { + "type": "integer" + } + ] + }, + "fontDisplay": { + "description": "CSS font-display value.", + "type": "string", + "default": "fallback", + "enum": [ "auto", "block", "fallback", "swap", "optional" ] + }, + "src": { + "description": "Paths or URLs to the font files.", + "oneOf": [ + { + "type": "string" + }, + { + "type": "array", + "items": { + "type": "string" + } + } + ], + "default": [] + }, + "fontStretch": { + "description": "CSS font-stretch value.", + "type": "string" + }, + "ascentOverride": { + "description": "CSS ascent-override value.", + "type": "string" + }, + "descentOverride": { + "description": "CSS descent-override value.", + "type": "string" + }, + "fontVariant": { + "description": "CSS font-variant value.", + "type": "string" + }, + "fontFeatureSettings": { + "description": "CSS font-feature-settings value.", + "type": "string" + }, + "fontVariationSettings": { + "description": "CSS font-variation-settings value.", + "type": "string" + }, + "lineGapOverride": { + "description": "CSS line-gap-override value.", + "type": "string" + }, + "sizeAdjust": { + "description": "CSS size-adjust value.", + "type": "string" + }, + "unicodeRange": { + "description": "CSS unicode-range value.", + "type": "string" + } + }, + "required": [ "fontFamily", "src" ], + "additionalProperties": false + } + }, "properties": { "$schema": { "description": "JSON schema URI for font-collection.json.", @@ -21,12 +114,12 @@ }, "font_families": { "type": "array", - "description": "Array of font families ready to be installed", + "description": "Array of font families ready to be installed.", "items": { "type": "object", "properties": { "font_family_settings": { - "description": "Font family settings similar to theme.json but without fontFace key.", + "description": "Font family theme.json settings, with added preview property.", "type": "object", "properties": { "name": { @@ -40,41 +133,31 @@ "fontFamily": { "description": "CSS font-family value.", "type": "string" - } - }, - "additionalProperties": false - }, - "font_faces": { - "description": "Array of font-face declarations.", - "type": "array", - "items": { - "type": "object", - "properties": { - "preview": { - "type": "string", - "description": "URL to a preview image of the font face" - }, - "font_face_settings": { - "description": "Font face settings as in theme.json", + }, + "preview": { + "type": "string", + "description": "URL to a preview image of the font family." + }, + "fontFace": { + "description": "Array of font-face definitions.", + "type": "array", + "items": { "allOf": [ { - "$ref": "./theme.json#/definitions/fontFace" + "$ref": "#/definitions/fontFace" } ] } } - } + }, + "additionalProperties": false }, "categories": { "type": "array", - "description": "Array of category slugs", + "description": "Array of category slugs.", "items": { "type": "string" } - }, - "preview": { - "type": "string", - "description": "URL to a preview image of the font family" } }, "required": [ "font_family_settings" ], @@ -83,7 +166,7 @@ }, "categories": { "type": "array", - "description": "Array of category objects", + "description": "Array of category objects.", "items": { "type": "object", "properties": { From 417718357da2016dcad776cba6f56da0bfccdae0 Mon Sep 17 00:00:00 2001 From: Tonya Mork Date: Thu, 1 Feb 2024 17:43:36 -0600 Subject: [PATCH 016/273] [Font Library] Update PHPUnit tests per Core coding standards and practices (#58502) * Rename test classes. Renamed test classes per WordPress Core's Test Classes coding standards: For a test covering the entire class: Tests_[GroupName]_[ClassName] For a test covering a class' method: Tests_[GroupName]_[ClassName]_[MethodName] For a function: Tests_[GroupName]_[FunctionName] See https://make.wordpress.org/core/handbook/testing/automated-testing/writing-phpunit-tests/#test-classes. * Update covers tag * Use set_up_before_class() instead of __construct(). Test classes should not have a constructor. Instead, use the set_up_before_class() fixture method to perform tasks that pre-setup all tests within the test class. * Use assertSame() instead of assertEquals(). This is an ongoing effort in Core. Reasoning is shared in the 6.5 Trac ticket: >The assertEquals() test method does not check that the types >of the expected and actual values match. This can hide subtle >bugs especially when the values are falsey. https://core.trac.wordpress.org/ticket/59655 * Rename test file to match func, class, or method. Per the WordPress Core coding standards, test files should be named according to the function, class, or method under test within its test class. Function under test: wpFunctioName.php Class under test: wpClassName.php Method under test: wpClassName/methodName.php See https://make.wordpress.org/core/handbook/testing/automated-testing/writing-phpunit-tests/#naming-and-organization. * Update dataProvider usage, formatting, and docs. Updates: * Refactors repetitive tests into a dataProvider. * Data provider's docblock. * Adds a brief explanation to each dataset (helps for understanding when a dataset fails). * Adds each argument name with a dataset. * Documents each argument in the test method's docblock. * Add assertion failure messages. When there are more than 1 assertion in a test method, a failure message helps to identify which assertion failed. Once an assertion fails, any other assertions after it in the same test method will not run. Thus, the message helps to know which one of the assertions failed. This can help with debugging process. * Reorder resets / cleanup. Resets are things like deleting posts, removing a filter/action, etc. These need to occur _before_ an assertion. Why? If a reset is after an assertion and any of the assertions before it fail, the test method bails out and the reset does not happen. To ensure resets happen, they need to be located in either: * tear_down() fixture. * just after invoking the thing under test but before the assertion(s). * Put expected as first arg in each assert. Per the PHPUnit handbook: assertSomething( expected, actual, message ); * Add fonts and font-library group annotation. All of the test classes now have these groups: * @group fonts * @group font-library These annotations allow for specific groups of tests to be run, which is helping in local development. Co-authored-by: hellofromtonya Co-authored-by: creativecoder --- .../fontFamilyBackwardsCompatibility.php | 118 ++++--- .../fonts/font-library/fontLibraryHooks.php | 13 +- .../wpFontCollection/__construct.php | 8 +- .../wpFontCollection/loadFromJson.php | 16 +- .../wpFontLibrary/getMimeTypes.php | 9 +- .../wpFontUtils/formatFontFamily.php | 6 +- .../wpFontUtils/getFontFaceSlug.php | 13 +- .../{fontsDir.php => wpFontsDir.php} | 20 +- .../wpRestFontCollectionsController.php | 23 +- .../wpRestFontFacesController.php | 269 +++++++++------ .../wpRestFontFamiliesController.php | 324 +++++++++++------- 11 files changed, 516 insertions(+), 303 deletions(-) rename phpunit/tests/fonts/font-library/{fontsDir.php => wpFontsDir.php} (80%) diff --git a/phpunit/tests/fonts/font-library/fontFamilyBackwardsCompatibility.php b/phpunit/tests/fonts/font-library/fontFamilyBackwardsCompatibility.php index a971bd5123430..4305237d53292 100644 --- a/phpunit/tests/fonts/font-library/fontFamilyBackwardsCompatibility.php +++ b/phpunit/tests/fonts/font-library/fontFamilyBackwardsCompatibility.php @@ -6,18 +6,28 @@ * * @package WordPress * @subpackage Font Library + * + * @group fonts + * @group font-library */ class Tests_Font_Family_Backwards_Compatibility extends WP_UnitTestCase { + private $post_ids_to_delete = array(); + public function set_up() { parent::set_up(); + $this->post_ids_to_delete = array(); delete_option( 'gutenberg_font_family_format_converted' ); } public function tear_down() { - parent::tear_down(); + foreach ( $this->post_ids_to_delete as $post_id ) { + wp_delete_post( $post_id, true ); + } delete_option( 'gutenberg_font_family_format_converted' ); + + parent::tear_down(); } public function test_font_faces_with_remote_src() { @@ -27,79 +37,74 @@ public function test_font_faces_with_remote_src() { gutenberg_convert_legacy_font_family_format(); - $font_family = get_post( $font_family_id ); + $font_family = $this->get_post( $font_family_id ); $font_faces = $this->get_font_faces( $font_family_id ); list( $font_face1, $font_face2, $font_face3 ) = $font_faces; // Updated font family post. - $this->assertSame( 'wp_font_family', $font_family->post_type ); - $this->assertSame( 'publish', $font_family->post_status ); + $this->assertSame( 'wp_font_family', $font_family->post_type, 'The font family post type should be wp_font_family.' ); + $this->assertSame( 'publish', $font_family->post_status, 'The font family post status should be publish.' ); $font_family_title = 'Open Sans'; - $this->assertSame( $font_family_title, $font_family->post_title ); + $this->assertSame( $font_family_title, $font_family->post_title, 'The font family post title should be Open Sans.' ); $font_family_slug = 'open-sans'; - $this->assertSame( $font_family_slug, $font_family->post_name ); + $this->assertSame( $font_family_slug, $font_family->post_name, 'The font family post name should be open-sans.' ); $font_family_content = wp_json_encode( json_decode( '{"fontFamily":"\'Open Sans\', sans-serif","preview":"https://s.w.org/images/fonts/16.7/previews/open-sans/open-sans.svg"}', true ) ); - $this->assertSame( $font_family_content, $font_family->post_content ); + $this->assertSame( $font_family_content, $font_family->post_content, 'The font family post content should match.' ); $meta = get_post_meta( $font_family_id, '_gutenberg_legacy_font_family', true ); - $this->assertSame( $legacy_content, $meta ); + $this->assertSame( $legacy_content, $meta, 'The _gutenberg_legacy_font_family post meta content should match.' ); // First font face post. - $this->assertSame( 'wp_font_face', $font_face1->post_type ); - $this->assertSame( $font_family_id, $font_face1->post_parent ); - $this->assertSame( 'publish', $font_face1->post_status ); + $this->assertSame( 'wp_font_face', $font_face1->post_type, 'The 1st font face post type should be wp_font_face.' ); + $this->assertSame( $font_family_id, $font_face1->post_parent, 'The 1st font face post parent should match.' ); + $this->assertSame( 'publish', $font_face1->post_status, 'The 1st font face post status should be publish.' ); $font_face1_title = 'open sans;normal;400;100%;U+0-10FFFF'; - $this->assertSame( $font_face1_title, $font_face1->post_title ); - $this->assertSame( sanitize_title( $font_face1_title ), $font_face1->post_name ); + $this->assertSame( $font_face1_title, $font_face1->post_title, 'The 1st font face post title should match.' ); + $this->assertSame( sanitize_title( $font_face1_title ), $font_face1->post_name, 'The 1st font face post name should match.' ); $font_face1_content = wp_json_encode( json_decode( '{"fontFamily":"Open Sans","fontStyle":"normal","fontWeight":"400","preview":"https://s.w.org/images/fonts/16.7/previews/open-sans/open-sans-400-normal.svg","src":"https://fonts.gstatic.com/s/opensans/v35/memSYaGs126MiZpBA-UvWbX2vVnXBbObj2OVZyOOSr4dVJWUgsjZ0C4nY1M2xLER.ttf"}' ) ); - $this->assertSame( $font_face1_content, $font_face1->post_content ); + $this->assertSame( $font_face1_content, $font_face1->post_content, 'The 1st font face post content should match.' ); // With a remote url, file post meta should not be set. $meta = get_post_meta( $font_face1->ID, '_wp_font_face_file', true ); - $this->assertSame( '', $meta ); + $this->assertSame( '', $meta, 'The _wp_font_face_file post meta for the 1st font face should be an empty string.' ); // Second font face post. - $this->assertSame( 'wp_font_face', $font_face2->post_type ); - $this->assertSame( $font_family_id, $font_face2->post_parent ); - $this->assertSame( 'publish', $font_face2->post_status ); + $this->assertSame( 'wp_font_face', $font_face2->post_type, 'The 2nd font face post type should be wp_font_face.' ); + $this->assertSame( $font_family_id, $font_face2->post_parent, 'The 2md font face post type should be wp_font_face.' ); + $this->assertSame( 'publish', $font_face2->post_status, 'The 2nd font face post status should be publish.' ); $font_face2_title = 'open sans;italic;400;100%;U+0-10FFFF'; - $this->assertSame( $font_face2_title, $font_face2->post_title ); - $this->assertSame( sanitize_title( $font_face2_title ), $font_face2->post_name ); + $this->assertSame( $font_face2_title, $font_face2->post_title, 'The 2nd font face post title should match.' ); + $this->assertSame( sanitize_title( $font_face2_title ), $font_face2->post_name, 'The 2nd font face post name should match.' ); $font_face2_content = wp_json_encode( json_decode( '{"fontFamily":"Open Sans","fontStyle":"italic","fontWeight":"400","preview":"https://s.w.org/images/fonts/16.7/previews/open-sans/open-sans-400-italic.svg","src":"https://fonts.gstatic.com/s/opensans/v35/memQYaGs126MiZpBA-UFUIcVXSCEkx2cmqvXlWq8tWZ0Pw86hd0Rk8ZkaVcUwaERZjA.ttf"}' ) ); - $this->assertSame( $font_face2_content, $font_face2->post_content ); + $this->assertSame( $font_face2_content, $font_face2->post_content, 'The 2nd font face post content should match.' ); // With a remote url, file post meta should not be set. $meta = get_post_meta( $font_face2->ID, '_wp_font_face_file', true ); - $this->assertSame( '', $meta ); + $this->assertSame( '', $meta, 'The _wp_font_face_file post meta for the 2nd font face should be an empty string.' ); // Third font face post. - $this->assertSame( 'wp_font_face', $font_face3->post_type ); - $this->assertSame( $font_family_id, $font_face3->post_parent ); - $this->assertSame( 'publish', $font_face3->post_status ); + $this->assertSame( 'wp_font_face', $font_face3->post_type, 'The 3rd font face post type should be wp_font_face.' ); + $this->assertSame( $font_family_id, $font_face3->post_parent, 'The 3rd font face post type should be wp_font_face.' ); + $this->assertSame( 'publish', $font_face3->post_status, 'The 3rd font face post status should be publish.' ); $font_face3_title = 'open sans;normal;700;100%;U+0-10FFFF'; - $this->assertSame( $font_face3_title, $font_face3->post_title ); - $this->assertSame( sanitize_title( $font_face3_title ), $font_face3->post_name ); + $this->assertSame( $font_face3_title, $font_face3->post_title, 'The 3rd font face post title should match.' ); + $this->assertSame( sanitize_title( $font_face3_title ), $font_face3->post_name, 'The 3rd font face post name should match.' ); $font_face3_content = wp_json_encode( json_decode( '{"fontFamily":"Open Sans","fontStyle":"normal","fontWeight":"700","preview":"https://s.w.org/images/fonts/16.7/previews/open-sans/open-sans-700-normal.svg","src":"https://fonts.gstatic.com/s/opensans/v35/memSYaGs126MiZpBA-UvWbX2vVnXBbObj2OVZyOOSr4dVJWUgsg-1y4nY1M2xLER.ttf"}' ) ); - $this->assertSame( $font_face3_content, $font_face3->post_content ); + $this->assertSame( $font_face3_content, $font_face3->post_content, 'The 3rd font face post content should match.' ); // With a remote url, file post meta should not be set. $meta = get_post_meta( $font_face3->ID, '_wp_font_face_file', true ); - $this->assertSame( '', $meta ); - - wp_delete_post( $font_family_id, true ); - wp_delete_post( $font_face1->ID, true ); - wp_delete_post( $font_face2->ID, true ); - wp_delete_post( $font_face3->ID, true ); + $this->assertSame( '', $meta, 'The _wp_font_face_file post meta for the 3rd font face should be an empty string.' ); } public function test_font_faces_with_local_src() { @@ -110,16 +115,14 @@ public function test_font_faces_with_local_src() { gutenberg_convert_legacy_font_family_format(); $font_faces = $this->get_font_faces( $font_family_id ); - $this->assertCount( 1, $font_faces ); + + $this->assertCount( 1, $font_faces, 'There should be 1 font face.' ); $font_face = reset( $font_faces ); // Check that file meta is present. $file_path = 'open-sans_normal_400.ttf'; $meta = get_post_meta( $font_face->ID, '_wp_font_face_file', true ); - $this->assertSame( $file_path, $meta ); - - wp_delete_post( $font_family_id, true ); - wp_delete_post( $font_face->ID, true ); + $this->assertSame( $file_path, $meta, 'The _wp_font_face_file should match.' ); } public function test_migration_only_runs_once() { @@ -135,12 +138,10 @@ public function test_migration_only_runs_once() { // Meta with backup content will not be present if migration isn't triggered. $meta = get_post_meta( $font_family_id, '_gutenberg_legacy_font_family', true ); $this->assertSame( '', $meta ); - - wp_delete_post( $font_family_id, true ); } protected function create_font_family( $content ) { - return wp_insert_post( + $post_id = wp_insert_post( array( 'post_type' => 'wp_font_family', 'post_status' => 'publish', @@ -149,10 +150,22 @@ protected function create_font_family( $content ) { 'post_content' => $content, ) ); + + $this->store_id_for_cleanup_in_teardown( $post_id ); + + return $post_id; + } + + private function get_post( $post_id ) { + $post = get_post( $post_id ); + + $this->store_id_for_cleanup_in_teardown( $post ); + + return $post; } protected function get_font_faces( $font_family_id ) { - return get_posts( + $posts = get_posts( array( 'post_parent' => $font_family_id, 'post_type' => 'wp_font_face', @@ -160,5 +173,24 @@ protected function get_font_faces( $font_family_id ) { 'orderby' => 'id', ) ); + + $this->store_id_for_cleanup_in_teardown( $posts ); + + return $posts; + } + + private function store_id_for_cleanup_in_teardown( $post ) { + if ( null === $post ) { + return; + } + + $posts = is_array( $post ) ? $post : array( $post ); + + foreach ( $posts as $post ) { + if ( null === $post ) { + continue; + } + $this->post_ids_to_delete[] = is_int( $post ) ? $post : $post->ID; + } } } diff --git a/phpunit/tests/fonts/font-library/fontLibraryHooks.php b/phpunit/tests/fonts/font-library/fontLibraryHooks.php index 2c471e2a9759c..550f3992c9f73 100644 --- a/phpunit/tests/fonts/font-library/fontLibraryHooks.php +++ b/phpunit/tests/fonts/font-library/fontLibraryHooks.php @@ -4,9 +4,12 @@ * * @package WordPress * @subpackage Font Library + * + * @group fonts + * @group font-library */ +class Tests_Fonts_FontLibraryHooks extends WP_UnitTestCase { -class Tests_Font_Library_Hooks extends WP_UnitTestCase { public function test_deleting_font_family_deletes_child_font_faces() { $font_family_id = self::factory()->post->create( array( @@ -33,8 +36,8 @@ public function test_deleting_font_family_deletes_child_font_faces() { wp_delete_post( $font_family_id, true ); - $this->assertNull( get_post( $font_face_id ) ); - $this->assertNotNull( get_post( $other_font_face_id ) ); + $this->assertNull( get_post( $font_face_id ), 'Font face post should also have been deleted.' ); + $this->assertNotNull( get_post( $other_font_face_id ), 'The other post should exist.' ); } public function test_deleting_font_faces_deletes_associated_font_files() { @@ -43,8 +46,8 @@ public function test_deleting_font_faces_deletes_associated_font_files() { wp_delete_post( $font_face_id, true ); - $this->assertFalse( file_exists( $font_path ) ); - $this->assertTrue( file_exists( $other_font_path ) ); + $this->assertFalse( file_exists( $font_path ), 'The font file should have been deleted when the post was deleted.' ); + $this->assertTrue( file_exists( $other_font_path ), 'The other font file should exist.' ); } protected function create_font_face_with_file( $filename ) { diff --git a/phpunit/tests/fonts/font-library/wpFontCollection/__construct.php b/phpunit/tests/fonts/font-library/wpFontCollection/__construct.php index bdc3d3b82331c..4021c9eeedf3f 100644 --- a/phpunit/tests/fonts/font-library/wpFontCollection/__construct.php +++ b/phpunit/tests/fonts/font-library/wpFontCollection/__construct.php @@ -11,11 +11,13 @@ * @covers WP_Font_Collection::__construct */ class Tests_Fonts_WpFontCollection_Construct extends WP_UnitTestCase { + /** * @dataProvider data_should_assign_properties_from_php_config * - * @param array $config Font collection config options. - * @param array $expected_data Expected output data. + * @param string $slug Font collection slug. + * @param array $config Font collection config options. + * @param array $expected_data Expected output data. */ public function test_should_assign_properties_from_php_config( $slug, $config, $expected_data ) { $collection = new WP_Font_Collection( $slug, $config ); @@ -32,7 +34,7 @@ public function test_should_assign_properties_from_php_config( $slug, $config, $ /** * Data provider. * - * @return array[] + * @return array */ public function data_should_assign_properties_from_php_config() { return array( diff --git a/phpunit/tests/fonts/font-library/wpFontCollection/loadFromJson.php b/phpunit/tests/fonts/font-library/wpFontCollection/loadFromJson.php index d3ee04eba29f5..6c7b3bd23a6f3 100644 --- a/phpunit/tests/fonts/font-library/wpFontCollection/loadFromJson.php +++ b/phpunit/tests/fonts/font-library/wpFontCollection/loadFromJson.php @@ -31,9 +31,9 @@ public function test_should_load_json_from_url() { add_filter( 'pre_http_request', array( $this, 'mock_request' ), 10, 3 ); $config = WP_Font_Collection::load_from_json( 'https://localhost/fonts/mock-font-collection.json' ); - $this->assertSame( self::$mock_collection_data, $config ); - remove_filter( 'pre_http_request', array( $this, 'mock_request' ) ); + + $this->assertSame( self::$mock_collection_data, $config ); } public function test_should_error_with_invalid_file_path() { @@ -54,9 +54,9 @@ public function test_should_error_with_invalid_url_response() { add_filter( 'pre_http_request', array( $this, 'mock_request_invalid_response' ), 10, 3 ); $config = WP_Font_Collection::load_from_json( 'https://localhost/fonts/missing-collection.json' ); - $this->assertWPError( $config, 'font_collection_json_missing' ); - remove_filter( 'pre_http_request', array( $this, 'mock_request_invalid_response' ) ); + + $this->assertWPError( $config, 'font_collection_json_missing' ); } public function test_should_error_with_invalid_json_from_file() { @@ -72,9 +72,9 @@ public function test_should_error_with_invalid_json_from_url() { add_filter( 'pre_http_request', array( $this, 'mock_request_invalid_json' ), 10, 3 ); $config = WP_Font_Collection::load_from_json( 'https://localhost/fonts/invalid-collection.json' ); - $this->assertWPError( $config, 'font_collection_decode_error' ); - remove_filter( 'pre_http_request', array( $this, 'mock_request_invalid_json' ) ); + + $this->assertWPError( $config, 'font_collection_decode_error' ); } public function test_should_error_with_json_from_file_missing_slug() { @@ -93,9 +93,9 @@ public function test_should_error_with_json_from_url_missing_slug() { $this->setExpectedIncorrectUsage( 'WP_Font_Collection::load_from_url' ); $config = WP_Font_Collection::load_from_json( 'https://localhost/fonts/missing-slug.json' ); - $this->assertWPError( $config, 'font_collection_invalid_json' ); - remove_filter( 'pre_http_request', array( $this, 'mock_request_missing_slug' ) ); + + $this->assertWPError( $config, 'font_collection_invalid_json' ); } public function mock_request( $preempt, $args, $url ) { diff --git a/phpunit/tests/fonts/font-library/wpFontLibrary/getMimeTypes.php b/phpunit/tests/fonts/font-library/wpFontLibrary/getMimeTypes.php index 579baa2d248e0..4fb97e754611f 100644 --- a/phpunit/tests/fonts/font-library/wpFontLibrary/getMimeTypes.php +++ b/phpunit/tests/fonts/font-library/wpFontLibrary/getMimeTypes.php @@ -13,21 +13,20 @@ class Tests_Fonts_WpFontLibrary_GetMimeTypes extends WP_Font_Library_UnitTestCase { /** - * * @dataProvider data_should_supply_correct_mime_type_for_php_version * - * @param array $php_version_id PHP_VERSION_ID value. - * @param array $expected Expected mime types. + * @param int $php_version_id PHP_VERSION_ID value. + * @param array $expected Expected mime types. */ public function test_should_supply_correct_mime_type_for_php_version( $php_version_id, $expected ) { $mimes = WP_Font_Library::get_expected_font_mime_types_per_php_version( $php_version_id ); - $this->assertEquals( $mimes, $expected ); + $this->assertSame( $expected, $mimes ); } /** * Data provider. * - * @return array[] + * @return array */ public function data_should_supply_correct_mime_type_for_php_version() { return array( diff --git a/phpunit/tests/fonts/font-library/wpFontUtils/formatFontFamily.php b/phpunit/tests/fonts/font-library/wpFontUtils/formatFontFamily.php index 53eaced487520..f1acf5422a8b4 100644 --- a/phpunit/tests/fonts/font-library/wpFontUtils/formatFontFamily.php +++ b/phpunit/tests/fonts/font-library/wpFontUtils/formatFontFamily.php @@ -15,8 +15,8 @@ class Tests_Fonts_WpFontUtils_FormatFontFamily extends WP_UnitTestCase { /** * @dataProvider data_should_format_font_family * - * @param string $font_family Font family. - * @param string $expected Expected family. + * @param string $font_family Font family to test. + * @param string $expected Expected family. */ public function test_should_format_font_family( $font_family, $expected ) { $this->assertSame( @@ -30,7 +30,7 @@ public function test_should_format_font_family( $font_family, $expected ) { /** * Data provider. * - * @return array[] + * @return array */ public function data_should_format_font_family() { return array( diff --git a/phpunit/tests/fonts/font-library/wpFontUtils/getFontFaceSlug.php b/phpunit/tests/fonts/font-library/wpFontUtils/getFontFaceSlug.php index ded19749b9123..de0b02e63185e 100644 --- a/phpunit/tests/fonts/font-library/wpFontUtils/getFontFaceSlug.php +++ b/phpunit/tests/fonts/font-library/wpFontUtils/getFontFaceSlug.php @@ -4,12 +4,18 @@ * * @package WordPress * @subpackage Font Library - * * + * + * @group fonts + * @group font-library + * * @covers WP_Font_Utils::get_font_face_slug */ class Tests_Fonts_WpFontUtils_GetFontFaceSlug extends WP_UnitTestCase { /** * @dataProvider data_get_font_face_slug_normalizes_values + * + * @param string[] $settings Settings to test. + * @param string $expected_slug Expected slug results. */ public function test_get_font_face_slug_normalizes_values( $settings, $expected_slug ) { $slug = WP_Font_Utils::get_font_face_slug( $settings ); @@ -17,6 +23,11 @@ public function test_get_font_face_slug_normalizes_values( $settings, $expected_ $this->assertSame( $expected_slug, $slug ); } + /** + * Data provider. + * + * @return array + */ public function data_get_font_face_slug_normalizes_values() { return array( 'Sets defaults' => array( diff --git a/phpunit/tests/fonts/font-library/fontsDir.php b/phpunit/tests/fonts/font-library/wpFontsDir.php similarity index 80% rename from phpunit/tests/fonts/font-library/fontsDir.php rename to phpunit/tests/fonts/font-library/wpFontsDir.php index 5c13f1d120f9a..a8f79888315bd 100644 --- a/phpunit/tests/fonts/font-library/fontsDir.php +++ b/phpunit/tests/fonts/font-library/wpFontsDir.php @@ -8,14 +8,15 @@ * @group fonts * @group font-library * - * @covers wp_get_font_dir + * @covers ::wp_get_font_dir */ class Tests_Fonts_WpFontDir extends WP_UnitTestCase { - private $dir_defaults; + private static $dir_defaults; - public function __construct() { - parent::__construct(); - $this->dir_defaults = array( + public static function set_up_before_class() { + parent::set_up_before_class(); + + static::$dir_defaults = array( 'path' => path_join( WP_CONTENT_DIR, 'fonts' ), 'url' => content_url( 'fonts' ), 'subdir' => '', @@ -27,7 +28,8 @@ public function __construct() { public function test_fonts_dir() { $font_dir = wp_get_font_dir(); - $this->assertEquals( $font_dir, $this->dir_defaults ); + + $this->assertSame( $font_dir, static::$dir_defaults ); } public function test_fonts_dir_with_filter() { @@ -57,14 +59,14 @@ function set_new_values( $defaults ) { 'error' => false, ); - $this->assertEquals( $font_dir, $expected, 'The wp_get_font_dir() method should return the expected values.' ); - // Remove the filter. remove_filter( 'font_dir', 'set_new_values' ); + $this->assertSame( $expected, $font_dir, 'The wp_get_font_dir() method should return the expected values.' ); + // Gets the fonts dir. $font_dir = wp_get_font_dir(); - $this->assertEquals( $font_dir, $this->dir_defaults, 'The wp_get_font_dir() method should return the default values.' ); + $this->assertSame( static::$dir_defaults, $font_dir, 'The wp_get_font_dir() method should return the default values.' ); } } diff --git a/phpunit/tests/fonts/font-library/wpRestFontCollectionsController.php b/phpunit/tests/fonts/font-library/wpRestFontCollectionsController.php index 06ae5edf3e989..e6143da6da646 100644 --- a/phpunit/tests/fonts/font-library/wpRestFontCollectionsController.php +++ b/phpunit/tests/fonts/font-library/wpRestFontCollectionsController.php @@ -7,10 +7,12 @@ * @since 6.5.0 * * @group restapi + * @group fonts + * @group font-library * * @coversDefaultClass WP_REST_Font_Collections_Controller */ -class WP_REST_Font_Collections_Controller_Test extends WP_Test_REST_Controller_Testcase { +class Tests_REST_WpRestFontCollectionsController extends WP_Test_REST_Controller_Testcase { protected static $admin_id; protected static $editor_id; protected static $mock_file; @@ -39,7 +41,6 @@ public static function wpTearDownAfterClass() { wp_unregister_font_collection( 'mock-col-slug' ); } - /** * @covers WP_REST_Font_Collections_Controller::register_routes */ @@ -61,7 +62,7 @@ public function test_get_items() { $response = rest_get_server()->dispatch( $request ); $content = $response->get_data(); $this->assertIsArray( $content ); - $this->assertEquals( 200, $response->get_status() ); + $this->assertSame( 200, $response->get_status() ); } /** @@ -71,7 +72,7 @@ public function test_get_item() { wp_set_current_user( self::$admin_id ); $request = new WP_REST_Request( 'GET', '/wp/v2/font-collections/mock-col-slug' ); $response = rest_get_server()->dispatch( $request ); - $this->assertEquals( 200, $response->get_status(), 'Response code is not 200' ); + $this->assertSame( 200, $response->get_status(), 'Response code is not 200' ); $response_data = $response->get_data(); $this->assertArrayHasKey( 'name', $response_data, 'Response data does not have the name key.' ); @@ -153,13 +154,13 @@ public function test_get_item_schema() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 200, $response->get_status() ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200.' ); $properties = $data['schema']['properties']; - $this->assertCount( 5, $properties ); - $this->assertArrayHasKey( 'slug', $properties ); - $this->assertArrayHasKey( 'name', $properties ); - $this->assertArrayHasKey( 'description', $properties ); - $this->assertArrayHasKey( 'font_families', $properties ); - $this->assertArrayHasKey( 'categories', $properties ); + $this->assertCount( 5, $properties, 'There should be 5 properties in the response data schema.' ); + $this->assertArrayHasKey( 'slug', $properties, 'The slug property should exist in the response data schema.' ); + $this->assertArrayHasKey( 'name', $properties, 'The name property should exist in the response data schema.' ); + $this->assertArrayHasKey( 'description', $properties, 'The description property should exist in the response data schema.' ); + $this->assertArrayHasKey( 'font_families', $properties, 'The slug font_families should exist in the response data schema.' ); + $this->assertArrayHasKey( 'categories', $properties, 'The categories property should exist in the response data schema.' ); } } diff --git a/phpunit/tests/fonts/font-library/wpRestFontFacesController.php b/phpunit/tests/fonts/font-library/wpRestFontFacesController.php index d6a95814b205a..de6ecc3d73514 100644 --- a/phpunit/tests/fonts/font-library/wpRestFontFacesController.php +++ b/phpunit/tests/fonts/font-library/wpRestFontFacesController.php @@ -7,10 +7,12 @@ * @since 6.5.0 * * @group restapi + * @group fonts + * @group font-library * - * @coversDefaultClass WP_REST_Font_Faces_Controller_Test + * @coversDefaultClass WP_REST_Font_Faces_Controller */ -class WP_REST_Font_Faces_Controller_Test extends WP_Test_REST_Controller_Testcase { +class Tests_REST_WpRestFontFacesController extends WP_Test_REST_Controller_Testcase { protected static $admin_id; protected static $editor_id; @@ -20,6 +22,8 @@ class WP_REST_Font_Faces_Controller_Test extends WP_Test_REST_Controller_Testcas protected static $font_face_id1; protected static $font_face_id2; + private static $post_ids_for_cleanup = array(); + protected static $default_settings = array( 'fontFamily' => '"Open Sans"', 'fontWeight' => '400', @@ -28,8 +32,8 @@ class WP_REST_Font_Faces_Controller_Test extends WP_Test_REST_Controller_Testcas ); public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { - self::$font_family_id = WP_REST_Font_Families_Controller_Test::create_font_family_post(); - self::$other_font_family_id = WP_REST_Font_Families_Controller_Test::create_font_family_post(); + self::$font_family_id = Tests_REST_WpRestFontFamiliesController::create_font_family_post(); + self::$other_font_family_id = Tests_REST_WpRestFontFamiliesController::create_font_family_post(); self::$font_face_id1 = self::create_font_face_post( self::$font_family_id, @@ -60,17 +64,32 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { 'role' => 'editor', ) ); + + self::$post_ids_for_cleanup = array(); } public static function wpTearDownAfterClass() { self::delete_user( self::$admin_id ); self::delete_user( self::$editor_id ); + + wp_delete_post( self::$font_family_id, true ); + wp_delete_post( self::$other_font_family_id, true ); + wp_delete_post( self::$font_face_id1, true ); + wp_delete_post( self::$font_face_id2, true ); + } + + public function tear_down() { + foreach ( self::$post_ids_for_cleanup as $post_id ) { + wp_delete_post( $post_id, true ); + } + self::$post_ids_for_cleanup = array(); + parent::tear_down(); } public static function create_font_face_post( $parent_id, $settings = array() ) { $settings = array_merge( self::$default_settings, $settings ); $title = WP_Font_Utils::get_font_face_slug( $settings ); - return self::factory()->post->create( + $post_id = self::factory()->post->create( wp_slash( array( 'post_type' => 'wp_font_face', @@ -82,6 +101,10 @@ public static function create_font_face_post( $parent_id, $settings = array() ) ) ) ); + + self::$post_ids_for_cleanup[] = $post_id; + + return $post_id; } /** @@ -128,24 +151,45 @@ public function test_font_faces_no_autosave_routes() { } /** - * @covers WP_REST_Font_Faces_Controller::get_context_param + * @doesNotPerformAssertions */ public function test_context_param() { - // Collection. - $request = new WP_REST_Request( 'OPTIONS', '/wp/v2/font-families/' . self::$font_family_id . '/font-faces' ); - $response = rest_get_server()->dispatch( $request ); - $data = $response->get_data(); - $this->assertArrayNotHasKey( 'allow_batch', $data['endpoints'][0] ); - $this->assertSame( 'view', $data['endpoints'][0]['args']['context']['default'] ); - $this->assertSame( array( 'view', 'embed', 'edit' ), $data['endpoints'][0]['args']['context']['enum'] ); + // See test_get_context_param(). + } - // Single. - $request = new WP_REST_Request( 'OPTIONS', '/wp/v2/font-families/' . self::$font_family_id . '/font-faces/' . self::$font_face_id1 ); + /** + * @dataProvider data_get_context_param + * + * @covers WP_REST_Font_Faces_Controller::get_context_param + * + * @param bool $single_route Whether to test a single route. + */ + public function test_get_context_param( $single_route ) { + $route = '/wp/v2/font-families/' . self::$font_family_id . '/font-faces'; + if ( $single_route ) { + $route .= '/' . self::$font_face_id1; + } + + $request = new WP_REST_Request( 'OPTIONS', $route ); $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertArrayNotHasKey( 'allow_batch', $data['endpoints'][0] ); - $this->assertSame( 'view', $data['endpoints'][0]['args']['context']['default'] ); - $this->assertSame( array( 'view', 'embed', 'edit' ), $data['endpoints'][0]['args']['context']['enum'] ); + + $endpoint_data = $data['endpoints'][0]; + $this->assertArrayNotHasKey( 'allow_batch', $endpoint_data, 'The allow_batch property should not exist in the endpoint data.' ); + $this->assertSame( 'view', $endpoint_data['args']['context']['default'], 'The endpoint\'s args::context::default should be set to view.' ); + $this->assertSame( array( 'view', 'embed', 'edit' ), $endpoint_data['args']['context']['enum'], 'The endpoint\'s args::context::enum should be set to [ view, embed, edit ].' ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_get_context_param() { + return array( + 'Collection' => array( false ), + 'Single' => array( true ), + ); } /** @@ -157,11 +201,11 @@ public function test_get_items() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 200, $response->get_status() ); - $this->assertCount( 2, $data ); - $this->assertArrayHasKey( '_links', $data[0] ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200' ); + $this->assertCount( 2, $data, 'There should be 2 properties in the response data.' ); + $this->assertArrayHasKey( '_links', $data[0], 'The _links property should exist in the response data 0.' ); $this->check_font_face_data( $data[0], self::$font_face_id2, $data[0]['_links'] ); - $this->assertArrayHasKey( '_links', $data[1] ); + $this->assertArrayHasKey( '_links', $data[1], 'The _links property should exist in the response data 1.' ); $this->check_font_face_data( $data[1], self::$font_face_id1, $data[1]['_links'] ); } @@ -172,11 +216,11 @@ public function test_get_items_no_permission() { wp_set_current_user( 0 ); $request = new WP_REST_Request( 'GET', '/wp/v2/font-families/' . self::$font_family_id . '/font-faces' ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_cannot_read', $response, 401 ); + $this->assertErrorResponse( 'rest_cannot_read', $response, 401, 'The response should return an error with a "rest_cannot_read" code and 401 status.' ); wp_set_current_user( self::$editor_id ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_cannot_read', $response, 403 ); + $this->assertErrorResponse( 'rest_cannot_read', $response, 403, 'The response should return an error with a "rest_cannot_read" code and 403 status.' ); } /** @@ -198,7 +242,7 @@ public function test_get_item() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 200, $response->get_status() ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200.' ); $this->check_font_face_data( $data, self::$font_face_id1, $response->get_links() ); } @@ -213,8 +257,9 @@ public function test_get_item_removes_extra_settings() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 200, $response->get_status() ); - $this->assertArrayNotHasKey( 'extra', $data['font_face_settings'] ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200.' ); + $this->assertArrayHasKey( 'font_face_settings', $data, 'The font_face_settings property should exist in the response data.' ); + $this->assertArrayNotHasKey( 'extra', $data['font_face_settings'], 'The extra property should exist in the font_face_settings data.' ); } /** @@ -230,6 +275,8 @@ public function test_get_item_malformed_post_content_returns_empty_settings() { ) ); + self::$post_ids_for_cleanup[] = $font_face_id; + $empty_settings = array( 'fontFamily' => '', 'src' => array(), @@ -240,10 +287,9 @@ public function test_get_item_malformed_post_content_returns_empty_settings() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 200, $response->get_status() ); - $this->assertSame( $empty_settings, $data['font_face_settings'] ); - - wp_delete_post( $font_face_id, true ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200.' ); + $this->assertArrayHasKey( 'font_face_settings', $data, 'The font_face_settings property should exist in the response data.' ); + $this->assertSame( $empty_settings, $data['font_face_settings'], 'The empty settings should exist in the font_face_settings data.' ); } /** @@ -264,11 +310,11 @@ public function test_get_item_no_permission() { $request = new WP_REST_Request( 'GET', '/wp/v2/font-families/' . self::$font_family_id . '/font-faces/' . self::$font_face_id1 ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_cannot_read', $response, 401 ); + $this->assertErrorResponse( 'rest_cannot_read', $response, 401, 'The response should return an error with a "rest_cannot_read" code and 401 status.' ); wp_set_current_user( self::$editor_id ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_cannot_read', $response, 403 ); + $this->assertErrorResponse( 'rest_cannot_read', $response, 403, 'The response should return an error with a "rest_cannot_read" code and 403 status.' ); } /** @@ -291,7 +337,7 @@ public function test_get_item_valid_parent_id() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 200, $response->get_status() ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200.' ); $this->assertSame( self::$font_family_id, $data['parent'], 'The returned parent id should match the font family id.' ); } @@ -333,7 +379,7 @@ public function test_create_item() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 201, $response->get_status() ); + $this->assertSame( 201, $response->get_status(), 'The response status should be 201.' ); $this->check_font_face_data( $data, $data['id'], $response->get_links() ); $this->check_file_meta( $data['id'], array( $data['font_face_settings']['src'] ) ); @@ -345,12 +391,11 @@ public function test_create_item() { 'fontFamily' => '"Open Sans"', 'fontWeight' => '200', 'fontStyle' => 'normal', - ) + ), + 'The font_face_settings data should match the expected data.' ); $this->assertSame( self::$font_family_id, $data['parent'], 'The returned parent id should match the font family id.' ); - - wp_delete_post( $data['id'], true ); } /** @@ -378,14 +423,12 @@ public function test_create_item_with_multiple_font_files() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 201, $response->get_status() ); + $this->assertSame( 201, $response->get_status(), 'The response status should be 201.' ); $this->check_font_face_data( $data, $data['id'], $response->get_links() ); $this->check_file_meta( $data['id'], $data['font_face_settings']['src'] ); $settings = $data['font_face_settings']; - $this->assertCount( 4, $settings['src'] ); - - wp_delete_post( $data['id'], true ); + $this->assertCount( 4, $settings['src'], 'There should be 4 items in the font_face_settings::src data.' ); } /** @@ -451,10 +494,8 @@ public function test_create_item_with_url_src() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 201, $response->get_status() ); + $this->assertSame( 201, $response->get_status(), 'The response status should be 201.' ); $this->check_font_face_data( $data, $data['id'], $response->get_links() ); - - wp_delete_post( $data['id'], true ); } /** @@ -487,12 +528,11 @@ public function test_create_item_with_all_properties() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - - $this->assertSame( 201, $response->get_status() ); - $this->assertArrayHasKey( 'font_face_settings', $data ); - $this->assertSame( $properties, $data['font_face_settings'] ); - wp_delete_post( $data['id'], true ); + + $this->assertSame( 201, $response->get_status(), 'The response status should be 201.' ); + $this->assertArrayHasKey( 'font_face_settings', $data, 'The font_face_settings property should exist in the response data.' ); + $this->assertSame( $properties, $data['font_face_settings'], 'The font_face_settings should match the expected properties.' ); } /** @@ -514,13 +554,13 @@ public function test_create_item_missing_parent() { * @covers WP_REST_Font_Faces_Controller::create_item */ public function test_create_item_with_duplicate_properties() { - $settings = array( + $settings = array( 'fontFamily' => '"Open Sans"', 'fontWeight' => '200', 'fontStyle' => 'italic', 'src' => home_url( '/wp-content/fonts/open-sans-italic-light.ttf' ), ); - $font_face_id = self::create_font_face_post( self::$font_family_id, $settings ); + self::create_font_face_post( self::$font_family_id, $settings ); wp_set_current_user( self::$admin_id ); $request = new WP_REST_Request( 'POST', '/wp/v2/font-families/' . self::$font_family_id . '/font-faces' ); @@ -528,12 +568,10 @@ public function test_create_item_with_duplicate_properties() { $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_duplicate_font_face', $response, 400 ); + $this->assertErrorResponse( 'rest_duplicate_font_face', $response, 400, 'The response should return an error for "rest_duplicate_font_face" with 400 status.' ); $expected_message = 'A font face matching those settings already exists.'; $message = $response->as_error()->get_error_messages()[0]; - $this->assertSame( $expected_message, $message ); - - wp_delete_post( $font_face_id, true ); + $this->assertSame( $expected_message, $message, 'The response error message should match.' ); } /** @@ -555,18 +593,19 @@ public function test_create_item_default_theme_json_version() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); + wp_delete_post( $data['id'], true ); - $this->assertSame( 201, $response->get_status() ); - $this->assertArrayHasKey( 'theme_json_version', $data ); + $this->assertSame( 201, $response->get_status(), 'The response status should be 201.' ); + $this->assertArrayHasKey( 'theme_json_version', $data, 'The theme_json_version property should exist in the response data.' ); $this->assertSame( 2, $data['theme_json_version'], 'The default theme.json version should be 2.' ); - - wp_delete_post( $data['id'], true ); } /** * @dataProvider data_create_item_invalid_theme_json_version * * @covers WP_REST_Font_Faces_Controller::create_item + * + * @param int $theme_json_version Version input to test. */ public function test_create_item_invalid_theme_json_version( $theme_json_version ) { wp_set_current_user( self::$admin_id ); @@ -578,6 +617,11 @@ public function test_create_item_invalid_theme_json_version( $theme_json_version $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); } + /** + * Data provider. + * + * @return array + */ public function data_create_item_invalid_theme_json_version() { return array( array( 1 ), @@ -589,6 +633,8 @@ public function data_create_item_invalid_theme_json_version() { * @dataProvider data_create_item_invalid_settings * * @covers WP_REST_Font_Faces_Controller::validate_create_font_face_settings + * + * @param mixed $settings Settings to test. */ public function test_create_item_invalid_settings( $settings ) { wp_set_current_user( self::$admin_id ); @@ -601,6 +647,11 @@ public function test_create_item_invalid_settings( $settings ) { $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); } + /** + * Data provider. + * + * @return array + */ public function data_create_item_invalid_settings() { return array( 'Missing fontFamily' => array( @@ -647,10 +698,10 @@ public function test_create_item_invalid_settings_json() { $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); + $this->assertErrorResponse( 'rest_invalid_param', $response, 400, 'The response should return an error for "rest_invalid_param" with 400 status.' ); $expected_message = 'font_face_settings parameter must be a valid JSON string.'; $message = $response->as_error()->get_all_error_data()[0]['params']['font_face_settings']; - $this->assertSame( $expected_message, $message ); + $this->assertSame( $expected_message, $message, 'The response error message should match.' ); } /** @@ -672,16 +723,19 @@ public function test_create_item_invalid_file_src() { $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); + $this->assertErrorResponse( 'rest_invalid_param', $response, 400, 'The response should return an error for "rest_invalid_param" with 400 status.' ); $expected_message = 'File ' . array_keys( $files )[0] . ' must be used in font_face_settings[src].'; $message = $response->as_error()->get_all_error_data()[0]['params']['font_face_settings']; - $this->assertSame( $expected_message, $message ); + $this->assertSame( $expected_message, $message, 'The response error message should match.' ); } /** * @dataProvider data_create_item_santize_font_family * * @covers WP_REST_Font_Face_Controller::sanitize_font_face_settings + * + * @param string $font_family_setting Setting to test. + * @param string $expected Expected result. */ public function test_create_item_santize_font_family( $font_family_setting, $expected ) { $settings = array_merge( self::$default_settings, array( 'fontFamily' => $font_family_setting ) ); @@ -692,15 +746,29 @@ public function test_create_item_santize_font_family( $font_family_setting, $exp $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 201, $response->get_status() ); - $this->assertSame( $expected, $data['font_face_settings']['fontFamily'] ); + $this->assertSame( 201, $response->get_status(), 'The response status should be 201.' ); + $this->assertSame( $expected, $data['font_face_settings']['fontFamily'], 'The response fontFamily should match.' ); } + /** + * Data provider. + * + * @return array + */ public function data_create_item_santize_font_family() { return array( - array( 'Libre Barcode 128 Text', '"Libre Barcode 128 Text"' ), - array( 'B612 Mono', '"B612 Mono"' ), - array( 'Open Sans, Noto Sans, sans-serif', '"Open Sans", "Noto Sans", sans-serif' ), + 'multiword font with integer' => array( + 'font_family_setting' => 'Libre Barcode 128 Text', + 'expected' => '"Libre Barcode 128 Text"', + ), + 'multiword font' => array( + 'font_family_setting' => 'B612 Mono', + 'expected' => '"B612 Mono"', + ), + 'comma-separated fonts' => array( + 'font_family_setting' => 'Open Sans, Noto Sans, sans-serif', + 'expected' => '"Open Sans", "Noto Sans", sans-serif', + ), ); } @@ -709,6 +777,9 @@ public function data_create_item_santize_font_family() { */ // public function test_create_item_no_permission() {} + /** + * @covers WP_REST_Font_Faces_Controller::update_item + */ public function test_update_item() { $request = new WP_REST_Request( 'POST', '/wp/v2/font-families/' . self::$font_family_id . '/font-faces/' . self::$font_face_id1 ); $response = rest_get_server()->dispatch( $request ); @@ -725,8 +796,8 @@ public function test_delete_item() { $request->set_param( 'force', true ); $response = rest_get_server()->dispatch( $request ); - $this->assertSame( 200, $response->get_status() ); - $this->assertNull( get_post( $font_face_id ) ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 201.' ); + $this->assertNull( get_post( $font_face_id ), 'The deleted post should not exist.' ); } /** @@ -739,15 +810,15 @@ public function test_delete_item_no_trash() { // Attempt trashing. $request = new WP_REST_Request( 'DELETE', '/wp/v2/font-families/' . self::$font_family_id . '/font-faces/' . $font_face_id ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_trash_not_supported', $response, 501 ); + $this->assertErrorResponse( 'rest_trash_not_supported', $response, 501, 'The response should return an error for "rest_trash_not_supported" with 501 status.' ); $request->set_param( 'force', 'false' ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_trash_not_supported', $response, 501 ); + $this->assertErrorResponse( 'rest_trash_not_supported', $response, 501, 'When "force" is false, the response should return an error for "rest_trash_not_supported" with 501 status.' ); // Ensure the post still exists. $post = get_post( $font_face_id ); - $this->assertNotEmpty( $post ); + $this->assertNotEmpty( $post, 'The post should still exists.' ); } /** @@ -781,7 +852,7 @@ public function test_delete_item_invalid_parent_id() { $request = new WP_REST_Request( 'DELETE', '/wp/v2/font-families/' . self::$other_font_family_id . '/font-faces/' . self::$font_face_id1 ); $request->set_param( 'force', true ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_font_face_parent_id_mismatch', $response, 404 ); + $this->assertErrorResponse( 'rest_font_face_parent_id_mismatch', $response, 404, 'The response should return an error for "rest_font_face_parent_id_mismatch" with 404 status.' ); $expected_message = 'The font face does not belong to the specified font family with id of "' . self::$other_font_family_id . '"'; $this->assertSame( $expected_message, $response->as_error()->get_error_messages()[0], 'The message must contain the correct parent ID.' ); @@ -796,12 +867,12 @@ public function test_delete_item_no_permissions() { wp_set_current_user( 0 ); $request = new WP_REST_Request( 'DELETE', '/wp/v2/font-families/' . self::$font_family_id . '/font-faces/' . $font_face_id ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_cannot_delete', $response, 401 ); + $this->assertErrorResponse( 'rest_cannot_delete', $response, 401, 'The response should return an error for "rest_cannot_delete" with 401 status for an invalid user.' ); wp_set_current_user( self::$editor_id ); $request = new WP_REST_Request( 'DELETE', '/wp/v2/font-families/' . self::$font_family_id . '/font-faces/' . $font_face_id ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_cannot_delete', $response, 403 ); + $this->assertErrorResponse( 'rest_cannot_delete', $response, 403, 'The response should return an error for "rest_cannot_delete" with 403 status for a user without permission.' ); } /** @@ -813,7 +884,7 @@ public function test_prepare_item() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 200, $response->get_status() ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200.' ); $this->check_font_face_data( $data, self::$font_face_id2, $response->get_links() ); } @@ -825,34 +896,38 @@ public function test_get_item_schema() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 200, $response->get_status() ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200.' ); $properties = $data['schema']['properties']; - $this->assertCount( 4, $properties ); - $this->assertArrayHasKey( 'id', $properties ); - $this->assertArrayHasKey( 'theme_json_version', $properties ); - $this->assertArrayHasKey( 'parent', $properties ); - $this->assertArrayHasKey( 'font_face_settings', $properties ); + $this->assertCount( 4, $properties, 'There should be 4 properties in the schema::properties data.' ); + $this->assertArrayHasKey( 'id', $properties, 'The id property should exist in the schema::properties data.' ); + $this->assertArrayHasKey( 'theme_json_version', $properties, 'The id property should exist in the schema::properties data.' ); + $this->assertArrayHasKey( 'parent', $properties, 'The id property should exist in the schema::properties data.' ); + $this->assertArrayHasKey( 'font_face_settings', $properties, 'The id property should exist in the schema::properties data.' ); } protected function check_font_face_data( $data, $post_id, $links ) { - $post = get_post( $post_id ); + self::$post_ids_for_cleanup[] = $post_id; + $post = get_post( $post_id ); - $this->assertArrayHasKey( 'id', $data ); - $this->assertSame( $post->ID, $data['id'] ); + $this->assertArrayHasKey( 'id', $data, 'The id property should exist in response data.' ); + $this->assertSame( $post->ID, $data['id'], 'The "id" from the response data should match the post ID.' ); - $this->assertArrayHasKey( 'parent', $data ); - $this->assertSame( $post->post_parent, $data['parent'] ); + $this->assertArrayHasKey( 'parent', $data, 'The parent property should exist in response data.' ); + $this->assertSame( $post->post_parent, $data['parent'], 'The "parent" from the response data should match the post parent.' ); - $this->assertArrayHasKey( 'theme_json_version', $data ); - $this->assertSame( WP_Theme_JSON::LATEST_SCHEMA, $data['theme_json_version'] ); + $this->assertArrayHasKey( 'theme_json_version', $data, 'The theme_json_version property should exist in response data.' ); + $this->assertSame( WP_Theme_JSON::LATEST_SCHEMA, $data['theme_json_version'], 'The "theme_json_version" from the response data should match WP_Theme_JSON::LATEST_SCHEMA.' ); - $this->assertArrayHasKey( 'font_face_settings', $data ); - $this->assertSame( $post->post_content, wp_json_encode( $data['font_face_settings'] ) ); + $this->assertArrayHasKey( 'font_face_settings', $data, 'The font_face_settings property should exist in response data.' ); + $this->assertSame( $post->post_content, wp_json_encode( $data['font_face_settings'] ), 'The encoded "font_face_settings" from the response data should match the post content.' ); - $this->assertNotEmpty( $links ); - $this->assertSame( rest_url( 'wp/v2/font-families/' . $post->post_parent . '/font-faces/' . $post->ID ), $links['self'][0]['href'] ); - $this->assertSame( rest_url( 'wp/v2/font-families/' . $post->post_parent . '/font-faces' ), $links['collection'][0]['href'] ); - $this->assertSame( rest_url( 'wp/v2/font-families/' . $post->post_parent ), $links['parent'][0]['href'] ); + $this->assertNotEmpty( $links, 'The links should not be empty in the response data.' ); + $expected = rest_url( 'wp/v2/font-families/' . $post->post_parent . '/font-faces/' . $post->ID ); + $this->assertSame( $expected, $links['self'][0]['href'], 'The links URL from the response data should match the post\'s REST endpoint.' ); + $expected = rest_url( 'wp/v2/font-families/' . $post->post_parent . '/font-faces' ); + $this->assertSame( $expected, $links['collection'][0]['href'], 'The links collection URL from the response data should match the REST endpoint.' ); + $expected = rest_url( 'wp/v2/font-families/' . $post->post_parent ); + $this->assertSame( $expected, $links['parent'][0]['href'], 'The links for a parent URL from the response data should match the parent\'s REST endpoint.' ); } protected function check_file_meta( $font_face_id, $srcs ) { diff --git a/phpunit/tests/fonts/font-library/wpRestFontFamiliesController.php b/phpunit/tests/fonts/font-library/wpRestFontFamiliesController.php index 601ff32e2b6c9..6468eff7e24a4 100644 --- a/phpunit/tests/fonts/font-library/wpRestFontFamiliesController.php +++ b/phpunit/tests/fonts/font-library/wpRestFontFamiliesController.php @@ -7,10 +7,12 @@ * @since 6.5.0 * * @group restapi + * @group fonts + * @group font-library * - * @coversDefaultClass WP_REST_Font_Families_Controller_Test + * @coversDefaultClass WP_REST_Font_Families_Controller */ -class WP_REST_Font_Families_Controller_Test extends WP_Test_REST_Controller_Testcase { +class Tests_REST_WpRestFontFamiliesController extends WP_Test_REST_Controller_Testcase { protected static $admin_id; protected static $editor_id; @@ -20,6 +22,8 @@ class WP_REST_Font_Families_Controller_Test extends WP_Test_REST_Controller_Test protected static $font_face_id1; protected static $font_face_id2; + private static $post_ids_to_cleanup = array(); + protected static $default_settings = array( 'name' => 'Open Sans', 'slug' => 'open-sans', @@ -54,7 +58,7 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { 'fontFamily' => 'Helvetica, Arial, sans-serif', ) ); - self::$font_face_id1 = WP_REST_Font_Faces_Controller_Test::create_font_face_post( + self::$font_face_id1 = Tests_REST_WpRestFontFacesController::create_font_face_post( self::$font_family_id1, array( 'fontFamily' => '"Open Sans"', @@ -63,7 +67,7 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { 'src' => home_url( '/wp-content/fonts/open-sans-medium.ttf' ), ) ); - self::$font_face_id2 = WP_REST_Font_Faces_Controller_Test::create_font_face_post( + self::$font_face_id2 = Tests_REST_WpRestFontFacesController::create_font_face_post( self::$font_family_id1, array( 'fontFamily' => '"Open Sans"', @@ -72,16 +76,32 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { 'src' => home_url( '/wp-content/fonts/open-sans-bold.ttf' ), ) ); + + static::$post_ids_to_cleanup = array(); } public static function wpTearDownAfterClass() { self::delete_user( self::$admin_id ); self::delete_user( self::$editor_id ); + + wp_delete_post( self::$font_family_id1 ); + wp_delete_post( self::$font_family_id2 ); + wp_delete_post( self::$font_face_id1 ); + wp_delete_post( self::$font_face_id2 ); + } + + public function tear_down() { + foreach ( static::$post_ids_to_cleanup as $post_id ) { + wp_delete_post( $post_id, true ); + } + static::$post_ids_to_cleanup = array(); + + parent::tear_down(); } public static function create_font_family_post( $settings = array() ) { $settings = array_merge( self::$default_settings, $settings ); - return self::factory()->post->create( + $post_id = self::factory()->post->create( wp_slash( array( 'post_type' => 'wp_font_family', @@ -97,6 +117,10 @@ public static function create_font_family_post( $settings = array() ) { ) ) ); + + static::$post_ids_to_cleanup[] = $post_id; + + return $post_id; } /** @@ -143,26 +167,46 @@ public function test_font_families_no_autosave_routes() { } /** - * @covers WP_REST_Font_Families_Controller::get_context_param + * @doesNotPerformAssertions */ public function test_context_param() { - // Collection. - $request = new WP_REST_Request( 'OPTIONS', '/wp/v2/font-families' ); - $response = rest_get_server()->dispatch( $request ); - $data = $response->get_data(); - $this->assertArrayNotHasKey( 'allow_batch', $data['endpoints'][0] ); - $this->assertSame( 'view', $data['endpoints'][0]['args']['context']['default'] ); - $this->assertSame( array( 'view', 'embed', 'edit' ), $data['endpoints'][0]['args']['context']['enum'] ); + // See test_get_context_param(). + } - // Single. - $request = new WP_REST_Request( 'OPTIONS', '/wp/v2/font-families/' . self::$font_family_id1 ); + /** + * @dataProvider data_get_context_param + * + * @covers WP_REST_Font_Families_Controller::get_context_param + * + * @param bool $single_route Whether to test a single route. + */ + public function test_get_context_param( $single_route ) { + $route = '/wp/v2/font-families'; + if ( $single_route ) { + $route .= '/' . self::$font_family_id1; + } + + $request = new WP_REST_Request( 'OPTIONS', $route ); $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertArrayNotHasKey( 'allow_batch', $data['endpoints'][0] ); - $this->assertSame( 'view', $data['endpoints'][0]['args']['context']['default'] ); - $this->assertSame( array( 'view', 'embed', 'edit' ), $data['endpoints'][0]['args']['context']['enum'] ); + + $endpoint_data = $data['endpoints'][0]; + $this->assertArrayNotHasKey( 'allow_batch', $endpoint_data, 'The allow_batch property should not exist in the endpoint data.' ); + $this->assertSame( 'view', $endpoint_data['args']['context']['default'], 'The endpoint\'s args::context::default should be set to view.' ); + $this->assertSame( array( 'view', 'embed', 'edit' ), $endpoint_data['args']['context']['enum'], 'The endpoint\'s args::context::enum should be set to [ view, embed, edit ].' ); } + /** + * Data provider. + * + * @return array + */ + public function data_get_context_param() { + return array( + 'Collection' => array( false ), + 'Single' => array( true ), + ); + } /** * @covers WP_REST_Font_Faces_Controller::get_items @@ -173,11 +217,11 @@ public function test_get_items() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 200, $response->get_status() ); - $this->assertCount( 2, $data ); - $this->assertArrayHasKey( '_links', $data[0] ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200.' ); + $this->assertCount( 2, $data, 'There should be 2 properties in the response data.' ); + $this->assertArrayHasKey( '_links', $data[0], 'The _links property should exist in the response data 0.' ); $this->check_font_family_data( $data[0], self::$font_family_id2, $data[0]['_links'] ); - $this->assertArrayHasKey( '_links', $data[1] ); + $this->assertArrayHasKey( '_links', $data[1], 'The _links property should exist in the response data 1.' ); $this->check_font_family_data( $data[1], self::$font_family_id1, $data[1]['_links'] ); } @@ -193,9 +237,10 @@ public function test_get_items_by_slug() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 200, $response->get_status() ); - $this->assertCount( 1, $data ); - $this->assertSame( $font_family->ID, $data[0]['id'] ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200.' ); + $this->assertCount( 1, $data, 'There should be 2 properties in the response data.' ); + $this->assertArrayHasKey( 'id', $data[0], 'The id property should exist in the response data.' ); + $this->assertSame( $font_family->ID, $data[0]['id'], 'The id should match the expected ID in the response data.' ); } /** @@ -205,12 +250,12 @@ public function test_get_items_no_permission() { wp_set_current_user( 0 ); $request = new WP_REST_Request( 'GET', '/wp/v2/font-families' ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_cannot_read', $response, 401 ); + $this->assertErrorResponse( 'rest_cannot_read', $response, 401, 'The response should return an error with a "rest_cannot_read" code and 401 status.' ); wp_set_current_user( self::$editor_id ); $request = new WP_REST_Request( 'GET', '/wp/v2/font-families' ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_cannot_read', $response, 403 ); + $this->assertErrorResponse( 'rest_cannot_read', $response, 403, 'The response should return an error with a "rest_cannot_read" code and 403 status.' ); } /** @@ -222,7 +267,7 @@ public function test_get_item() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 200, $response->get_status() ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200.' ); $this->check_font_family_data( $data, self::$font_family_id1, $response->get_links() ); } @@ -236,19 +281,19 @@ public function test_get_item_embedded_font_faces() { $response = rest_get_server()->dispatch( $request ); $data = rest_get_server()->response_to_data( $response, true ); - $this->assertSame( 200, $response->get_status() ); - $this->assertArrayHasKey( '_embedded', $data ); - $this->assertArrayHasKey( 'font_faces', $data['_embedded'] ); - $this->assertCount( 2, $data['_embedded']['font_faces'] ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200.' ); + $this->assertArrayHasKey( '_embedded', $data, 'The _embedded property should exist in the response data.' ); + $this->assertArrayHasKey( 'font_faces', $data['_embedded'], 'The font_faces property should exist in _embedded data.' ); + $this->assertCount( 2, $data['_embedded']['font_faces'], 'There should be 2 font_faces in the _embedded data.' ); foreach ( $data['_embedded']['font_faces'] as $font_face ) { - $this->assertArrayHasKey( 'id', $font_face ); + $this->assertArrayHasKey( 'id', $font_face, 'The id property should exist in the _embedded font_face data.' ); $font_face_request = new WP_REST_Request( 'GET', '/wp/v2/font-families/' . self::$font_family_id1 . '/font-faces/' . $font_face['id'] ); $font_face_response = rest_get_server()->dispatch( $font_face_request ); $font_face_data = rest_get_server()->response_to_data( $font_face_response, true ); - $this->assertSame( $font_face_data, $font_face ); + $this->assertSame( $font_face_data, $font_face, 'The embedded font_face data should match when the data from a single request.' ); } } @@ -263,10 +308,8 @@ public function test_get_item_removes_extra_settings() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 200, $response->get_status() ); - $this->assertArrayNotHasKey( 'fontFace', $data['font_family_settings'] ); - - wp_delete_post( $font_family_id, true ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200.' ); + $this->assertArrayNotHasKey( 'fontFace', $data['font_family_settings'], 'The fontFace property should not exist in the font_family_settings data.' ); } /** @@ -281,6 +324,8 @@ public function test_get_item_malformed_post_content_returns_empty_settings() { ) ); + static::$post_ids_to_cleanup[] = $font_family_id; + $empty_settings = array( 'name' => '', // Slug will default to the post id. @@ -294,10 +339,8 @@ public function test_get_item_malformed_post_content_returns_empty_settings() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 200, $response->get_status() ); - $this->assertSame( $empty_settings, $data['font_family_settings'] ); - - wp_delete_post( $font_family_id, true ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200.' ); + $this->assertSame( $empty_settings, $data['font_family_settings'], 'The empty settings should exist in the font_family_settings data.' ); } /** @@ -318,11 +361,11 @@ public function test_get_item_no_permission() { $request = new WP_REST_Request( 'GET', '/wp/v2/font-families/' . self::$font_family_id1 ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_cannot_read', $response, 401 ); + $this->assertErrorResponse( 'rest_cannot_read', $response, 401, 'The response should return an error with a "rest_cannot_read" code and 401 status.' ); wp_set_current_user( self::$editor_id ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_cannot_read', $response, 403 ); + $this->assertErrorResponse( 'rest_cannot_read', $response, 403, 'The response should return an error with a "rest_cannot_read" code and 403 status.' ); } /** @@ -338,14 +381,12 @@ public function test_create_item() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 201, $response->get_status() ); + $this->assertSame( 201, $response->get_status(), 'The response status should be 201.' ); $this->check_font_family_data( $data, $data['id'], $response->get_links() ); $reponse_settings = $data['font_family_settings']; - $this->assertSame( $settings, $reponse_settings ); - $this->assertEmpty( $data['font_faces'] ); - - wp_delete_post( $data['id'], true ); + $this->assertSame( $settings, $reponse_settings, 'The expected settings should exist in the font_family_settings data.' ); + $this->assertEmpty( $data['font_faces'], 'The font_faces should be empty or not exist in the response data.' ); } /** @@ -360,17 +401,19 @@ public function test_create_item_default_theme_json_version() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 201, $response->get_status() ); - $this->assertArrayHasKey( 'theme_json_version', $data ); - $this->assertSame( 2, $data['theme_json_version'], 'The default theme.json version should be 2.' ); + static::$post_ids_to_cleanup[] = $data['id']; - wp_delete_post( $data['id'], true ); + $this->assertSame( 201, $response->get_status(), 'The response status should be 201.' ); + $this->assertArrayHasKey( 'theme_json_version', $data, 'The theme_json_version property should exist in the response data.' ); + $this->assertSame( 2, $data['theme_json_version'], 'The default theme.json version should be 2.' ); } /** * @dataProvider data_create_item_invalid_theme_json_version * * @covers WP_REST_Font_Faces_Controller::create_item + * + * @param int $theme_json_version Version to test. */ public function test_create_item_invalid_theme_json_version( $theme_json_version ) { wp_set_current_user( self::$admin_id ); @@ -382,6 +425,11 @@ public function test_create_item_invalid_theme_json_version( $theme_json_version $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); } + /** + * Data provider. + * + * @return array + */ public function data_create_item_invalid_theme_json_version() { return array( array( 1 ), @@ -393,6 +441,8 @@ public function data_create_item_invalid_theme_json_version() { * @dataProvider data_create_item_with_default_preview * * @covers WP_REST_Font_Faces_Controller::sanitize_font_family_settings + * + * @param array $settings Settings to test. */ public function test_create_item_with_default_preview( $settings ) { wp_set_current_user( self::$admin_id ); @@ -402,14 +452,19 @@ public function test_create_item_with_default_preview( $settings ) { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 201, $response->get_status() ); - $response_settings = $data['font_family_settings']; - $this->assertArrayHasKey( 'preview', $response_settings ); - $this->assertSame( '', $response_settings['preview'] ); + static::$post_ids_to_cleanup[] = $data['id']; - wp_delete_post( $data['id'], true ); + $this->assertSame( 201, $response->get_status(), 'The response status should be 201.' ); + $response_settings = $data['font_family_settings']; + $this->assertArrayHasKey( 'preview', $response_settings, 'The preview property should exist in the font_family_settings data.' ); + $this->assertSame( '', $response_settings['preview'], 'The preview data should be an empty string.' ); } + /** + * Data provider. + * + * @return array + */ public function data_create_item_with_default_preview() { $default_settings = array( 'name' => 'Open Sans', @@ -430,6 +485,8 @@ public function data_create_item_with_default_preview() { * @dataProvider data_create_item_invalid_settings * * @covers WP_REST_Font_Faces_Controller::validate_create_font_face_settings + * + * @param array $settings Settings to test. */ public function test_create_item_invalid_settings( $settings ) { wp_set_current_user( self::$admin_id ); @@ -441,6 +498,11 @@ public function test_create_item_invalid_settings( $settings ) { $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); } + /** + * Data provider. + * + * @return array + */ public function data_create_item_invalid_settings() { return array( 'Missing name' => array( @@ -484,10 +546,10 @@ public function test_create_item_invalid_settings_json() { $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); + $this->assertErrorResponse( 'rest_invalid_param', $response, 400, 'The response should return an error for "rest_invalid_param" with 400 status.' ); $expected_message = 'font_family_settings parameter must be a valid JSON string.'; $message = $response->as_error()->get_all_error_data()[0]['params']['font_family_settings']; - $this->assertSame( $expected_message, $message ); + $this->assertSame( $expected_message, $message, 'The response error message should match.' ); } /** @@ -501,10 +563,10 @@ public function test_create_item_with_duplicate_slug() { $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_duplicate_font_family', $response, 400 ); + $this->assertErrorResponse( 'rest_duplicate_font_family', $response, 400, 'The response should return an error for "rest_duplicate_font_family" with 400 status.' ); $expected_message = 'A font family with slug "helvetica" already exists.'; $message = $response->as_error()->get_error_messages()[0]; - $this->assertSame( $expected_message, $message ); + $this->assertSame( $expected_message, $message, 'The response error message should match.' ); } /** @@ -516,7 +578,7 @@ public function test_create_item_no_permission() { $request = new WP_REST_Request( 'POST', '/wp/v2/font-families' ); $request->set_param( 'font_family_settings', wp_json_encode( $settings ) ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_cannot_create', $response, 401 ); + $this->assertErrorResponse( 'rest_cannot_create', $response, 401, 'The response should return an error for "rest_cannot_create" with 401 status.' ); wp_set_current_user( self::$editor_id ); $request = new WP_REST_Request( 'POST', '/wp/v2/font-families' ); @@ -532,7 +594,7 @@ public function test_create_item_no_permission() { ) ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_cannot_create', $response, 403 ); + $this->assertErrorResponse( 'rest_cannot_create', $response, 403, 'The response should return an error for "rest_cannot_create" with 403 status.' ); } /** @@ -556,7 +618,7 @@ public function test_update_item() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 200, $response->get_status() ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200.' ); $this->check_font_family_data( $data, $font_family_id, $response->get_links() ); $expected_settings = array( @@ -565,14 +627,15 @@ public function test_update_item() { 'fontFamily' => $settings['fontFamily'], 'preview' => $settings['preview'], ); - $this->assertSame( $expected_settings, $data['font_family_settings'] ); - - wp_delete_post( $font_family_id, true ); + $this->assertSame( $expected_settings, $data['font_family_settings'], 'The response font_family_settings should match expected settings.' ); } /** * @dataProvider data_update_item_individual_settings + * * @covers WP_REST_Font_Families_Controller::update_item + * + * @param array $settings Settings to test. */ public function test_update_item_individual_settings( $settings ) { wp_set_current_user( self::$admin_id ); @@ -583,15 +646,18 @@ public function test_update_item_individual_settings( $settings ) { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 200, $response->get_status() ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200.' ); $key = key( $settings ); $value = current( $settings ); - $this->assertArrayHasKey( $key, $data['font_family_settings'] ); - $this->assertSame( $value, $data['font_family_settings'][ $key ] ); - - wp_delete_post( $font_family_id, true ); + $this->assertArrayHasKey( $key, $data['font_family_settings'], 'The expected key should exist in the font_family_settings data.' ); + $this->assertSame( $value, $data['font_family_settings'][ $key ], 'The font_family_settings data should match.' ); } + /** + * Data provider. + * + * @return array + */ public function data_update_item_individual_settings() { return array( array( array( 'name' => 'Opened Sans' ) ), @@ -602,10 +668,13 @@ public function data_update_item_individual_settings() { ); } - /** + /** * @dataProvider data_update_item_santize_font_family * * @covers WP_REST_Font_Families_Controller::sanitize_font_face_settings + * + * @param string $font_family_setting Font family setting to test. + * @param string $expected Expected result. */ public function test_update_item_santize_font_family( $font_family_setting, $expected ) { wp_set_current_user( self::$admin_id ); @@ -616,17 +685,29 @@ public function test_update_item_santize_font_family( $font_family_setting, $exp $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 200, $response->get_status() ); - $this->assertSame( $expected, $data['font_family_settings']['fontFamily'] ); - - wp_delete_post( $font_family_id, true ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200.' ); + $this->assertSame( $expected, $data['font_family_settings']['fontFamily'], 'The font family should match.' ); } + /** + * Data provider. + * + * @return array + */ public function data_update_item_santize_font_family() { return array( - array( 'Libre Barcode 128 Text', '"Libre Barcode 128 Text"' ), - array( 'B612 Mono', '"B612 Mono"' ), - array( 'Open Sans, Noto Sans, sans-serif', '"Open Sans", "Noto Sans", sans-serif' ), + 'multiword font with integer' => array( + 'font_family_setting' => 'Libre Barcode 128 Text', + 'expected' => '"Libre Barcode 128 Text"', + ), + 'multiword font' => array( + 'font_family_setting' => 'B612 Mono', + 'expected' => '"B612 Mono"', + ), + 'comma-separated fonts' => array( + 'font_family_setting' => 'Open Sans, Noto Sans, sans-serif', + 'expected' => '"Open Sans", "Noto Sans", sans-serif', + ), ); } @@ -634,6 +715,8 @@ public function data_update_item_santize_font_family() { * @dataProvider data_update_item_invalid_settings * * @covers WP_REST_Font_Faces_Controller::update_item + * + * @param array $settings Settings to test. */ public function test_update_item_empty_settings( $settings ) { wp_set_current_user( self::$admin_id ); @@ -646,6 +729,11 @@ public function test_update_item_empty_settings( $settings ) { $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); } + /** + * Data provider. + * + * @return array + */ public function data_update_item_invalid_settings() { return array( 'Empty name' => array( @@ -675,10 +763,10 @@ public function test_update_item_update_slug_not_allowed() { ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); + $this->assertErrorResponse( 'rest_invalid_param', $response, 400, 'The response should return an error for "rest_invalid_param" with 400 status.' ); $expected_message = 'font_family_settings[slug] cannot be updated.'; $message = $response->as_error()->get_all_error_data()[0]['params']['font_family_settings']; - $this->assertSame( $expected_message, $message ); + $this->assertSame( $expected_message, $message, 'The response error message should match.' ); } /** @@ -691,7 +779,7 @@ public function test_update_item_invalid_font_family_id() { $request = new WP_REST_Request( 'POST', '/wp/v2/font-families/' . REST_TESTS_IMPOSSIBLY_HIGH_NUMBER ); $request->set_param( 'font_family_settings', wp_json_encode( $settings ) ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_post_invalid_id', $response, 404 ); + $this->assertErrorResponse( 'rest_post_invalid_id', $response, 404, 'The response should return an error for "rest_post_invalid_id" with 404 status.' ); } /** @@ -704,13 +792,13 @@ public function test_update_item_no_permission() { $request = new WP_REST_Request( 'POST', '/wp/v2/font-families/' . self::$font_family_id1 ); $request->set_param( 'font_family_settings', wp_json_encode( $settings ) ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_cannot_edit', $response, 401 ); + $this->assertErrorResponse( 'rest_cannot_edit', $response, 401, 'The response should return an error for "rest_cannot_edit" with 401 status for an invalid user.' ); wp_set_current_user( self::$editor_id ); $request = new WP_REST_Request( 'POST', '/wp/v2/font-families/' . self::$font_family_id1 ); $request->set_param( 'font_family_settings', wp_json_encode( $settings ) ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_cannot_edit', $response, 403 ); + $this->assertErrorResponse( 'rest_cannot_edit', $response, 403, 'The response should return an error for "rest_cannot_edit" with 403 status for a user without permission.' ); } @@ -724,8 +812,8 @@ public function test_delete_item() { $request['force'] = true; $response = rest_get_server()->dispatch( $request ); - $this->assertSame( 200, $response->get_status() ); - $this->assertNull( get_post( $font_family_id ) ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200.' ); + $this->assertNull( get_post( $font_family_id ), 'The post should not exist after deleting.' ); } /** @@ -738,17 +826,15 @@ public function test_delete_item_no_trash() { // Attempt trashing. $request = new WP_REST_Request( 'DELETE', '/wp/v2/font-families/' . $font_family_id ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_trash_not_supported', $response, 501 ); + $this->assertErrorResponse( 'rest_trash_not_supported', $response, 501, 'The response should return an error for "rest_trash_not_supported" with 501 status.' ); $request->set_param( 'force', 'false' ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_trash_not_supported', $response, 501 ); + $this->assertErrorResponse( 'rest_trash_not_supported', $response, 501, 'When "force" is false, the response should return an error for "rest_trash_not_supported" with 501 status.' ); // Ensure the post still exists. $post = get_post( $font_family_id ); - $this->assertNotEmpty( $post ); - - wp_delete_post( $font_family_id, true ); + $this->assertNotEmpty( $post, 'The post should still exist.' ); } /** @@ -770,14 +856,12 @@ public function test_delete_item_no_permissions() { wp_set_current_user( 0 ); $request = new WP_REST_Request( 'DELETE', '/wp/v2/font-families/' . $font_family_id ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_cannot_delete', $response, 401 ); + $this->assertErrorResponse( 'rest_cannot_delete', $response, 401, 'The response should return an error for "rest_cannot_delete" with 401 status for an invalid user.' ); wp_set_current_user( self::$editor_id ); $request = new WP_REST_Request( 'DELETE', '/wp/v2/font-families/' . $font_family_id ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'rest_cannot_delete', $response, 403 ); - - wp_delete_post( $font_family_id, true ); + $this->assertErrorResponse( 'rest_cannot_delete', $response, 403, 'The response should return an error for "rest_cannot_delete" with 403 status for a user without permission.' ); } /** @@ -789,7 +873,7 @@ public function test_prepare_item() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 200, $response->get_status() ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200.' ); $this->check_font_family_data( $data, self::$font_family_id2, $response->get_links() ); } @@ -801,23 +885,24 @@ public function test_get_item_schema() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); - $this->assertSame( 200, $response->get_status() ); + $this->assertSame( 200, $response->get_status(), 'The response status should be 200.' ); $properties = $data['schema']['properties']; - $this->assertCount( 4, $properties ); - $this->assertArrayHasKey( 'id', $properties ); - $this->assertArrayHasKey( 'theme_json_version', $properties ); - $this->assertArrayHasKey( 'font_faces', $properties ); - $this->assertArrayHasKey( 'font_family_settings', $properties ); + $this->assertCount( 4, $properties, 'There should be 4 properties in the schema::properties data.' ); + $this->assertArrayHasKey( 'id', $properties, 'The id property should exist in the schema::properties data.' ); + $this->assertArrayHasKey( 'theme_json_version', $properties, 'The theme_json_version property should exist in the schema::properties data.' ); + $this->assertArrayHasKey( 'font_faces', $properties, 'The font_faces property should exist in the schema::properties data.' ); + $this->assertArrayHasKey( 'font_family_settings', $properties, 'The font_family_settings property should exist in the schema::properties data.' ); } protected function check_font_family_data( $data, $post_id, $links ) { - $post = get_post( $post_id ); + static::$post_ids_to_cleanup[] = $post_id; + $post = get_post( $post_id ); - $this->assertArrayHasKey( 'id', $data ); - $this->assertSame( $post->ID, $data['id'] ); + $this->assertArrayHasKey( 'id', $data, 'The id property should exist in response data.' ); + $this->assertSame( $post->ID, $data['id'], 'The "id" from the response data should match the post ID.' ); - $this->assertArrayHasKey( 'theme_json_version', $data ); - $this->assertSame( WP_Theme_JSON::LATEST_SCHEMA, $data['theme_json_version'] ); + $this->assertArrayHasKey( 'theme_json_version', $data, 'The theme_json_version property should exist in response data.' ); + $this->assertSame( WP_Theme_JSON::LATEST_SCHEMA, $data['theme_json_version'], 'The "theme_json_version" from the response data should match WP_Theme_JSON::LATEST_SCHEMA.' ); $font_face_ids = get_children( array( @@ -828,13 +913,13 @@ protected function check_font_family_data( $data, $post_id, $links ) { 'orderby' => 'ID', ) ); - $this->assertArrayHasKey( 'font_faces', $data ); + $this->assertArrayHasKey( 'font_faces', $data, 'The font_faces property should exist in the response data.' ); foreach ( $font_face_ids as $font_face_id ) { - $this->assertContains( $font_face_id, $data['font_faces'] ); + $this->assertContains( $font_face_id, $data['font_faces'], 'The ID is in the font_faces data.' ); } - $this->assertArrayHasKey( 'font_family_settings', $data ); + $this->assertArrayHasKey( 'font_family_settings', $data, 'The font_family_settings property should exist in the response data.' ); $settings = $data['font_family_settings']; $expected_settings = array( 'name' => $post->post_title, @@ -842,11 +927,13 @@ protected function check_font_family_data( $data, $post_id, $links ) { 'fontFamily' => $settings['fontFamily'], 'preview' => $settings['preview'], ); - $this->assertSame( $expected_settings, $settings ); + $this->assertSame( $expected_settings, $settings, 'The font_family_settings should match.' ); - $this->assertNotEmpty( $links ); - $this->assertSame( rest_url( 'wp/v2/font-families/' . $post->ID ), $links['self'][0]['href'] ); - $this->assertSame( rest_url( 'wp/v2/font-families' ), $links['collection'][0]['href'] ); + $this->assertNotEmpty( $links, 'The links should not be empty in the response data.' ); + $expected = rest_url( 'wp/v2/font-families/' . $post->ID ); + $this->assertSame( $expected, $links['self'][0]['href'], 'The links URL from the response data should match the post\'s REST endpoint.' ); + $expected = rest_url( 'wp/v2/font-families' ); + $this->assertSame( $expected, $links['collection'][0]['href'], 'The links collection URL from the response data should match the REST endpoint.' ); if ( ! $font_face_ids ) { return; @@ -855,12 +942,13 @@ protected function check_font_family_data( $data, $post_id, $links ) { // Check font_face links, if present. $this->assertArrayHasKey( 'font_faces', $links ); foreach ( $links['font_faces'] as $index => $link ) { - $this->assertSame( rest_url( 'wp/v2/font-families/' . $post->ID . '/font-faces/' . $font_face_ids[ $index ] ), $link['href'] ); + $expected = rest_url( 'wp/v2/font-families/' . $post->ID . '/font-faces/' . $font_face_ids[ $index ] ); + $this->assertSame( $expected, $link['href'], 'The links for a font faces URL from the response data should match the REST endpoint.' ); $embeddable = isset( $link['attributes']['embeddable'] ) ? $link['attributes']['embeddable'] : $link['embeddable']; - $this->assertTrue( $embeddable ); + $this->assertTrue( $embeddable, 'The embeddable should be true.' ); } } } From 51b732cdde877c594d3a5321bc22de85eedde6ce Mon Sep 17 00:00:00 2001 From: Sarah Norris <1645628+mikachan@users.noreply.github.com> Date: Fri, 2 Feb 2024 00:05:00 +0000 Subject: [PATCH 017/273] Font Library modal: Try to improve checkbox labelling (#58339) * Revert allowing label to be false * Replace custom labels with label prop * Try using aria-labelledby * Make FontDemo clickable * Try hiding label with CSS * Update components changelog * Fix font library e2e test * Remove checkboxId prop * Conditionally render label * Try using separate label tag * Update e2e test * Update snapshot * Update CheckboxControl test --- packages/components/CHANGELOG.md | 4 ++++ .../components/src/checkbox-control/README.md | 3 +-- .../src/checkbox-control/test/index.tsx | 4 ++-- .../components/src/checkbox-control/types.ts | 5 ++--- .../collection-font-variant.js | 16 +++++++++------- .../font-library-modal/library-font-variant.js | 16 +++++++++------- 6 files changed, 27 insertions(+), 21 deletions(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 20b64d798f403..94f69636b3e05 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -21,6 +21,10 @@ - `Tabs`: improve controlled mode focus handling and keyboard navigation ([#57696](https://github.com/WordPress/gutenberg/pull/57696)). - Expand theming support in the `COLORS` variable object ([#58097](https://github.com/WordPress/gutenberg/pull/58097)). +### Enhancements + +- `CheckboxControl`: Remove ability for label prop to be false ([#58339](https://github.com/WordPress/gutenberg/pull/58339)). + ## 25.16.0 (2024-01-24) ### Enhancements diff --git a/packages/components/src/checkbox-control/README.md b/packages/components/src/checkbox-control/README.md index 8044cef0535ea..e89ee489feca9 100644 --- a/packages/components/src/checkbox-control/README.md +++ b/packages/components/src/checkbox-control/README.md @@ -69,12 +69,11 @@ const MyCheckboxControl = () => { The set of props accepted by the component will be specified below. Props not included in this set will be applied to the input element. -#### `label`: `string|false` +#### `label`: `string` A label for the input field, that appears at the side of the checkbox. The prop will be rendered as content a label element. If no prop is passed an empty label is rendered. -If the prop is set to false no label is rendered. - Required: No diff --git a/packages/components/src/checkbox-control/test/index.tsx b/packages/components/src/checkbox-control/test/index.tsx index 55e1447661392..411d20e820d3c 100644 --- a/packages/components/src/checkbox-control/test/index.tsx +++ b/packages/components/src/checkbox-control/test/index.tsx @@ -60,8 +60,8 @@ describe( 'CheckboxControl', () => { expect( label ).toBeInTheDocument(); } ); - it( 'should not render label element if label is set to false', () => { - render( ); + it( 'should not render label element if label is not set', () => { + render( ); const label = screen.queryByRole( 'label' ); expect( label ).not.toBeInTheDocument(); diff --git a/packages/components/src/checkbox-control/types.ts b/packages/components/src/checkbox-control/types.ts index 48a1d02affb70..07ab55493fc66 100644 --- a/packages/components/src/checkbox-control/types.ts +++ b/packages/components/src/checkbox-control/types.ts @@ -19,10 +19,9 @@ export type CheckboxControlProps = Pick< /** * A label for the input field, that appears at the side of the checkbox. * The prop will be rendered as content a label element. If no prop is - * passed an empty label is rendered. If the prop is set to false no label - * is rendered. + * passed an empty label is rendered. */ - label?: string | false; + label?: string; /** * If checked is true the checkbox will be checked. If checked is false the * checkbox will be unchecked. If no value is passed the checkbox will be diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/collection-font-variant.js b/packages/edit-site/src/components/global-styles/font-library-modal/collection-font-variant.js index ab090a0cd0b4d..0db8932a98e34 100644 --- a/packages/edit-site/src/components/global-styles/font-library-modal/collection-font-variant.js +++ b/packages/edit-site/src/components/global-styles/font-library-modal/collection-font-variant.js @@ -35,21 +35,23 @@ function CollectionFontVariant( { ); return ( -