-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Changes from all commits
70094be
862f4d1
9b9d0b4
ff8ebc5
233bf97
9e3e1e8
6ded461
8a69d40
6c32b80
e85aac7
304c28e
fac01f9
a4b3dc4
851ae0d
9c11f15
01afd0d
e49be26
3212804
116fc09
66ab07d
56cd505
25196ff
6cb6a38
0144e34
4cac9bd
af79c2b
782dd99
b4a11c0
42ef0c3
deb3e2d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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, | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
}, | ||
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense to move this filtering to |
||
return Object.fromEntries( | ||
Object.entries( metaFields ).filter( | ||
( [ key ] ) => key !== 'footnotes' && key.charAt( 0 ) !== '_' | ||
|
There was a problem hiding this comment.
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 ofuseSelect
and modify it with the selector?There was a problem hiding this comment.
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 theuseSelect
, it interprets it is a new object each time it goes through it, resulting in this warning: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.
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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. AnygetFieldsList
callback can select from any store it wants.The easiest solution is to:
fieldsList
as the top-level object in theuseSelect
result.const fieldsList = useSelect( ... )
canUpdateBlockBindings
select to its ownuseSelect
call. The two selects don't have anything in common anyway, the select from completely different stores.That should remove the warning.
There was a problem hiding this comment.
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.