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

Page Content Focus: Ignore page content within a Query Loop block #52351

Merged
merged 2 commits into from
Jul 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 26 additions & 9 deletions packages/block-editor/src/components/block-edit/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,25 @@ import BlockContext from '../block-context';
*/
const DEFAULT_BLOCK_CONTEXT = {};

export const Edit = ( props ) => {
const Edit = ( props ) => {
const { name } = props;
const blockType = getBlockType( name );

if ( ! blockType ) {
return null;
}

// `edit` and `save` are functions or components describing the markup
// with which a block is displayed. If `blockType` is valid, assign
// them preferentially as the render value for the block.
const Component = blockType.edit || blockType.save;

return <Component { ...props } />;
};

const EditWithFilters = withFilters( 'editor.BlockEdit' )( Edit );
Copy link
Member Author

@noisysocks noisysocks Jul 6, 2023

Choose a reason for hiding this comment

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

@youknowriad Just double checking that it wasn't intentional that we don't pass context and className to components that use editor.BlockEdit?

I would expect that components using the editor.BlockEdit filter receive the exact same props as a block's edit component, which is what this change results in.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's hard to tell for me. but yeah you're reasoning is correct I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool yeah me neither thanks for the gut check!

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this might have caused a breaking change for folks relying on the editor.BlockEdit filter to pass context to BlockEdit; see report in #53869. I'm not sure if that was intended behaviour to begin with 😅 but applying the filters to the internal Edit component instead of the outer wrapper results in it no longer being possible.


const EditWithGeneratedProps = ( props ) => {
const { attributes = {}, name } = props;
const blockType = getBlockType( name );
const blockContext = useContext( BlockContext );
Expand All @@ -49,13 +67,8 @@ export const Edit = ( props ) => {
return null;
}

// `edit` and `save` are functions or components describing the markup
// with which a block is displayed. If `blockType` is valid, assign
// them preferentially as the render value for the block.
const Component = blockType.edit || blockType.save;

if ( blockType.apiVersion > 1 ) {
return <Component { ...props } context={ context } />;
return <EditWithFilters { ...props } context={ context } />;
}

// Generate a class name for the block's editable form.
Expand All @@ -69,8 +82,12 @@ export const Edit = ( props ) => {
);

return (
<Component { ...props } context={ context } className={ className } />
<EditWithFilters
{ ...props }
context={ context }
className={ className }
/>
);
};

export default withFilters( 'editor.BlockEdit' )( Edit );
export default EditWithGeneratedProps;
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
/**
* Internal dependencies
*/
import { Edit } from '../edit';
import Edit from '../edit';
import { BlockContextProvider } from '../../block-context';

const noop = () => {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ export default function useListViewClientIds( { blocks, rootClientId } ) {
const {
getDraggedBlockClientIds,
getSelectedBlockClientIds,
getListViewClientIdsTree,
getEnabledClientIdsTree,
} = unlock( select( blockEditorStore ) );

return {
selectedClientIds: getSelectedBlockClientIds(),
draggedClientIds: getDraggedBlockClientIds(),
clientIdsTree:
blocks ?? getListViewClientIdsTree( rootClientId ),
blocks ?? getEnabledClientIdsTree( rootClientId ),
};
},
[ blocks, rootClientId ]
Expand Down
9 changes: 3 additions & 6 deletions packages/block-editor/src/store/private-selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,21 +136,18 @@ export const isBlockSubtreeDisabled = createSelector(
*
* @return {Object[]} Tree of block objects with only clientID and innerBlocks set.
*/
export const getListViewClientIdsTree = createSelector(
export const getEnabledClientIdsTree = createSelector(
( state, rootClientId = '' ) => {
return getBlockOrder( state, rootClientId ).flatMap( ( clientId ) => {
if ( getBlockEditingMode( state, clientId ) !== 'disabled' ) {
return [
{
clientId,
innerBlocks: getListViewClientIdsTree(
state,
clientId
),
innerBlocks: getEnabledClientIdsTree( state, clientId ),
},
];
}
return getListViewClientIdsTree( state, clientId );
return getEnabledClientIdsTree( state, clientId );
} );
},
( state ) => [
Expand Down
10 changes: 5 additions & 5 deletions packages/block-editor/src/store/test/private-selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
getLastInsertedBlocksClientIds,
getBlockEditingMode,
isBlockSubtreeDisabled,
getListViewClientIdsTree,
getEnabledClientIdsTree,
getEnabledBlockParents,
} from '../private-selectors';

Expand Down Expand Up @@ -391,7 +391,7 @@ describe( 'private selectors', () => {
} );
} );

describe( 'getListViewClientIdsTree', () => {
describe( 'getEnabledClientIdsTree', () => {
const baseState = {
settings: {},
blocks: {
Expand Down Expand Up @@ -462,7 +462,7 @@ describe( 'private selectors', () => {
...baseState,
blockEditingModes: new Map( [] ),
};
expect( getListViewClientIdsTree( state ) ).toEqual( [
expect( getEnabledClientIdsTree( state ) ).toEqual( [
{
clientId: '6cf70164-9097-4460-bcbf-200560546988',
innerBlocks: [],
Expand Down Expand Up @@ -500,7 +500,7 @@ describe( 'private selectors', () => {
blockEditingModes: new Map( [] ),
};
expect(
getListViewClientIdsTree(
getEnabledClientIdsTree(
state,
'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337'
)
Expand Down Expand Up @@ -534,7 +534,7 @@ describe( 'private selectors', () => {
[ '9b9c5c3f-2e46-4f02-9e14-9fe9515b958f', 'contentOnly' ],
] ),
};
expect( getListViewClientIdsTree( state ) ).toEqual( [
expect( getEnabledClientIdsTree( state ) ).toEqual( [
{
clientId: 'b26fc763-417d-4f01-b81c-2ec61e14a972',
innerBlocks: [],
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,22 @@ import { useEffect } from '@wordpress/element';
* Internal dependencies
*/
import { unlock } from '../../lock-unlock';
import { PAGE_CONTENT_BLOCK_TYPES } from './constants';

const { useBlockEditingMode } = unlock( blockEditorPrivateApis );

const PAGE_CONTENT_BLOCK_TYPES = [
'core/post-title',
'core/post-featured-image',
'core/post-content',
];

/**
* Component that when rendered, makes it so that the site editor allows only
* page content to be edited.
*/
export default function DisableNonPageContentBlocks() {
useDisableNonPageContentBlocks();
return null;
}

/**
Expand All @@ -43,8 +49,11 @@ export function useDisableNonPageContentBlocks() {

const withDisableNonPageContentBlocks = createHigherOrderComponent(
( BlockEdit ) => ( props ) => {
const isContent = PAGE_CONTENT_BLOCK_TYPES.includes( props.name );
const mode = isContent ? 'contentOnly' : undefined;
const isDescendentOfQueryLoop = !! props.context.queryId;
const isPageContent =
PAGE_CONTENT_BLOCK_TYPES.includes( props.name ) &&
! isDescendentOfQueryLoop;
const mode = isPageContent ? 'contentOnly' : undefined;
useBlockEditingMode( mode );
return <BlockEdit { ...props } />;
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,24 @@ import {
store as blockEditorStore,
privateApis as blockEditorPrivateApis,
} from '@wordpress/block-editor';
import { useMemo } from '@wordpress/element';

/**
* Internal dependencies
*/
import { PAGE_CONTENT_BLOCK_TYPES } from '../../page-content-focus-manager/constants';
import { unlock } from '../../../lock-unlock';

const { BlockQuickNavigation } = unlock( blockEditorPrivateApis );

export default function PageContent() {
const clientIds = useSelect(
const clientIdsTree = useSelect(
( select ) =>
select( blockEditorStore ).__experimentalGetGlobalBlocksByName(
PAGE_CONTENT_BLOCK_TYPES
),
unlock( select( blockEditorStore ) ).getEnabledClientIdsTree(),
[]
);
const clientIds = useMemo(
() => clientIdsTree.map( ( { clientId } ) => clientId ),
[ clientIdsTree ]
);
return <BlockQuickNavigation clientIds={ clientIds } />;
}