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

Block bindings: Bring bindings UI in Site Editor #64072

Merged
merged 30 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
70094be
Initial commit. Add meta field to post types.
cbravobernal Jul 29, 2024
862f4d1
Add post meta
cbravobernal Jul 31, 2024
9b9d0b4
Add todos
cbravobernal Jul 31, 2024
ff8ebc5
Add fields in all postType
SantosGuillamot Aug 2, 2024
233bf97
WIP: Add first version to link templates and entities
SantosGuillamot Aug 2, 2024
9e3e1e8
Revert "WIP: Add first version to link templates and entities"
SantosGuillamot Aug 13, 2024
6ded461
Only expose public fields
SantosGuillamot Aug 13, 2024
8a69d40
Add subtype to meta properties
SantosGuillamot Aug 13, 2024
6c32b80
Render the appropriate fields depending on the postType in templates
SantosGuillamot Aug 13, 2024
e85aac7
Use context postType when available
SantosGuillamot Aug 14, 2024
304c28e
Fetch the data on render, preventing one click needed
cbravobernal Aug 29, 2024
fac01f9
Yoda conditions..
cbravobernal Aug 29, 2024
a4b3dc4
Try: Expose registered meta fields in schema
SantosGuillamot Sep 2, 2024
851ae0d
Try: Create a resolver to get registered post meta
SantosGuillamot Sep 3, 2024
9c11f15
Use rest namespace
cbravobernal Sep 4, 2024
01afd0d
Move actions and selectors to private.
cbravobernal Sep 4, 2024
e49be26
Add unlocking and import
cbravobernal Sep 5, 2024
3212804
Merge useSelect
cbravobernal Sep 5, 2024
116fc09
Fix duplicated
cbravobernal Sep 5, 2024
66ab07d
Add object_subtype to schema
SantosGuillamot Sep 6, 2024
56cd505
Update docs to object_subtype
cbravobernal Sep 6, 2024
25196ff
Add explanatory comment
cbravobernal Sep 6, 2024
6cb6a38
Block Bindings: Use default values in connected custom fields in temp…
SantosGuillamot Sep 10, 2024
0144e34
Try removing all object subtype
cbravobernal Sep 10, 2024
4cac9bd
Fix e2e
cbravobernal Sep 10, 2024
af79c2b
Update code
cbravobernal Sep 11, 2024
782dd99
Fix `useSelect` warning
SantosGuillamot Sep 16, 2024
b4a11c0
Remove old comment
SantosGuillamot Sep 17, 2024
42ef0c3
Remove support for generic templates
SantosGuillamot Sep 17, 2024
deb3e2d
Revert changes to e2e tests
SantosGuillamot Sep 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 58 additions & 43 deletions packages/block-editor/src/hooks/block-bindings.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import { store as blockEditorStore } from '../store';

const { DropdownMenuV2 } = unlock( componentsPrivateApis );

const EMPTY_OBJECT = {};

const useToolsPanelDropdownMenuProps = () => {
const isMobile = useViewportMatch( 'medium', '<' );
return ! isMobile
Expand Down Expand Up @@ -182,11 +184,66 @@ function EditableBlockBindingsPanelItems( {
export const BlockBindingsPanel = ( { name: blockName, metadata } ) => {
const registry = useRegistry();
const blockContext = useContext( BlockContext );
const { bindings } = metadata || {};
const { removeAllBlockBindings } = useBlockBindingsUtils();
const bindableAttributes = getBindableAttributes( blockName );
const dropdownMenuProps = useToolsPanelDropdownMenuProps();

// `useSelect` is used purposely here to ensure `getFieldsList`
// is updated whenever there are updates in block context.
// `source.getFieldsList` may also call a selector via `registry.select`.
const _fieldsList = {};
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason to keep that const outside of useSelect and modify it with the selector?

Copy link
Contributor

Choose a reason for hiding this comment

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

If I move the const inside the useSelect, it interprets it is a new object each time it goes through it, resulting in this warning:

The 'useSelect' hook returns different values when called with the same state and parameters. This can lead to unnecessary rerenders.

I must say I am not fully familiar with how this should work, but it is the only way I found to avoid that message.

Copy link
Member

Choose a reason for hiding this comment

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

Can you open PR without this local variable that hides the underlying issue? I would recommend asking for help from some folks who have experienced with the internals of useSelect like @jsnajdr and @Mamaduka.

Copy link
Member

Choose a reason for hiding this comment

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

Ideally you should memoize the fieldsList value but here it's almost impossible because the data come from so many different sources. Any getFieldsList callback can select from any store it wants.

The easiest solution is to:

  • return the fieldsList as the top-level object in the useSelect result. const fieldsList = useSelect( ... )
  • move the canUpdateBlockBindings select to its own useSelect call. The two selects don't have anything in common anyway, the select from completely different stores.

That should remove the warning.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've started exploring this here. As commented there, it seems just making these changes is not enough. We can keep the discussion in the other PR.

const { fieldsList, canUpdateBlockBindings } = useSelect(
( select ) => {
if ( ! bindableAttributes || bindableAttributes.length === 0 ) {
return EMPTY_OBJECT;
}
const { getBlockBindingsSources } = unlock( blocksPrivateApis );
const registeredSources = getBlockBindingsSources();
Object.entries( registeredSources ).forEach(
( [ sourceName, { getFieldsList, usesContext } ] ) => {
if ( getFieldsList ) {
// Populate context.
const context = {};
if ( usesContext?.length ) {
for ( const key of usesContext ) {
context[ key ] = blockContext[ key ];
}
}
const sourceList = getFieldsList( {
registry,
context,
} );
// Only add source if the list is not empty.
if ( sourceList ) {
_fieldsList[ sourceName ] = { ...sourceList };
}
}
}
);
return {
fieldsList:
Object.values( _fieldsList ).length > 0
? _fieldsList
: EMPTY_OBJECT,
canUpdateBlockBindings:
select( blockEditorStore ).getSettings()
.canUpdateBlockBindings,
};
},
[ blockContext, bindableAttributes, registry ]
);
// Return early if there are no bindable attributes.
if ( ! bindableAttributes || bindableAttributes.length === 0 ) {
return null;
}
// Remove empty sources from the list of fields.
Object.entries( fieldsList ).forEach( ( [ key, value ] ) => {
if ( ! Object.keys( value ).length ) {
delete fieldsList[ key ];
}
} );
gziolo marked this conversation as resolved.
Show resolved Hide resolved
// Filter bindings to only show bindable attributes and remove pattern overrides.
const { bindings } = metadata || {};
const filteredBindings = { ...bindings };
Object.keys( filteredBindings ).forEach( ( key ) => {
if (
Expand All @@ -197,48 +254,6 @@ export const BlockBindingsPanel = ( { name: blockName, metadata } ) => {
}
} );

const { canUpdateBlockBindings } = useSelect( ( select ) => {
return {
canUpdateBlockBindings:
select( blockEditorStore ).getSettings().canUpdateBlockBindings,
};
}, [] );

if ( ! bindableAttributes || bindableAttributes.length === 0 ) {
return null;
}

const fieldsList = {};
const { getBlockBindingsSources } = unlock( blocksPrivateApis );
const registeredSources = getBlockBindingsSources();
Object.entries( registeredSources ).forEach(
( [ sourceName, { getFieldsList, usesContext } ] ) => {
if ( getFieldsList ) {
// Populate context.
const context = {};
if ( usesContext?.length ) {
for ( const key of usesContext ) {
context[ key ] = blockContext[ key ];
}
}
const sourceList = getFieldsList( {
registry,
context,
} );
// Only add source if the list is not empty.
if ( sourceList ) {
fieldsList[ sourceName ] = { ...sourceList };
}
}
}
);
// Remove empty sources.
Object.entries( fieldsList ).forEach( ( [ key, value ] ) => {
if ( ! Object.keys( value ).length ) {
delete fieldsList[ key ];
}
} );

// Lock the UI when the user can't update bindings or there are no fields to connect to.
const readOnly =
! canUpdateBlockBindings || ! Object.keys( fieldsList ).length;
Expand Down
2 changes: 2 additions & 0 deletions packages/core-data/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import reducer from './reducer';
import * as selectors from './selectors';
import * as privateSelectors from './private-selectors';
import * as actions from './actions';
import * as privateActions from './private-actions';
import * as resolvers from './resolvers';
import createLocksActions from './locks/actions';
import {
Expand Down Expand Up @@ -79,6 +80,7 @@ const storeConfig = () => ( {
*/
export const store = createReduxStore( STORE_NAME, storeConfig() );
unlock( store ).registerPrivateSelectors( privateSelectors );
unlock( store ).registerPrivateActions( privateActions );
register( store ); // Register store after unlocking private selectors to allow resolvers to use them.

export { default as EntityProvider } from './entity-provider';
Expand Down
16 changes: 16 additions & 0 deletions packages/core-data/src/private-actions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
SantosGuillamot marked this conversation as resolved.
Show resolved Hide resolved
* Returns an action object used in signalling that the registered post meta
* fields for a post type have been received.
*
* @param {string} postType Post type slug.
* @param {Object} registeredPostMeta Registered post meta.
*
* @return {Object} Action object.
*/
export function receiveRegisteredPostMeta( postType, registeredPostMeta ) {
return {
type: 'RECEIVE_REGISTERED_POST_META',
postType,
registeredPostMeta,
};
}
12 changes: 12 additions & 0 deletions packages/core-data/src/private-selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,15 @@ export function getEntityRecordPermissions(
) {
return getEntityRecordsPermissions( state, kind, name, id )[ 0 ];
}

/**
* Returns the registered post meta fields for a given post type.
*
* @param state Data state.
* @param postType Post type.
*
* @return Registered post meta fields.
*/
export function getRegisteredPostMeta( state: State, postType: string ) {
return state.registeredPostMeta?.[ postType ] ?? {};
}
20 changes: 20 additions & 0 deletions packages/core-data/src/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,25 @@ export function defaultTemplates( state = {}, action ) {
return state;
}

/**
* Reducer returning an object of registered post meta.
*
* @param {Object} state Current state.
* @param {Object} action Dispatched action.
*
* @return {Object} Updated state.
*/
export function registeredPostMeta( state = {}, action ) {
switch ( action.type ) {
case 'RECEIVE_REGISTERED_POST_META':
return {
...state,
[ action.postType ]: action.registeredPostMeta,
};
}
return state;
}

export default combineReducers( {
terms,
users,
Expand All @@ -649,4 +668,5 @@ export default combineReducers( {
userPatternCategories,
navigationFallbackId,
defaultTemplates,
registeredPostMeta,
} );
26 changes: 26 additions & 0 deletions packages/core-data/src/resolvers.js
Original file line number Diff line number Diff line change
Expand Up @@ -984,3 +984,29 @@ export const getRevision =
dispatch.receiveRevisions( kind, name, recordKey, record, query );
}
};

/**
* Requests a specific post type options from the REST API.
*
* @param {string} postType Post type slug.
*/
export const getRegisteredPostMeta =
( postType ) =>
async ( { dispatch, resolveSelect } ) => {
try {
const {
rest_namespace: restNamespace = 'wp/v2',
rest_base: restBase,
} = ( await resolveSelect.getPostType( postType ) ) || {};
const options = await apiFetch( {
path: `${ restNamespace }/${ restBase }/?context=edit`,
method: 'OPTIONS',
} );
dispatch.receiveRegisteredPostMeta(
postType,
options?.schema?.properties?.meta?.properties
);
} catch {
dispatch.receiveRegisteredPostMeta( postType, false );
gziolo marked this conversation as resolved.
Show resolved Hide resolved
}
};
1 change: 1 addition & 0 deletions packages/core-data/src/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export interface State {
navigationFallbackId: EntityRecordKey;
userPatternCategories: Array< UserPatternCategory >;
defaultTemplates: Record< string, string >;
registeredPostMeta: Record< string, { postType: string } >;
}

type EntityRecordKey = string | number;
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e-tests/plugins/block-bindings.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function gutenberg_test_block_bindings_registration() {
'show_in_rest' => true,
'type' => 'string',
'single' => true,
'default' => 'Value of the text_custom_field',
'default' => 'Value of the text custom field',
)
);
register_meta(
Expand Down
46 changes: 31 additions & 15 deletions packages/editor/src/bindings/post-meta.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,43 @@ import { store as coreDataStore } from '@wordpress/core-data';
* Internal dependencies
*/
import { store as editorStore } from '../store';
import { unlock } from '../lock-unlock';

function getMetadata( registry, context ) {
gziolo marked this conversation as resolved.
Show resolved Hide resolved
let metaFields = {};
const { type } = registry.select( editorStore ).getCurrentPost();
gziolo marked this conversation as resolved.
Show resolved Hide resolved
const { getEditedEntityRecord } = registry.select( coreDataStore );
const { getRegisteredPostMeta } = unlock(
registry.select( coreDataStore )
);

if ( type === 'wp_template' ) {
gziolo marked this conversation as resolved.
Show resolved Hide resolved
const fields = getRegisteredPostMeta( context?.postType );
// Populate the `metaFields` object with the default values.
Object.entries( fields || {} ).forEach( ( [ key, props ] ) => {
metaFields[ key ] = props.default;
} );
} else {
metaFields = getEditedEntityRecord(
gziolo marked this conversation as resolved.
Show resolved Hide resolved
'postType',
context?.postType,
context?.postId
).meta;
}

return metaFields;
}

export default {
name: 'core/post-meta',
getValues( { registry, context, bindings } ) {
const meta = registry
.select( coreDataStore )
.getEditedEntityRecord(
'postType',
context?.postType,
context?.postId
)?.meta;
const metaFields = getMetadata( registry, context );

const newValues = {};
for ( const [ attributeName, source ] of Object.entries( bindings ) ) {
// Use the key if the value is not set.
newValues[ attributeName ] =
meta?.[ source.args.key ] ?? source.args.key;
metaFields?.[ source.args.key ] ?? source.args.key;
}
return newValues;
},
Expand Down Expand Up @@ -82,19 +103,14 @@ export default {
return true;
},
getFieldsList( { registry, context } ) {
const metaFields = registry
.select( coreDataStore )
.getEditedEntityRecord(
'postType',
context?.postType,
context?.postId
).meta;
const metaFields = getMetadata( registry, context );

if ( ! metaFields || ! Object.keys( metaFields ).length ) {
return null;
}

// Remove footnotes or private keys from the list of fields.
// TODO: Remove this once we retrieve the fields from 'types' endpoint in post or page editor.
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this be handled separately, and is there an open PR for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no PR yet. We managed to not touch any REST API related code. I don't know if it will be done for 6.7.

Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to move this filtering to getMetadata helper? Are the fields with _ prefix included in the schema since they are considered private?

return Object.fromEntries(
Object.entries( metaFields ).filter(
( [ key ] ) => key !== 'footnotes' && key.charAt( 0 ) !== '_'
Expand Down
Loading
Loading