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

Add menu_order sorting option to Query Loop block #68781

Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from

Conversation

kasparsd
Copy link
Contributor

@kasparsd kasparsd commented Jan 20, 2025

What?

Add the menu_order sorting option in addition to post title and date.

Why?

Fixes #42710.

Users are able to set a numerical menu order for post types that support it (pages, by default). Smaller values rise to the top while larger values sink to the bottom, as implemented by WP_Query ordering.

How?

  1. Extend the QueryControls component to include menu_order as one of the options.
  2. Add "Menu order descending" and "Menu order ascending" options to the order selector.

Questions

  1. While the menu_order value is available for all post types in the database, only those that register support for page-attributes feature will have the setting available. What's the best approach to including the menu_order option only to post types with the page attributes enabled? Here is how the "Order" gets added conditionally to the dataviews:

    postTypeConfig.supports?.[ 'page-attributes' ]
    ? reorderPage
    : undefined,

Testing Instructions

  1. Create multiple pages.
  2. Set custom menu order values for each of the pages either in the page list (under Quick Edit) or via page settings which are hard to find (three dots menu).
  3. Create a new page and include a Query Loop block.
  4. Select pages post type and order by "Menu order".
  5. Confirm that the query loop output is rendering the posts in the expected order.

Testing Instructions for Keyboard

This change is extending an existing selector so the keyboard navigation remains unchanged.

Screenshots or screencast

Before After
no-order-option with-order-option

Ordering examples with updated preview:

Descending:

menu-order-descending

Ascending:

menu-order-ascending

@kasparsd kasparsd requested a review from ajitbohra as a code owner January 20, 2025 09:55
Copy link

github-actions bot commented Jan 20, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: kasparsd <[email protected]>
Co-authored-by: carolinan <[email protected]>
Co-authored-by: karmatosed <[email protected]>
Co-authored-by: Humanify-nl <[email protected]>
Co-authored-by: mikeritter <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@kasparsd kasparsd requested a review from fabiankaegy as a code owner January 20, 2025 09:56
@carolinan carolinan added [Type] Enhancement A suggestion for improvement. [Block] Query Loop Affects the Query Loop Block labels Jan 20, 2025
@@ -75,6 +75,37 @@ export function QueryControls( {
// but instead are destructured inline where necessary.
...props
}: QueryControlsProps ) {
const orderByList = [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The name here matches the props.categoriesList convention used below with orderBy being the feature.

@@ -23,6 +23,16 @@ const orderOptions = [
label: __( 'Z → A' ),
value: 'title/desc',
},
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These options should be available only to post types that support page attributes. Where and how do we add that logic? Here is a reference to how the "order" setting is decided at the page settings three-dots:

postTypeConfig.supports?.[ 'page-attributes' ]
? reorderPage
: undefined,

Copy link
Contributor

@carolinan carolinan Jan 20, 2025

Choose a reason for hiding this comment

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

I don't know if it helps but there is a PageAttributesCheck, that you can take inspiration from:
https://github.com/WordPress/gutenberg/blob/trunk/packages/editor/src/components/page-attributes/check.js
A check for the post type support should probably go in the block's utils.js file.

@carolinan
Copy link
Contributor

Thank you for working on this.
I understand that the menu order is the correct technical name, but it may be confusing for users, is there a better label for the select, that will make it easy to use for both developers and users?

In the block editor when you edit a page and change the order, the label for the option is just "Order", but "Order by order" is not a helpful label 😆

@kasparsd
Copy link
Contributor Author

Thank you for reviewing the changeset @carolinan!

but "Order by order" is not a helpful label

Right!? That is why I left it at "menu order" because I couldn't think of a better term that would work equally well between various post types. How about "Sort by manual order" or "Sort by custom order"?

@karmatosed
Copy link
Member

First, kudos for working on this, @kasparsd. Second, while the dropdown is totally native, I did wonder about bringing the same filtering now in from DataViews and the documentation in Storybook here. I don't want to derail this ticket though. Let's ship good things quickly. I will make another one as I feel some unification can be done. Beyond that, thank you again; it's so great to see.

@kasparsd
Copy link
Contributor Author

kasparsd commented Jan 20, 2025

Thank you @karmatosed for reviewing the ticket and providing the additional context!

I feel some unification can be done

I had the exact same feeling when I saw the ordering options repeated in three different places across the codebase. It feels like there must be some historic context that would explain that and potentially an existing initiative that is working to resolve this.

So I'm openly asking for support for (1) how to bring in the contextual availability for the menu_order option, and (2) how to name the actual option because "order by order" doesn't sound great and "menu order" isn't used anywhere in the UI.

@carolinan
Copy link
Contributor

carolinan commented Jan 21, 2025

It also can't use "Page order" since it can be enabled on other post types.
"manual order" sounds like I should be able to drag and drop the items to a completely custom order.

@karmatosed
Copy link
Member

karmatosed commented Jan 21, 2025

I had the exact same feeling when I saw the ordering options repeated in three different places across the codebase. It feels like there must be some historic context that would explain that and potentially an existing initiative that is working to resolve this.

My thinking was a different issue to solve that but happy to bring it to this. What I wanted was to get this one in. Let's do a quick audit of where and how are close to it for now. I am picking where order is set to allow a comparison and also showing the context:

  • Using dropdown component as you have.
  • Using popover show menu as posts do. Posts have a two-step which I am not fond of if honest. You select menu, show popover then also a modal appears on 'order'. We probably don't want that pattern here.
image

I am very reluctant to bring in another popover here as in this query interface they seem to be used primarily to expose hidden menus, this isn't the right place for that.

To recap what we see today in that section:

image

A possible idea would be to even consider if 'order by' should be in toolbar. I am slightly against this because I have seen many agencies extend this section. This is why I don't want to just add icons here either.

My recommendation

  • For now, stick with dropdown.
  • Revisit this area and others in another ticket.
  • Add the words 'Ascending by menu' or 'Descending by menu'. To align with language shared where order happens in modal, this probably could be 'Ascending by order' and 'Descending by order'.

My suggestion comes based on the current word patterns but it would be great to check those based on translations.

@kasparsd
Copy link
Contributor Author

Thanks for the additional context @karmatosed! I've updated the labels per your recommendation. I believe this is the best we can do without revisiting the whole "menu order" label across the admin (quick edit, page order configuration, etc.).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Query Loop Affects the Query Loop Block [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Query Loop Block should offer 'order by' 'page attribute' 'order'
3 participants