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

Open convert to links modal on select of a page item #48723

Merged
merged 2 commits into from
Mar 7, 2023
Merged

Open convert to links modal on select of a page item #48723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f596d2e
open convert to links modal on select of a page item
draganescu Mar 3, 2023
e9bcf96
unlock page list inside navigation block prompt to convert on select …
draganescu Mar 3, 2023
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
16 changes: 15 additions & 1 deletion docs/reference-guides/data/data-core-block-editor.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -860,6 +860,20 @@ _Returns_

- `string`: Client Id of moving block.

### hasDraggedInnerBlock

Returns true if one of the block's inner blocks is dragged.

_Parameters_

- _state_ `Object`: Editor state.
- _clientId_ `string`: Block client ID.
- _deep_ `boolean`: Perform a deep check.

_Returns_

- `boolean`: Whether the block has an inner block dragged

### hasInserterItems

Determines whether there are items to show in the inserter.
Expand Down Expand Up @@ -909,7 +923,7 @@ _Parameters_

_Returns_

- `boolean`: Whether the block as an inner block selected
- `boolean`: Whether the block has an inner block selected

### isAncestorBeingDragged

Expand Down
19 changes: 18 additions & 1 deletion packages/block-editor/src/store/selectors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1199,7 +1199,7 @@ export function isBlockSelected( state, clientId ) {
* @param {string} clientId Block client ID.
* @param {boolean} deep Perform a deep check.
*
* @return {boolean} Whether the block as an inner block selected
* @return {boolean} Whether the block has an inner block selected
*/
export function hasSelectedInnerBlock( state, clientId, deep = false ) {
return getBlockOrder( state, clientId ).some(
Expand All @@ -1210,6 +1210,23 @@ export function hasSelectedInnerBlock( state, clientId, deep = false ) {
);
}

/**
* Returns true if one of the block's inner blocks is dragged.
*
* @param {Object} state Editor state.
* @param {string} clientId Block client ID.
* @param {boolean} deep Perform a deep check.
*
* @return {boolean} Whether the block has an inner block dragged
*/
export function hasDraggedInnerBlock( state, clientId, deep = false ) {
Copy link
Member

Choose a reason for hiding this comment

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

We can also do this inline in the useSelect callback and avoid introducing a new selector for just one use case.

Here's a similar example:

gutenberg/packages/edit-site/src/components/save-button/index.js

Lines 23 to 25 in d19dbd6

isSaving: dirtyEntityRecords.some( ( record ) =>
isSavingEntityRecord( record.kind, record.name, record.key )
),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want this new selector, it's as useful as hasSelectedInnerBlock and this is a perfect opportunity to introduce it via using it.

Copy link
Member

Choose a reason for hiding this comment

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

Just sharing my personal preference of avoiding adding selector to packages scope, when I can :)

return getBlockOrder( state, clientId ).some(
( innerClientId ) =>
isBlockBeingDragged( state, innerClientId ) ||
( deep && hasDraggedInnerBlock( state, innerClientId, deep ) )
);
}

/**
* Returns true if the block corresponding to the specified client ID is
* currently selected but isn't the last of the selected blocks. Here "last"
Expand Down
1 change: 0 additions & 1 deletion packages/block-library/src/page-list-item/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
* External dependencies
*/
import classnames from 'classnames';

/**
* WordPress dependencies
*/
Expand Down
112 changes: 70 additions & 42 deletions packages/block-library/src/page-list/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ import {
Button,
} from '@wordpress/components';
import { __, sprintf } from '@wordpress/i18n';
import { useMemo, useState, useEffect } from '@wordpress/element';
import { useMemo, useState, useEffect, useCallback } from '@wordpress/element';
import { useEntityRecords } from '@wordpress/core-data';
import { useSelect } from '@wordpress/data';
import { useSelect, useDispatch } from '@wordpress/data';

/**
* Internal dependencies
Expand Down Expand Up @@ -113,36 +113,16 @@ function BlockContent( {
}
}

function ConvertToLinks( { onClick, disabled } ) {
draganescu marked this conversation as resolved.
Show resolved Hide resolved
const [ isOpen, setOpen ] = useState( false );
const openModal = () => setOpen( true );
const closeModal = () => setOpen( false );

return (
<>
<BlockControls group="other">
<ToolbarButton title={ __( 'Edit' ) } onClick={ openModal }>
{ __( 'Edit' ) }
</ToolbarButton>
</BlockControls>
{ isOpen && (
<ConvertToLinksModal
onClick={ onClick }
onClose={ closeModal }
disabled={ disabled }
/>
) }
</>
);
}

export default function PageListEdit( {
context,
clientId,
attributes,
setAttributes,
} ) {
const { parentPageID } = attributes;
const [ isOpen, setOpen ] = useState( false );
const openModal = useCallback( () => setOpen( true ), [] );
const closeModal = () => setOpen( false );

const { records: pages, hasResolved: hasResolvedPages } = useEntityRecords(
'postType',
Expand Down Expand Up @@ -263,34 +243,69 @@ export default function PageListEdit( {
parentPageID,
] );

const innerBlocksProps = useInnerBlocksProps( blockProps, {
allowedBlocks: [ 'core/page-list-item' ],
renderAppender: false,
__unstableDisableDropZone: true,
templateLock: 'all',
onInput: NOOP,
onChange: NOOP,
draganescu marked this conversation as resolved.
Show resolved Hide resolved
value: blockList,
} );

const { isNested } = useSelect(
const {
isNested,
hasSelectedChild,
parentBlock,
hasDraggedChild,
isChildOfNavigation,
} = useSelect(
( select ) => {
const { getBlockParentsByBlockName } = select( blockEditorStore );
const {
getBlockParentsByBlockName,
hasSelectedInnerBlock,
getBlockRootClientId,
hasDraggedInnerBlock,
} = select( blockEditorStore );
const blockParents = getBlockParentsByBlockName(
clientId,
'core/navigation-submenu',
true
);
const navigationBlockParents = getBlockParentsByBlockName(
clientId,
'core/navigation',
true
);
return {
isNested: blockParents.length > 0,
isChildOfNavigation: navigationBlockParents.length > 0,
hasSelectedChild: hasSelectedInnerBlock( clientId, true ),
hasDraggedChild: hasDraggedInnerBlock( clientId, true ),
parentBlock: getBlockRootClientId( clientId ),
};
},
[ clientId ]
);

const innerBlocksProps = useInnerBlocksProps( blockProps, {
Copy link
Contributor

Choose a reason for hiding this comment

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

@draganescu and I looked into passing the convertToNavigationLinks function down into the child block where the modal could be opened from there, but we couldn't find where the props ended up since they didn't get added to the edit props.

Copy link
Contributor

Choose a reason for hiding this comment

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

Another option could have been creating a context just for this scenario that the data needed in the child could have access to, but that also seems a little convoluted.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think it's worth looking into modifying the system for inner blocks?

allowedBlocks: [ 'core/page-list-item' ],
renderAppender: false,
__unstableDisableDropZone: true,
templateLock: isChildOfNavigation ? false : 'all',
onInput: NOOP,
onChange: NOOP,
value: blockList,
} );

const { selectBlock } = useDispatch( blockEditorStore );

useEffect( () => {
if ( hasSelectedChild || hasDraggedChild ) {
openModal();
selectBlock( parentBlock );
}
}, [
hasSelectedChild,
hasDraggedChild,
parentBlock,
selectBlock,
openModal,
] );

useEffect( () => {
setAttributes( { isNested } );
}, [ isNested ] );
}, [ isNested, setAttributes ] );

return (
<>
Expand Down Expand Up @@ -325,10 +340,23 @@ export default function PageListEdit( {
) }
</InspectorControls>
{ allowConvertToLinks && (
<ConvertToLinks
disabled={ ! hasResolvedPages }
onClick={ convertToNavigationLinks }
/>
<>
<BlockControls group="other">
<ToolbarButton
title={ __( 'Edit' ) }
onClick={ openModal }
>
{ __( 'Edit' ) }
</ToolbarButton>
</BlockControls>
{ isOpen && (
<ConvertToLinksModal
onClick={ convertToNavigationLinks }
onClose={ closeModal }
disabled={ ! hasResolvedPages }
/>
) }
</>
) }
<BlockContent
blockProps={ blockProps }
Expand Down