-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Page List: add ability to convert to navigation links #30390
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
Changes from all commits
4dbea0c
1bb3a15
1fdf57f
c6a0268
5db1eb7
e35f132
4337b7e
f70d114
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import { Button, Modal } from '@wordpress/components'; | ||
| import { __ } from '@wordpress/i18n'; | ||
| import { useDispatch, useSelect } from '@wordpress/data'; | ||
| import { store as coreDataStore } from '@wordpress/core-data'; | ||
| import { createBlock as create } from '@wordpress/blocks'; | ||
| import { store as blockEditorStore } from '@wordpress/block-editor'; | ||
|
|
||
| const PAGE_FIELDS = [ 'id', 'title', 'link', 'type', 'parent' ]; | ||
| const MAX_PAGE_COUNT = 100; | ||
|
|
||
| export const convertSelectedBlockToNavigationLinks = ( { | ||
| pages, | ||
| clientId, | ||
| replaceBlock, | ||
| createBlock, | ||
| } ) => () => { | ||
| if ( ! pages ) { | ||
| return; | ||
| } | ||
|
|
||
| const linkMap = {}; | ||
| const navigationLinks = []; | ||
| pages.forEach( ( { id, title, link: url, type, parent } ) => { | ||
| // See if a placeholder exists. This is created if children appear before parents in list | ||
| const innerBlocks = linkMap[ id ]?.innerBlocks ?? []; | ||
| linkMap[ id ] = createBlock( | ||
| 'core/navigation-link', | ||
| { | ||
| id, | ||
| label: title.rendered, | ||
| url, | ||
| type, | ||
| kind: 'post-type', | ||
| }, | ||
| innerBlocks | ||
| ); | ||
|
|
||
| if ( ! parent ) { | ||
| navigationLinks.push( linkMap[ id ] ); | ||
| } else { | ||
| if ( ! linkMap[ parent ] ) { | ||
| // Use a placeholder if the child appears before parent in list | ||
| linkMap[ parent ] = { innerBlocks: [] }; | ||
| } | ||
| const parentLinkInnerBlocks = linkMap[ parent ].innerBlocks; | ||
| parentLinkInnerBlocks.push( linkMap[ id ] ); | ||
| } | ||
| } ); | ||
|
|
||
| replaceBlock( clientId, navigationLinks ); | ||
| }; | ||
|
|
||
| export default function ConvertToLinksModal( { onClose, clientId } ) { | ||
| const { pages, pagesFinished } = useSelect( | ||
| ( select ) => { | ||
| const { getEntityRecords, hasFinishedResolution } = select( | ||
| coreDataStore | ||
| ); | ||
| const query = [ | ||
| 'postType', | ||
| 'page', | ||
| { | ||
| per_page: MAX_PAGE_COUNT, | ||
| _fields: PAGE_FIELDS, | ||
| // TODO: When https://core.trac.wordpress.org/ticket/39037 REST API support for multiple orderby | ||
| // values is resolved, update 'orderby' to [ 'menu_order', 'post_title' ] to provide a consistent | ||
| // sort. | ||
| orderby: 'menu_order', | ||
| order: 'asc', | ||
| }, | ||
| ]; | ||
| return { | ||
| pages: getEntityRecords( ...query ), | ||
| pagesFinished: hasFinishedResolution( | ||
| 'getEntityRecords', | ||
| query | ||
| ), | ||
| }; | ||
| }, | ||
| [ clientId ] | ||
| ); | ||
| const { replaceBlock } = useDispatch( blockEditorStore ); | ||
|
|
||
| return ( | ||
| <Modal | ||
| closeLabel={ __( 'Close' ) } | ||
| onRequestClose={ onClose } | ||
| title={ __( 'Convert to links' ) } | ||
| className={ 'wp-block-page-list-modal' } | ||
| aria={ { describedby: 'wp-block-page-list-modal__description' } } | ||
| > | ||
| <p id={ 'wp-block-page-list-modal__description' }> | ||
| { __( | ||
| 'To edit this navigation menu, convert it to single page links. This allows you to add, re-order, remove items, or edit their labels.' | ||
| ) } | ||
| </p> | ||
| <p> | ||
| { __( | ||
| "Note: if you add new pages to your site, you'll need to add them to your navigation menu." | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This paragraph won't be read out as part of the description, is that ok?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, it's read out if you're manually arrowing through the whole content of the modal, but not when you first land on it. So, if you press "Edit", the modal opens and "Convert to links" is read out, followed by the first paragraph of the content. If user just presses tab after that, they'll skip through the buttons and never reach the rest of the text. I guess that might be ok though, as the note just relays complementary info and isn't essential to understand what the "convert" action does. |
||
| ) } | ||
| </p> | ||
| <div className="wp-block-page-list-modal-buttons"> | ||
| <Button isTertiary onClick={ onClose }> | ||
| { __( 'Cancel' ) } | ||
| </Button> | ||
| <Button | ||
| isPrimary | ||
| disabled={ ! pagesFinished } | ||
gwwar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| onClick={ convertSelectedBlockToNavigationLinks( { | ||
| pages, | ||
| replaceBlock, | ||
| clientId, | ||
| createBlock: create, | ||
| } ) } | ||
| > | ||
| { __( 'Convert' ) } | ||
| </Button> | ||
| </div> | ||
| </Modal> | ||
| ); | ||
| } | ||

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.
Funny that this seems to return the pages in more or less oldest to most recent order, whereas the same parameter passed to
get_pagesreturns a completely different order 😕Uh oh!
There was an error while loading. Please reload this page.
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.
This is because get_pages has a default order of
ascand WP_Query (used by the REST API) has a default order ofdesc, I'll double check which one we need to respect for menu_order.It looks like we can't expect a stable sort from using menu_order, when items are equal to each other (eg menu_order of 0), so I'll need another dimension to sort on in addition to menu_order. Do folks prefer title alphabetizing or something like post date?
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.
For what it's worth, I don't think it's crucial to get this 100% right in this one PR. It's can be a nice enhancement in the future, but the value being able to convert the page list outweighs the downside that sorting might not be perfect, especially paired with the fact that you get immediate access to mover controls right after the conversion.
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.
Looks like it won't be possible to provide a consistent ordering until we can implement https://core.trac.wordpress.org/ticket/39037.
Good to know @tellthemachines! I'll look into adding multiple orderby support in the REST API next, then start on some general performance profiling for Navigation Links.
I've incorporated feedback and added a few explanations in e35f132