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.

Sign up for GitHub

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: Disable editing of bound block attributes in editor UI #58085

Merged
merged 27 commits into from
Jan 24, 2024
Merged

Block Bindings: Disable editing of bound block attributes in editor UI #58085

Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
2f071ad
Add actions and selectors to register new sources
SantosGuillamot Jan 22, 2024
57ab9b2
Add hook to read the bindings attribute in Edit
SantosGuillamot Jan 22, 2024
445405f
Add context to all the blocks with bindings
SantosGuillamot Jan 22, 2024
af7f870
Lock rich text when `isContentBound` is true
SantosGuillamot Jan 22, 2024
c3567a4
Adapt paragraph and heading blocks UI
SantosGuillamot Jan 22, 2024
35c8c64
Adapt button block UI
SantosGuillamot Jan 22, 2024
989f456
Adapt image block UI
SantosGuillamot Jan 22, 2024
a4dc34a
Register post meta source
SantosGuillamot Jan 22, 2024
3515538
Don't use placeholder if attribute is `src` or `href`
SantosGuillamot Jan 23, 2024
41709fa
Always share placeholder in case meta is empty
SantosGuillamot Jan 23, 2024
1567f17
Remove `keyToLabel` and use just label
SantosGuillamot Jan 23, 2024
1984749
Remove source component until it is needed
SantosGuillamot Jan 23, 2024
9644946
Use translations in the source label
SantosGuillamot Jan 23, 2024
478f861
Move `select` inside `useSource`
SantosGuillamot Jan 23, 2024
2acf6bd
Read `lockEditorUI` prop and add it for patterns
SantosGuillamot Jan 23, 2024
a8a6da3
Move logic to lock editing directly to RichText
SantosGuillamot Jan 23, 2024
30b635e
Improve `useSelect` destructuring
SantosGuillamot Jan 23, 2024
a6f5fde
Load all image controls if attributes are bound
SantosGuillamot Jan 23, 2024
7d0cb9a
Remove unnecessary condition
SantosGuillamot Jan 23, 2024
e6a5a4d
Move `lockAttributesEditing` to source definition
SantosGuillamot Jan 23, 2024
7c1ca5a
Move `useSelect` into existing hook
SantosGuillamot Jan 23, 2024
4246260
Fix `RichText` not being selected on click
SantosGuillamot Jan 23, 2024
5a0cee8
Lock button and image controls only when selected
SantosGuillamot Jan 23, 2024
54c313d
Remove unnecesarry optional chaining
SantosGuillamot Jan 24, 2024
aaf8652
Move `shouldDisableEditing` logic inside callback
SantosGuillamot Jan 24, 2024
1f34e08
Merge branch 'trunk' into add/block-bindings-support-in-the-editor
SantosGuillamot Jan 24, 2024
895e6dd
Fix formatting issue
SantosGuillamot Jan 24, 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
Prev Previous commit
Next Next commit
Remove keyToLabel and use just label
  • Loading branch information
@SantosGuillamot
SantosGuillamot committed Jan 23, 2024
commit 1567f177389a7292ec78584cc182381f44543f05
12 changes: 2 additions & 10 deletions packages/editor/src/bindings/post-meta.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,6 @@ import { store as editorStore } from '../store';

const { getCurrentPostType } = select( editorStore );

// Prettify the name until the label is available in the REST API endpoint.
const keyToLabel = ( key ) => {
return key
.split( '_' )
.map( ( word ) => word.charAt( 0 ).toUpperCase() + word.slice( 1 ) )
.join( ' ' );
};

export default {
name: 'post_meta',
label: 'Post Meta',
Expand All @@ -36,14 +28,14 @@ export default {
);

if ( postType === 'wp_template' ) {
return { placeholder: keyToLabel( metaKey ) };
return { placeholder: metaKey };
}
const metaValue = meta[ metaKey ];
const updateMetaValue = ( newValue ) => {
setMeta( { ...meta, [ metaKey ]: newValue } );
};
return {
placeholder: keyToLabel( metaKey ),
placeholder: metaKey,
useValue: [ metaValue, updateMetaValue ],
};
},
Copy link
Contributor

@michalczaplinski michalczaplinski Jan 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Do we have to return an object with the useValue key? I see that the useValue
key is immediately destructured in

gutenberg/packages/block-editor/src/hooks/use-bindings-attributes.js

Lines 63 to 64 in a4dc34a

useValue: [ metaValue = null ] = [],
} = source.useSource(

and not used anywhere else.

Can we not just return metaValue and updateMetaValue directly?

I see that updateMetaValue is not currently used so we could even skip returning it for now, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Additionally, I'm not sure if it's acceptable that useSource() returns a
different object depending on the code path. Right now, we return the
{placeholder} if it's a template and {useValue} otherwise.

Ideally, the return type should always be the same.

Copy link
Contributor

@michalczaplinski michalczaplinski Jan 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Additionally, I'm not sure if it's acceptable that useSource() returns a
different object depending on the code path. Right now, we return the
{ placeholder } if it's a template and { useValue } otherwise.

Ideally, the return type should always be the same.

Oh, nevermind. I see you've already updated 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.

Can we not just return metaValue and updateMetaValue directly?

I used that trying to keep the "hooks syntax", but I'm fine returning just metaValue and updataMetaValue if that works and it is preferred.

Copy link
Contributor

Choose a reason for hiding this comment

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

If the main motivation was to similarity to React hooks syntax then I would argue that it's similar but not the same so might be even more confusing. useValue here is not a function that returns an array but simply an array.

returning just metaValue and updataMetaValue if that works

That would be my preference but I'm happy to hear other opinions 🙂

Copy link
Contributor

Choose a reason for hiding this comment

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

I would consider this option, however, detecting and propagating changes without using a React hook requires a different strategy, likely through subscribing to property value changes.

Expand Down