Make the filter links links in the posts list table filterable#9950
Make the filter links links in the posts list table filterable#9950SirLouen wants to merge 4 commits into
Conversation
|
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 Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
| ); | ||
|
|
||
| if ( 'current' === $css_class ) { | ||
| if ( 'current' === $edit_filter_link_vars['css_class'] ) { |
There was a problem hiding this comment.
What if other CSS classes had been added? This seems like an ideal place to use WP_HTML_Tag_Processor::parse_class_list() proposed in #10043 by @dmsnell. In that way, this code could instead be:
| if ( 'current' === $edit_filter_link_vars['css_class'] ) { | |
| if ( in_array( 'current', iterator_to_array( WP_HTML_Tag_Processor::parse_class_list( $edit_filter_link_vars['css_class'] ), true ) { |
There was a problem hiding this comment.
with all of the attribute activity going on here we could also benefit at large from using the HTML API.
$processor = new WP_HTML_Tag_Processor( '<a>LINK_TEXT</a>' );
$processor->next_token();
$processor->set_attribute( 'href', $edit_filter_link_vars['url'] );
$processor->set_attribute( 'class', $edit_filter_link_vars['css_class'] ?? null );
$processor->set_attribute( 'aria-current', 'page' );
$processor->next_token();
$processor->set_modifiable_text( $edit_filter_link_vars['link_text'] );
return $processor->get_updated_html();There was a problem hiding this comment.
well I don’t know. to be honest I didn’t fully understand the new filter, but I think the HTML API can play a nice role in constructing the HTML wherever it’s created, even if that’s before filtering.
| * } | ||
| * @param array $args URL parameters for the link. | ||
| */ | ||
| $edit_filter_link_vars = apply_filters( 'edit_filter_links', $edit_filter_link_vars, $args ); |
There was a problem hiding this comment.
Is this the right filter name? Shouldn't it be singular?
| $edit_filter_link_vars = apply_filters( 'edit_filter_links', $edit_filter_link_vars, $args ); | |
| $edit_filter_link_vars = apply_filters( 'edit_filter_link', $edit_filter_link_vars, $args ); |
There was a problem hiding this comment.
Or rather, this filter still is difficult to understand.
Since it is specifically for filtering the link used in a column of the post list table, maybe:
post_list_column_edit_link
There was a problem hiding this comment.
But it also seems strange that the link itself is not being filtered. It's the array that goes into constructing the link. That being the case, then it should be something like: post_list_column_edit_link_vars.
But that feels wrong too. Why not just let the HTML be constructed as it had been before, and then add a filter for the resulting HTML? This feels better to me:
diff --git a/src/wp-admin/includes/class-wp-posts-list-table.php b/src/wp-admin/includes/class-wp-posts-list-table.php
index bc25dd0045..82781f82f2 100644
--- a/src/wp-admin/includes/class-wp-posts-list-table.php
+++ b/src/wp-admin/includes/class-wp-posts-list-table.php
@@ -272,13 +272,27 @@ class WP_Posts_List_Table extends WP_List_Table {
}
}
- return sprintf(
+ $edit_link = sprintf(
'<a href="%s"%s%s>%s</a>',
esc_url( $url ),
$class_html,
$aria_current,
$link_text
);
+
+ /**
+ * Filters an edit link which appears in a column of the post list table.
+ *
+ * This links to another post list table view to list posts by an author or posts in a category, for example.
+ *
+ * @since 9.6.0
+ *
+ * @param string $edit_link Edit link.
+ * @param string[] $args Associative array of URL parameters for the link.
+ * @param string $link_text Link text.
+ * @param string $css_class Optional. Class attribute. Default empty string.
+ */
+ return apply_filters( 'post_list_table_edit_link', $edit_link, $args, $link_text, $css_class );
}
/**There was a problem hiding this comment.
Then the HTML Tag Processor can be used to manipulate the edit link as desired.
There was a problem hiding this comment.
I will note that one problem I see over and over again with this kind of filtering is that we really don’t have a solution for proper escaping. by passing HTML through a filter we have mushed together actual HTML and escaped text nodes. this makes it hard to properly filter, because people would need to use the HTML Processor if they want to filter it after that. they should not use string functions.
I’ve been working on some ideas for improving this situation but they aren’t going to be ready for 6.9.0 and they aren’t going to fully address this.
at least when we are talking about the constituent parts we can agree that the values are “raw PHP strings.” once we mush things we lose the ability to track which parts are which.
this is a long-winded answer to a simple question, and it’s not in any way unique to this change, but it’s the reason I don’t feel like I have a simple response.
I’m reluctant to pass the HTML Processor or Tag Processor as the input to filters, but this is a kind of example where we could be exploring newer semantic filters.
maybe we could export the proposed attributes and children of the A element and let people much with them.
The original proposal seems simpler and raises fewer questions than this version of the patch.
add_filter('edit_link_url', 'wp_edit_link_filter');
function wp_edit_link_filter($url){
return add_query_arg(array('taonomy'=>'slug'), $url);
}
| * } | ||
| * @param array $args URL parameters for the link. | ||
| */ | ||
| $edit_filter_link_vars = apply_filters( 'edit_filter_links', $edit_filter_link_vars, $args ); |
There was a problem hiding this comment.
But it also seems strange that the link itself is not being filtered. It's the array that goes into constructing the link. That being the case, then it should be something like: post_list_column_edit_link_vars.
But that feels wrong too. Why not just let the HTML be constructed as it had been before, and then add a filter for the resulting HTML? This feels better to me:
diff --git a/src/wp-admin/includes/class-wp-posts-list-table.php b/src/wp-admin/includes/class-wp-posts-list-table.php
index bc25dd0045..82781f82f2 100644
--- a/src/wp-admin/includes/class-wp-posts-list-table.php
+++ b/src/wp-admin/includes/class-wp-posts-list-table.php
@@ -272,13 +272,27 @@ class WP_Posts_List_Table extends WP_List_Table {
}
}
- return sprintf(
+ $edit_link = sprintf(
'<a href="%s"%s%s>%s</a>',
esc_url( $url ),
$class_html,
$aria_current,
$link_text
);
+
+ /**
+ * Filters an edit link which appears in a column of the post list table.
+ *
+ * This links to another post list table view to list posts by an author or posts in a category, for example.
+ *
+ * @since 9.6.0
+ *
+ * @param string $edit_link Edit link.
+ * @param string[] $args Associative array of URL parameters for the link.
+ * @param string $link_text Link text.
+ * @param string $css_class Optional. Class attribute. Default empty string.
+ */
+ return apply_filters( 'post_list_table_edit_link', $edit_link, $args, $link_text, $css_class );
}
/**3 participants
Refreshing and adding an improvement to 35099.patch
Trac ticket: https://core.trac.wordpress.org/ticket/35099
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.