Skip to content

Commit

Permalink
Block Bindings: Disable editing of bound block attributes in editor UI (
Browse files Browse the repository at this point in the history
#58085)

* Add actions and selectors to register new sources

* Add hook to read the bindings attribute in Edit

* Add context to all the blocks with bindings

* Lock rich text when `isContentBound` is true

* Adapt paragraph and heading blocks UI

* Adapt button block UI

* Adapt image block UI

* Register post meta source

* Don't use placeholder if attribute is `src` or `href`

* Always share placeholder in case meta is empty

* Remove `keyToLabel` and use just label

* Remove source component until it is needed

* Use translations in the source label

* Move `select` inside `useSource`

* Read `lockEditorUI` prop and add it for patterns

* Move logic to lock editing directly to RichText

* Improve `useSelect` destructuring

* Load all image controls if attributes are bound

* Remove unnecessary condition

* Move `lockAttributesEditing` to source definition

* Move `useSelect` into existing hook

* Fix `RichText` not being selected on click

* Lock button and image controls only when selected

* Remove unnecesarry optional chaining

* Move `shouldDisableEditing` logic inside callback

* Fix formatting issue
  • Loading branch information
SantosGuillamot authored Jan 24, 2024
1 parent ea32957 commit fd46752
Show file tree
Hide file tree
Showing 15 changed files with 475 additions and 97 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ _::-webkit-full-page-media, _:future, :root .has-multi-selection .block-editor-b
.block-editor-block-list__block.is-highlighted,
.block-editor-block-list__block.is-highlighted ~ .is-multi-selected,
&.is-navigate-mode .block-editor-block-list__block.is-selected,
.block-editor-block-list__block:not([contenteditable]):focus {
.block-editor-block-list__block:not([contenteditable="true"]):focus {
outline: none;

// We're using a pseudo element to overflow placeholder borders
Expand Down
58 changes: 48 additions & 10 deletions packages/block-editor/src/components/rich-text/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
removeFormat,
} from '@wordpress/rich-text';
import { Popover } from '@wordpress/components';
import { getBlockType } from '@wordpress/blocks';

/**
* Internal dependencies
Expand All @@ -44,6 +45,7 @@ import FormatEdit from './format-edit';
import { getAllowedFormats } from './utils';
import { Content } from './content';
import { withDeprecations } from './with-deprecations';
import { unlock } from '../../lock-unlock';

export const keyboardShortcutContext = createContext();
export const inputEventContext = createContext();
Expand Down Expand Up @@ -113,18 +115,24 @@ export function RichTextWrapper(
props = removeNativeProps( props );

const anchorRef = useRef();
const { clientId, isSelected: isBlockSelected } = useBlockEditContext();
const {
clientId,
isSelected: isBlockSelected,
name: blockName,
} = useBlockEditContext();
const selector = ( select ) => {
// Avoid subscribing to the block editor store if the block is not
// selected.
if ( ! isBlockSelected ) {
return { isSelected: false };
}

const { getSelectionStart, getSelectionEnd } =
const { getSelectionStart, getSelectionEnd, getBlockAttributes } =
select( blockEditorStore );
const selectionStart = getSelectionStart();
const selectionEnd = getSelectionEnd();
const blockBindings =
getBlockAttributes( clientId )?.metadata?.bindings;

This comment has been minimized.

Copy link
@gziolo

gziolo Jan 25, 2024

Member

We might simplify the check for bindings to something close to it getBlockAttributes( clientId )?.metadata?.bindings?.[identifier].

One remark is, if there is no idefentifier then we skip the binding - that's an edge case because we expect that supported core blocks have it set.


let isSelected;

Expand All @@ -137,18 +145,44 @@ export function RichTextWrapper(
isSelected = selectionStart.clientId === clientId;
}

// Disable Rich Text editing if block bindings specify that.
let shouldDisableEditing = false;
if ( blockBindings ) {
const blockTypeAttributes = getBlockType( blockName ).attributes;
const { getBlockBindingsSource } = unlock(
select( blockEditorStore )
);
for ( const [ attribute, args ] of Object.entries(
blockBindings
) ) {
// If any of the attributes with source "rich-text" is part of the bindings,
// has a source with `lockAttributesEditing`, disable it.
if (
blockTypeAttributes?.[ attribute ]?.source ===
'rich-text' &&
getBlockBindingsSource( args.source.name )
?.lockAttributesEditing
) {
shouldDisableEditing = true;
break;
}
}
}

return {
selectionStart: isSelected ? selectionStart.offset : undefined,
selectionEnd: isSelected ? selectionEnd.offset : undefined,
isSelected,
shouldDisableEditing,
};
};
const { selectionStart, selectionEnd, isSelected } = useSelect( selector, [
clientId,
identifier,
originalIsSelected,
isBlockSelected,
] );
const { selectionStart, selectionEnd, isSelected, shouldDisableEditing } =
useSelect( selector, [
clientId,
identifier,
originalIsSelected,
isBlockSelected,

This comment has been minimized.

Copy link
@gziolo

gziolo Jan 25, 2024

Member

Include identifier prop.

] );
const { getSelectionStart, getSelectionEnd, getBlockRootClientId } =
useSelect( blockEditorStore );
const { selectionChange } = useDispatch( blockEditorStore );
Expand Down Expand Up @@ -376,7 +410,7 @@ export function RichTextWrapper(
useFirefoxCompat(),
anchorRef,
] ) }
contentEditable={ true }
contentEditable={ ! shouldDisableEditing }
suppressContentEditableWarning={ true }
className={ classnames(
'block-editor-rich-text__editable',
Expand All @@ -389,7 +423,11 @@ export function RichTextWrapper(
// select blocks when Shift Clicking into an element with
// tabIndex because Safari will focus the element. However,
// Safari will correctly ignore nested contentEditable elements.
tabIndex={ props.tabIndex === 0 ? null : props.tabIndex }
tabIndex={
props.tabIndex === 0 && ! shouldDisableEditing
? null
: props.tabIndex
}

This comment has been minimized.

Copy link
@gziolo

gziolo Jan 25, 2024

Member

Check whether aria-disabled is necessary for accessibility purposes when in readonly mode.

data-wp-block-attribute-key={ identifier }
/>
</>
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/hooks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import contentLockUI from './content-lock-ui';
import './metadata';
import blockHooks from './block-hooks';
import blockRenaming from './block-renaming';
import './use-bindings-attributes';

createBlockEditFilter(
[
Expand Down
148 changes: 148 additions & 0 deletions packages/block-editor/src/hooks/use-bindings-attributes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
/**
* WordPress dependencies
*/
import { getBlockType } from '@wordpress/blocks';
import { createHigherOrderComponent } from '@wordpress/compose';
import { useRegistry, useSelect } from '@wordpress/data';
import { addFilter } from '@wordpress/hooks';
/**
* Internal dependencies
*/
import { store as blockEditorStore } from '../store';
import { useBlockEditContext } from '../components/block-edit/context';
import { unlock } from '../lock-unlock';

/** @typedef {import('@wordpress/compose').WPHigherOrderComponent} WPHigherOrderComponent */
/** @typedef {import('@wordpress/blocks').WPBlockSettings} WPBlockSettings */

/**
* Given a binding of block attributes, returns a higher order component that
* overrides its `attributes` and `setAttributes` props to sync any changes needed.
*
* @return {WPHigherOrderComponent} Higher-order component.
*/

const BLOCK_BINDINGS_ALLOWED_BLOCKS = {
'core/paragraph': [ 'content' ],
'core/heading': [ 'content' ],
'core/image': [ 'url', 'title', 'alt' ],
'core/button': [ 'url', 'text' ],
};

const createEditFunctionWithBindingsAttribute = () =>
createHigherOrderComponent(
( BlockEdit ) => ( props ) => {
const { clientId, name: blockName } = useBlockEditContext();
const { getBlockBindingsSource } = unlock(
useSelect( blockEditorStore )
);
const { getBlockAttributes, updateBlockAttributes } =
useSelect( blockEditorStore );

const updatedAttributes = getBlockAttributes( clientId );
if ( updatedAttributes?.metadata?.bindings ) {
Object.entries( updatedAttributes.metadata.bindings ).forEach(
( [ attributeName, settings ] ) => {
const source = getBlockBindingsSource(
settings.source.name
);

if ( source ) {
// Second argument (`updateMetaValue`) will be used to update the value in the future.
const {
placeholder,
useValue: [ metaValue = null ] = [],
} = source.useSource(
props,
settings.source.attributes
);

if ( placeholder && ! metaValue ) {
// If the attribute is `src` or `href`, a placeholder can't be used because it is not a valid url.
// Adding this workaround until attributes and metadata fields types are improved and include `url`.
const htmlAttribute =
getBlockType( blockName ).attributes[
attributeName
].attribute;
if (
htmlAttribute === 'src' ||
htmlAttribute === 'href'
) {
updatedAttributes[ attributeName ] = null;
} else {
updatedAttributes[ attributeName ] =
placeholder;
}
}

if ( metaValue ) {
updatedAttributes[ attributeName ] = metaValue;
}
}
}
);
}

const registry = useRegistry();

return (
<>
<BlockEdit
key="edit"
attributes={ updatedAttributes }
setAttributes={ ( newAttributes, blockId ) =>
registry.batch( () =>
updateBlockAttributes( blockId, newAttributes )
)
}
{ ...props }
/>
</>
);
},
'useBoundAttributes'
);

/**
* Filters a registered block's settings to enhance a block's `edit` component
* to upgrade bound attributes.
*
* @param {WPBlockSettings} settings Registered block settings.
*
* @return {WPBlockSettings} Filtered block settings.
*/
function shimAttributeSource( settings ) {
if ( ! ( settings.name in BLOCK_BINDINGS_ALLOWED_BLOCKS ) ) {
return settings;
}
settings.edit = createEditFunctionWithBindingsAttribute()( settings.edit );

return settings;
}

addFilter(
'blocks.registerBlockType',
'core/editor/custom-sources-backwards-compatibility/shim-attribute-source',
shimAttributeSource
);

// Add the context to all blocks.
addFilter(
'blocks.registerBlockType',
'core/block-bindings-ui',
( settings, name ) => {
if ( ! ( name in BLOCK_BINDINGS_ALLOWED_BLOCKS ) ) {
return settings;
}
const contextItems = [ 'postId', 'postType', 'queryId' ];

This comment has been minimized.

Copy link
@gziolo

gziolo Jan 25, 2024

Member

Do we need queryId here?

This comment has been minimized.

Copy link
@gziolo

gziolo Jan 25, 2024

Member

It's used with post related blocks to prevent editing. We can skip it for now.

const usesContextArray = settings.usesContext;
const oldUsesContextArray = new Set( usesContextArray );
contextItems.forEach( ( item ) => {
if ( ! oldUsesContextArray.has( item ) ) {
usesContextArray.push( item );
}
} );
settings.usesContext = usesContextArray;
return settings;
}
);
10 changes: 10 additions & 0 deletions packages/block-editor/src/store/private-actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -360,3 +360,13 @@ export function stopEditingAsBlocks( clientId ) {
dispatch.__unstableSetTemporarilyEditingAsBlocks();
};
}

export function registerBlockBindingsSource( source ) {
return {
type: 'REGISTER_BLOCK_BINDINGS_SOURCE',
sourceName: source.name,
sourceLabel: source.label,
useSource: source.useSource,
lockAttributesEditing: source.lockAttributesEditing,
};
}
8 changes: 8 additions & 0 deletions packages/block-editor/src/store/private-selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -341,3 +341,11 @@ export const getAllPatterns = createRegistrySelector( ( select ) =>
export function getLastFocus( state ) {
return state.lastFocus;
}

export function getAllBlockBindingsSources( state ) {
return state.blockBindingsSources;
}

export function getBlockBindingsSource( state, sourceName ) {
return state.blockBindingsSources[ sourceName ];
}
15 changes: 15 additions & 0 deletions packages/block-editor/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -2023,6 +2023,20 @@ export function lastFocus( state = false, action ) {
return state;
}

function blockBindingsSources( state = {}, action ) {
if ( action.type === 'REGISTER_BLOCK_BINDINGS_SOURCE' ) {
return {
...state,
[ action.sourceName ]: {
label: action.sourceLabel,
useSource: action.useSource,
lockAttributesEditing: action.lockAttributesEditing,
},
};
}
return state;
}

function blockPatterns( state = [], action ) {
switch ( action.type ) {
case 'RECEIVE_BLOCK_PATTERNS':
Expand Down Expand Up @@ -2062,6 +2076,7 @@ const combinedReducers = combineReducers( {
blockRemovalRules,
openedBlockSettingsMenu,
registeredInserterMediaCategories,
blockBindingsSources,
blockPatterns,
} );

Expand Down
Loading

1 comment on commit fd46752

@github-actions
Copy link

Choose a reason for hiding this comment

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

Flaky tests detected in fd46752.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7644966480
📝 Reported issues:

Please sign in to comment.