-
Notifications
You must be signed in to change notification settings - Fork 23
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
Introduce feature allowing modifying parent/child relationships. #172
Conversation
3fadc4e
to
8c02f5e
Compare
class-simple-page-ordering.php
Outdated
esc_url( $move_out_link ), | ||
/* translators: %s: Post title. */ | ||
esc_attr( sprintf( __( 'Move “%s” out', 'simple-page-ordering' ), $title ) ), | ||
__( 'Move out >', 'simple-page-ordering' ) |
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.
I'm not super keen on move in / move out as the text links -- it needs something clearer but also short so it fits the actions toolbar. Suggestions are encouraged, thanks.
Some shell commands for getting a bunch of posts with the same ID in the title as the post ID. Warning: it empties the entire post table. wp site empty
wp post generate --post_type=page
for i in {1..100}; do wp post update $i --post_title="Page $i"; done;
# To set menu order to the ID too
for i in {1..100}; do wp post update $i --post_title="Page $i" --menu_order=$i; done; |
Hello hi! After some noodling, I think the best option is
Lmk if any of these feel right or if you'd like me to keep thinking on it! |
Hello @peterwilsoncc, I have just tested this new edition, and it works well for me. I was wondering if we could implement a similar behavior for moving in and out of pages by using drag and drop? similar to how we navigate up and down pages. I am not familiar with the technical aspects of this change, but I wanted to share the idea and get your feedback. I noticed a similar flow in menu ordering as well: Screen.recording.1.webm |
@ankitguptaindia I had an attempt at that but couldn't get it working in an intuitive fashion. As the pages use tables rather than lists, the jQuery UI library only expects items to move up or down, I attempted to fake it but the drop targets' behaviour wasn't great. |
@peterwilsoncc besides tweaking the copy, is there anything else needed here before this can move forward towards review/release? |
@jeffpaul That should do. I'm worried |
@peterwilsoncc I'm fine with you YOLOing the decision on the copy and we can iterate as needed if there's a consensus in the community for something different |
@jeffpaul I've just pushed some changes that alter the copy to match that of the native menu pages in WordPress Core. "Move Under Page Name" sets the page as a child of page name I think the copy is clearer than move in/move out and matching existing core text is probably the best thing to do here. |
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.
@peterwilsoncc thank you The PR LGTM and also work as expected.
Only the description should be updated that mentions the "Move in"/"Move out" links.
@faisal-alvi I've updated the description with the new link text. |
Description of the Change
Adds row actions to move a page up or down the page hierarchy.
The "Move Out from under Parent Page name" action:
The "Move Under Page Name" action
At the moment this always triggers a full page refresh.
Closes #25.
How to test the Change
wp post generate --post_type=page
Changelog Entry
Credits
Props @peterwilsoncc.
Checklist: