Skip to content

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

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

Merged
ajlende 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 1 commit
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
Prev Previous commit
unlock page list inside navigation block prompt to convert on select … 
…or drag
  • Loading branch information
@draganescu
draganescu committed Mar 3, 2023
commit e9bcf96923978f2c8e3226643fcf596ac94b33dc
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

@Mamaduka Mamaduka Mar 8, 2023

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

@draganescu draganescu Mar 8, 2023

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

@Mamaduka Mamaduka Mar 8, 2023

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
14 changes: 1 addition & 13 deletions 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 @@ -5,7 +5,6 @@ import classnames from 'classnames';
/**
* WordPress dependencies
*/
import { useEffect } from '@wordpress/element';
import { useBlockProps, useInnerBlocksProps } from '@wordpress/block-editor';
import { useSelect } from '@wordpress/data';
import { store as coreStore } from '@wordpress/core-data';
Expand Down Expand Up @@ -35,12 +34,7 @@ function useFrontPageId() {
}, [] );
}

export default function PageListItemEdit( {
context,
attributes,
isSelected,
setAttributes,
} ) {
export default function PageListItemEdit( { context, attributes } ) {
const { id, label, link, hasChildren } = attributes;
const isNavigationChild = 'showSubmenuIcon' in context;
const frontPageId = useFrontPageId();
Expand All @@ -55,12 +49,6 @@ export default function PageListItemEdit( {

const innerBlocksProps = useInnerBlocksProps( blockProps );

useEffect( () => {
if ( isSelected ) {
setAttributes( { isSelected: true } );
}
}, [ isSelected ] );

return (
<li
key={ id }
Expand Down
71 changes: 51 additions & 20 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 @@ -121,7 +121,7 @@ export default function PageListEdit( {
} ) {
const { parentPageID } = attributes;
const [ isOpen, setOpen ] = useState( false );
const openModal = () => setOpen( true );
const openModal = useCallback( () => setOpen( true ), [] );
const closeModal = () => setOpen( false );

const { records: pages, hasResolved: hasResolvedPages } = useEntityRecords(
Expand Down Expand Up @@ -243,38 +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: () => {
if ( hasResolvedPages ) {
openModal();
}
},
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

@ajlende ajlende Mar 7, 2023

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

@ajlende ajlende Mar 7, 2023

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

@scruffian scruffian Mar 7, 2023

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