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 back the check for hierarchical post types, instead of just relying on page-attributes #108

Merged
merged 3 commits into from
Oct 28, 2022

Conversation

dkotter
Copy link
Collaborator

@dkotter dkotter commented Oct 27, 2022

Description of the Change

In #103, we changed our sortable check to only rely on if a post type supports page-attributes. This broke sorting for sites that use post types that are hierarchical but don't have that attribute set. While it may better align with how core is set up to only look at page-attributes, it seems best to revert that change so we're not breaking functionality on sites that rely on the old behavior.

This isn't a complete revert of the changes introduced in #103 but does bring back the missing functionality.

How to test the Change

  1. Create a post type that has page-attributes set
  2. Add content to this post type
  3. Ensure you can sort this content and it reflects properly on the front-end
  4. Create a post type that doesn't have page-attributes set but is set as hierarchical
  5. Run through the same test steps and ensure things work

Changelog Entry

Changed - Allow hierarchical post types that don't have page-attributes set to be sorted.

Credits

Props @dkotter

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

…ing on page-attributes. Add that information back to our readmes. This in essence reverts the changes done in #103
@dkotter dkotter self-assigned this Oct 27, 2022
@dkotter dkotter requested a review from peterwilsoncc October 27, 2022 20:47
@dkotter dkotter added this to the 2.4.3 milestone Oct 27, 2022
Copy link
Contributor

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

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

LGTM and tests well.

Is it worth changing the How can I exclude certain custom post types? section of the readme to:


How can I modify sortable post types?

Post types can be included or excluded by using the simple_page_ordering_is_sortable filter.

For example, to exclude the excluded_post_type custom post type, add the following snippet in the theme function file or custom plugin:

add_filter( 'simple_page_ordering_is_sortable', function( $sortable, $post_type ) {
	if ( 'excluded_post_type' === $post_type ) {
		return false;
	}
	return $sortable;
}, 10, 2 );

To include the include_post_type custom post type, add the following snippet in the theme function file or custom plugin:

add_filter( 'simple_page_ordering_is_sortable', function( $sortable, $post_type ) {
	if ( 'include_post_type' === $post_type ) {
		return true;
	}
	return $sortable;
}, 10, 2 );

simple-page-ordering.php Show resolved Hide resolved
@dkotter dkotter merged commit 4d4fd43 into develop Oct 28, 2022
@dkotter dkotter deleted the revert/103 branch October 28, 2022 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants