Page List: sort by menu_order and page title for consistent ordering #30683
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: -3 B (0%) Total Size: 1.06 MB
ℹ️ View Unchanged
|
gwwar
force-pushed
the
try/page-list-stable-sort
branch
from
e1ae225 to
421cf9e
Compare
annezazu
added
[Block] Page List
gwwar
force-pushed
the
try/page-list-stable-sort
branch
from
421cf9e to
2df228e
Compare
| // sort. | ||
| orderby: 'menu_order', | ||
| //TODO: orderby menu_order,title when https://core.trac.wordpress.org/ticket/39037 is implemented | ||
| orderby: 'title', |
There was a problem hiding this comment.
This feels more disruptive when we have ordered pages than what's in trunk, because it makes all the links jump around, whereas before they would only change if you had more than one item with the same order. If pages aren't ordered though, it behaves better. Not sure which is best 😕 I wonder how commonly the "order" feature is used?
Alternatively, would passing 'menu_order, ID' instead of 'menu_order, post_title' to get_pages solve the inconsistency issue? That would render the page list exactly the same as this query does with menu_order. The downside is that it won't give us the same order as the classic Pages widget with the menu order option selected, so if the main concern is being able to use Page List as a replacement for the widget, we should stick to 'menu_order, post_title'.
#31382 doesn't mention the Navigation at all, so I assume the problem is mostly with the order within Page List itself, which kind of points to it being used as a replacement widget.
There was a problem hiding this comment.
The rest api doesn't support multiple sorts, so results will be inconsistent depending on query size. We could leave it as menu_order.
For multiple sorts, I do have WordPress/wordpress-develop#1186 but I got a little stuck since it has a breaking change for an existing hook
There was a problem hiding this comment.
We can always revisit this later once the REST issue is fixed. title will work better with no ordering, and that's the default state, so I guess we can go with that for now 😄
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow up to #30390 . Fixes #31382
and pairs with WordPress/wordpress-develop#1186example.mp4
Many pages have a
menu_orderof 0. We should not expect a stable sort order fromWP_Queryorget_pages, when comparing two items of equal sort value. (Though this may change if WP_Query is updated https://core.trac.wordpress.org/ticket/47642#comment:13 )Proposed changes in the PR add a secondary sort of 'title' the PageList
render_callback. Since we can't sort by multiple columns for the REST API (see https://core.trac.wordpress.org/ticket/39037) this also updates the PageList->NavigationLinks transform to use title sorting for now.Testing Instructions