-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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: lock editing of blocks by default #58787
Block Bindings: lock editing of blocks by default #58787
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: +75 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
|
||
export default { | ||
name: 'core/pattern-overrides', | ||
label: __( 'Pattern Overrides' ), |
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.
Shouldn't we use the _x()
like we do on the PHP side (example). If yes, that would also apply to the post-meta
source.
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.
They should be exactly the same as on the server. Good catch 👍
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.
Good catch 🙂 Changed in this commit.
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.
Would it not be simpler to add a default true
value to the reducer on this line instead of adding the !== false
checks in multiple places? I think it would also make it easier to update the default value in the future.
Other than that, I think it totally makes sense! 👍
Basically I was suggesting this: diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js
index c465e39021..b625c5989d 100644
--- a/packages/block-editor/src/store/reducer.js
+++ b/packages/block-editor/src/store/reducer.js
@@ -2051,7 +2051,7 @@ function blockBindingsSources( state = {}, action ) {
[ action.sourceName ]: {
label: action.sourceLabel,
useSource: action.useSource,
- lockAttributesEditing: action.lockAttributesEditing,
+ lockAttributesEditing: action.lockAttributesEditing ?? true,
},
};
} It would default to |
eef62cb
to
79a42c5
Compare
Totally makes sense! Thanks for pointing that out 🙂 I made the changes in this commit. Let me know if that's what you had in mind. |
/** | ||
* Internal dependencies | ||
*/ | ||
import { store as editorStore } from '../store'; | ||
|
||
export default { | ||
name: 'core/post-meta', | ||
label: __( 'Post Meta' ), | ||
label: _x( 'Post Meta' ), |
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.
We should mirror: https://github.com/WordPress/wordpress-develop/blob/acad00ecabcf44a6c4e552c4fd518ee708fd131f/src/wp-includes/block-bindings/post-meta.php#L52
_x( 'Post Meta', 'block bindings source' ),
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.
You're right! Done in this commit.
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 made the changes in this commit. Let me know if that's what you had in mind.
Yup, perfect 🙂
All in all LGTM! 👍
* trunk: (273 commits) Remove preffered style variations legacy support (#58930) Style theme variations: add property extraction and merge utils (#58803) Migrate `change-detection` to Playwright (#58767) Update Changelog for 17.6.6 Docs: Clarify the status of the wp-block-styles theme support, and its intent (#58915) Use `data_wp_context` helper in core blocks and remove `data-wp-interactive` object (#58943) Try double enter for details block. (#58903) Template revisions API: move from experimental to compat/6.4 (#58920) Editor: Remove inline toolbar preference (#58945) Clean up link control CSS. (#58934) Font Library: Show error message when no fonts found to install (#58914) Block Bindings: lock editing of blocks by default (#58787) Editor: Remove the 'all' rendering mode (#58935) Pagination Numbers: Add `data-wp-key` to pagination numbers if enhanced pagination is enabled (#58189) Close link preview if collapsed selection when creating link (#58896) Fix incorrect useAnchor positioning when switching from virtual to rich text elements (#58900) Upgrade Floating UI packages, fix nested iframe positioning bug (#58932) Site editor: fix start patterns store selector (#58813) Revert "Rich text: pad multiple spaces through en/em replacement (#56341)" (#58792) Documentation: Clarify the performance reference commit and how to pick it (#58927) ...
* Lock editing by default when bindings exist * Use default in post meta source * Set `lockEditing` to false in pattern overrides * Move default value to reducer * Use `_x` for sources translations * Add context to translations
I just cherry-picked this PR to the backports/beta1 branch to get it included in the next release: 72a22e5 |
* Lock editing by default when bindings exist * Use default in post meta source * Set `lockEditing` to false in pattern overrides * Move default value to reducer * Use `_x` for sources translations * Add context to translations
* Lock editing by default when bindings exist * Use default in post meta source * Set `lockEditing` to false in pattern overrides * Move default value to reducer * Use `_x` for sources translations * Add context to translations
What?
This pull request locks the editing of the rich text and the appropriate controls of button and image blocks by default when a binding exists.
Why?
Right now, if sources want to lock the editing when an attribute is bound to it, they have to specify that explicitly. However, the editor APIs are still private, so there is no way to do that for external sources. I believe locking it by default would cover most of the use cases.
Once we add support to editing the custom fields, we can change the default.
How?
I changed the locking conditionals to only unlock the editing when it is set to false and I added that to the pattern overrides source, because they don't need that.
Testing Instructions
Tests should cover the use cases.
For manual testing:
Ensure that the existing bindings keep working as expected:
In a page
Test paragraph
Test heading
Repeat the paragraph test but using a heading.
Test button
Test image
In a template
Go to a page template, for example, and repeat the process. In this case, the blocks can't show the value of the custom fields because it depends on each page. They should show a placeholder instead.
Test pattern syncing overrides