Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stabilise role attribute property #65484

Merged
merged 15 commits into from
Sep 24, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ describe( 'use-transformed-patterns', () => {
},
content: {
type: 'boolean',
__experimentalRole: 'content',
role: 'content',
},
level: {
type: 'number',
__experimentalRole: 'content',
role: 'content',
},
color: {
type: 'string',
__experimentalRole: 'other',
role: 'other',
},
},
save() {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ describe( 'BlockSwitcher - utils', () => {
},
content: {
type: 'boolean',
__experimentalRole: 'content',
role: 'content',
},
level: {
type: 'number',
__experimentalRole: 'content',
role: 'content',
},
color: {
type: 'string',
__experimentalRole: 'other',
role: 'other',
},
},
save() {},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* WordPress dependencies
*/
import { __experimentalGetBlockAttributesNamesByRole as getBlockAttributesNamesByRole } from '@wordpress/blocks';
import { getBlockAttributesNamesByRole } from '@wordpress/blocks';

/**
* Try to find a matching block by a block's name in a provided
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { chevronDown } from '@wordpress/icons';
*/
import BlockIcon from '../block-icon';
import { store as blockEditorStore } from '../../store';
import { unlock } from '../../lock-unlock';

function VariationsButtons( {
className,
Expand Down Expand Up @@ -142,18 +143,16 @@ function __experimentalBlockVariationTransforms( { blockClientId } ) {
const { updateBlockAttributes } = useDispatch( blockEditorStore );
const { activeBlockVariation, variations, isContentOnly } = useSelect(
( select ) => {
const {
getActiveBlockVariation,
getBlockVariations,
__experimentalHasContentRoleAttribute,
} = select( blocksStore );
const { getActiveBlockVariation, getBlockVariations } =
select( blocksStore );

const { getBlockName, getBlockAttributes, getBlockEditingMode } =
select( blockEditorStore );

const name = blockClientId && getBlockName( blockClientId );

const isContentBlock =
__experimentalHasContentRoleAttribute( name );
const { hasContentRoleAttribute } = unlock( select( blocksStore ) );
const isContentBlock = hasContentRoleAttribute( name );

return {
activeBlockVariation: getActiveBlockVariation(
Expand Down
21 changes: 11 additions & 10 deletions packages/block-editor/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -2479,7 +2479,7 @@ export const __experimentalGetPatternsByBlockTypes = createRegistrySelector(
* Determines the items that appear in the available pattern transforms list.
*
* For now we only handle blocks without InnerBlocks and take into account
* the `__experimentalRole` property of blocks' attributes for the transformation.
* the `role` property of blocks' attributes for the transformation.
*
* We return the first set of possible eligible block patterns,
* by checking the `blockTypes` property. We still have to recurse through
Expand All @@ -2501,7 +2501,7 @@ export const __experimentalGetPatternTransformItems = createRegistrySelector(
}
/**
* For now we only handle blocks without InnerBlocks and take into account
* the `__experimentalRole` property of blocks' attributes for the transformation.
* the `role` property of blocks' attributes for the transformation.
* Note that the blocks have been retrieved through `getBlock`, which doesn't
* return the inner blocks of an inner block controller, so we still need
* to check for this case too.
Expand Down Expand Up @@ -3006,10 +3006,11 @@ export const getBlockEditingMode = createRegistrySelector(
// The rest of the blocks depend on whether they are content blocks or not.
// This "flattens" the sections tree.
const name = getBlockName( state, clientId );
const isContent =
select( blocksStore ).__experimentalHasContentRoleAttribute(
name
);
const { hasContentRoleAttribute } = unlock(
select( blocksStore )
);
const isContent = hasContentRoleAttribute( name );

return isContent ? 'contentOnly' : 'disabled';
}

Expand All @@ -3029,10 +3030,10 @@ export const getBlockEditingMode = createRegistrySelector(
// If the parent of the block is contentOnly locked, check whether it's a content block.
if ( templateLock === 'contentOnly' ) {
const name = getBlockName( state, clientId );
const isContent =
select( blocksStore ).__experimentalHasContentRoleAttribute(
name
);
const { hasContentRoleAttribute } = unlock(
select( blocksStore )
);
const isContent = hasContentRoleAttribute( name );
return isContent ? 'contentOnly' : 'disabled';
}
// Otherwise, check if there's an ancestor that is contentOnly
Expand Down
4 changes: 2 additions & 2 deletions packages/block-editor/src/store/test/private-selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,10 @@ describe( 'private selectors', () => {
blockEditingModes: new Map( [] ),
};

const __experimentalHasContentRoleAttribute = jest.fn( () => false );
const hasContentRoleAttribute = jest.fn( () => false );
getBlockEditingMode.registry = {
select: jest.fn( () => ( {
__experimentalHasContentRoleAttribute,
hasContentRoleAttribute,
Comment on lines -127 to +130
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should these tests be moved to selectors tests now?

} ) ),
};

Expand Down
22 changes: 13 additions & 9 deletions packages/block-editor/src/store/test/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { select, dispatch } from '@wordpress/data';
import * as selectors from '../selectors';
import { store } from '../';
import { sectionRootClientIdKey } from '../private-keys';
import { lock } from '../../lock-unlock';

const {
getBlockName,
Expand Down Expand Up @@ -4475,14 +4476,14 @@ describe( 'getBlockEditingMode', () => {
},
};

const __experimentalHasContentRoleAttribute = jest.fn( ( name ) => {
// consider paragraphs as content blocks.
return name === 'core/p';
} );
const hasContentRoleAttribute = jest.fn( () => false );

const fauxPrivateAPIs = {};

lock( fauxPrivateAPIs, { hasContentRoleAttribute } );
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a correct mock to me.


getBlockEditingMode.registry = {
select: jest.fn( () => ( {
__experimentalHasContentRoleAttribute,
} ) ),
select: jest.fn( () => fauxPrivateAPIs ),
};

it( 'should return default by default', () => {
Expand Down Expand Up @@ -4586,7 +4587,7 @@ describe( 'getBlockEditingMode', () => {
},
},
};
__experimentalHasContentRoleAttribute.mockReturnValueOnce( false );
hasContentRoleAttribute.mockReturnValueOnce( false );
expect(
getBlockEditingMode( state, 'b3247f75-fd94-4fef-97f9-5bfd162cc416' )
).toBe( 'disabled' );
Expand All @@ -4602,7 +4603,7 @@ describe( 'getBlockEditingMode', () => {
},
},
};
__experimentalHasContentRoleAttribute.mockReturnValueOnce( true );
hasContentRoleAttribute.mockReturnValueOnce( true );
expect(
getBlockEditingMode( state, 'b3247f75-fd94-4fef-97f9-5bfd162cc416' )
).toBe( 'contentOnly' );
Expand Down Expand Up @@ -4642,12 +4643,15 @@ describe( 'getBlockEditingMode', () => {
} );

it( 'in navigation mode, blocks with content attributes within sections are contentOnly', () => {
hasContentRoleAttribute.mockReturnValueOnce( true );
expect(
getBlockEditingMode(
navigationModeStateWithRootSection,
'b3247f75-fd94-4fef-97f9-5bfd162cc416'
)
).toBe( 'contentOnly' );

hasContentRoleAttribute.mockReturnValueOnce( true );
Comment on lines 4645 to +4654
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this test needs to include a test case that asserts the inverse cases. What do you think?

expect(
getBlockEditingMode(
navigationModeStateWithRootSection,
Expand Down
8 changes: 4 additions & 4 deletions packages/block-library/src/audio/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,24 @@
"attributes": {
"blob": {
"type": "string",
"__experimentalRole": "local"
"role": "local"
},
"src": {
"type": "string",
"source": "attribute",
"selector": "audio",
"attribute": "src",
"__experimentalRole": "content"
"role": "content"
},
"caption": {
"type": "rich-text",
"source": "rich-text",
"selector": "figcaption",
"__experimentalRole": "content"
"role": "content"
},
"id": {
"type": "number",
"__experimentalRole": "content"
"role": "content"
},
"autoplay": {
"type": "boolean",
Expand Down
10 changes: 5 additions & 5 deletions packages/block-library/src/button/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,34 +26,34 @@
"source": "attribute",
"selector": "a",
"attribute": "href",
"__experimentalRole": "content"
"role": "content"
},
"title": {
"type": "string",
"source": "attribute",
"selector": "a,button",
"attribute": "title",
"__experimentalRole": "content"
"role": "content"
},
"text": {
"type": "rich-text",
"source": "rich-text",
"selector": "a,button",
"__experimentalRole": "content"
"role": "content"
},
"linkTarget": {
"type": "string",
"source": "attribute",
"selector": "a",
"attribute": "target",
"__experimentalRole": "content"
"role": "content"
},
"rel": {
"type": "string",
"source": "attribute",
"selector": "a",
"attribute": "rel",
"__experimentalRole": "content"
"role": "content"
},
"placeholder": {
"type": "string"
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/categories/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
},
"label": {
"type": "string",
"__experimentalRole": "content"
"role": "content"
},
"showLabel": {
"type": "boolean",
Expand Down
12 changes: 6 additions & 6 deletions packages/block-library/src/embed/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,21 @@
"attributes": {
"url": {
"type": "string",
"__experimentalRole": "content"
"role": "content"
},
"caption": {
"type": "rich-text",
"source": "rich-text",
"selector": "figcaption",
"__experimentalRole": "content"
"role": "content"
},
"type": {
"type": "string",
"__experimentalRole": "content"
"role": "content"
},
"providerNameSlug": {
"type": "string",
"__experimentalRole": "content"
"role": "content"
},
"allowResponsive": {
"type": "boolean",
Expand All @@ -32,12 +32,12 @@
"responsive": {
"type": "boolean",
"default": false,
"__experimentalRole": "content"
"role": "content"
},
"previewable": {
"type": "boolean",
"default": true,
"__experimentalRole": "content"
"role": "content"
}
},
"supports": {
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/file/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
},
"blob": {
"type": "string",
"__experimentalRole": "local"
"role": "local"
},
"href": {
"type": "string"
Expand Down
4 changes: 2 additions & 2 deletions packages/block-library/src/form-input/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"default": "Label",
"selector": ".wp-block-form-input__label-content",
"source": "rich-text",
"__experimentalRole": "content"
"role": "content"
},
"inlineLabel": {
"type": "boolean",
Expand All @@ -41,7 +41,7 @@
"selector": ".wp-block-form-input__input",
"source": "attribute",
"attribute": "placeholder",
"__experimentalRole": "content"
"role": "content"
},
"value": {
"type": "string",
Expand Down
8 changes: 4 additions & 4 deletions packages/block-library/src/form-input/deprecated.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const v2 = {
default: 'Label',
selector: '.wp-block-form-input__label-content',
source: 'html',
__experimentalRole: 'content',
role: 'content',
},
inlineLabel: {
type: 'boolean',
Expand All @@ -59,7 +59,7 @@ const v2 = {
selector: '.wp-block-form-input__input',
source: 'attribute',
attribute: 'placeholder',
__experimentalRole: 'content',
role: 'content',
},
value: {
type: 'string',
Expand Down Expand Up @@ -155,7 +155,7 @@ const v1 = {
default: 'Label',
selector: '.wp-block-form-input__label-content',
source: 'html',
__experimentalRole: 'content',
role: 'content',
},
inlineLabel: {
type: 'boolean',
Expand All @@ -173,7 +173,7 @@ const v1 = {
selector: '.wp-block-form-input__input',
source: 'attribute',
attribute: 'placeholder',
__experimentalRole: 'content',
role: 'content',
},
value: {
type: 'string',
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/heading/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"type": "rich-text",
"source": "rich-text",
"selector": "h1,h2,h3,h4,h5,h6",
"__experimentalRole": "content"
"role": "content"
},
"level": {
"type": "number",
Expand Down
Loading
Loading