Introduce 'View all' post type filter on pull screen #725
Conversation
includes/classes/PullListTable.php
Outdated
| @@ -258,7 +259,7 @@ public function column_date( $post ) { | |||
| public function column_default( $item, $column_name ) { | |||
| switch ( $column_name ) { | |||
| case 'name': | |||
| return $item['post_title']; | |||
| return $item->post_title; | |||
There was a problem hiding this comment.
Looking at this, doesn't seem we even use this output, as the name column gets rendered in the _column_name callback function. Might be worth just removing this case.
There was a problem hiding this comment.
@dkotter Updated.
includes/classes/PullListTable.php
Outdated
| @@ -258,7 +259,7 @@ public function column_date( $post ) { | |||
| public function column_default( $item, $column_name ) { | |||
| switch ( $column_name ) { | |||
| case 'name': | |||
| return $item['post_title']; | |||
| return $item->post_title; | |||
| case 'url': | |||
There was a problem hiding this comment.
I know this is unrelated to any changes here but also doesn't seem we ever use a url column and if so, probably worth removing this case statement here as we're modifying other things in this UI
There was a problem hiding this comment.
I agree that even some light cleanup is worthwhile while we're working on this section of the code.
There was a problem hiding this comment.
@dkotter Updated.
|
@elliott-stocks Overall looks good, left a few minor comments. But there is an issue in regards to external connections. If you select As an example, here's one place that will throw a PHP error when |
Thanks, @dkotter ! Do we want to enable this for external connections? I think it works well for internal but not too sure about external - happy to disable the "view all" for external if you agree. |
I guess my expectation would be that this would work on both. It would be a little weird from a user perspective to lose options on that Pull screen if you switch connection types. But that said, because of the way we handle running a query on an external connection, not sure there's a straight forward way to make this all work, at least without having to refactor that query portion. I guess in theory we could loop through the post types and run individual queries for each of those and then combine and reduce those results to the most recent. There are performance issues to consider there though, especially for sites that have lots of post types. |
If we just want to add it for Internal Connections now, that's fine with me. I do have concerns both about the user expectations that both connection types would behave the same and it does make the code a little messier, as I think we would need to have conditionals in a few places to change behavior depending on the connection type. But I'm happy to review any changes to this PR if we want to just apply this to Internal Connections for now. |
|
Thanks, both! @dkotter I've pushed updates to address the default column logic and to limit the new filter to internal connections. |
Description of the Change
Introduce a 'View all' filter on the pull screen to allow users to view all post types by default instead of filtering manually. Also added a new column to the pull screen table to show which post type is currently in view - useful for when all are being displayed.
This handles one of the planned changes outlined in #392.
Alternate Designs
N/A
Benefits
Allows users to view all available post types on the pull screen without having to filter separately.
Possible Drawbacks
Verification Process
Checklist:
Applicable Issues
#392
Changelog Entry