#16502 closed enhancement (fixed)
Quick Edit contents should only be rendered if quick edit button in actions after filtering
Reported by: | wyrfel | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | 6.4 | Priority: | normal |
Severity: | normal | Version: | 3.0.4 |
Component: | Quick/Bulk Edit | Keywords: | has-patch needs-testing commit |
Focuses: | Cc: |
Description (last modified by )
_the_post()
has a static call to get_inline_data()
to generate the quick_edit
content. This happens even if after running the (undocumented) post_row_actions
filters the Quick-Edit link ($actions['inline hide-if-no-js']
) is not amongst the actions anymore.
Prefixing the get_inline_data()
call with
if (isset($actions['inline hide-if-no-js'])
would be a simple solution, enabling people to cancel the performance-impacting quick-edit form generation alltogether.
Attachments (5)
Change History (69)
#2
@
14 years ago
- Milestone changed from Awaiting Review to Future Release
- Type changed from defect (bug) to enhancement
#6
@
3 years ago
- Description modified (diff)
- Keywords needs-testing added
- Milestone set to Future Release
Also, Core disables quick edit for (reusable) blocks.
Sample filter to use for testing other post types:
add_filter( 'post_row_actions', 'list_row_actions_remove_quick_edit', 10, 1 ); function list_row_actions_remove_quick_edit( $actions ) { unset( $actions['inline hide-if-no-js'] ); return $actions; }
This ticket was mentioned in Slack in #core by abhanonstopnews. View the logs.
2 years ago
#10
@
2 years ago
Hi there!
I have tested patch 16502.1.diff and it not working for me. After the patch when i click on Quick Edit
it will remove all the input values from the edit form.
@SergeyBiryukov @sabernhardt Is it working for you?
#11
@
2 years ago
- Keywords needs-patch added; has-patch needs-testing removed
Wow. I apparently did not test with post types where the Quick Edit button *is* available (unless maybe something changed since I tested). We need a new patch.
The column_title
function is not going to find $actions['inline hide-if-no-js']
when it does not make sure the $actions
array is set first.
Thanks for testing!
This ticket was mentioned in Slack in #core by marybaum. View the logs.
2 years ago
This ticket was mentioned in PR #3152 on WordPress/wordpress-develop by mukeshpanchal27.
2 years ago
#13
- Keywords has-patch added; needs-patch removed
Trac ticket:
#15
@
2 years ago
- Keywords needs-testing added
Ping @sabernhardt @SergeyBiryukov for the peer review so we can move forward on this ticket.
#16
@
2 years ago
- Keywords has-testing-info changes-requested added
Test Report
Patch tested: https://github.com/WordPress/wordpress-develop/pull/3152
Steps to Reproduce or Test
- Create a new file
wp-content/plugins/test-16502.php
with the following contents:<?php /** * Plugin Name: Test #16502 * Description: For testing <a href='https://core.trac.wordpress.org/ticket/16502'>16502</a>. * Author: WordPress Core Contributors * Author URI: https://make.wordpress.org/core * License: GPLv2 or later * Version: 1.0.0 */ add_action( 'init', 'test_16502_cpt' ); add_action( 'admin_bar_menu', 'test_16502_adminbar', 999 ); add_filter( 'post_row_actions', 'test_16502_hide_quick_edit', 10, 1 ); add_filter( 'page_row_actions', 'test_16502_hide_quick_edit', 10, 1 ); add_filter( 'tag_row_actions', 'test_16502_hide_quick_edit', 10, 1 ); add_filter( 'comment_row_actions', 'test_16502_hide_quick_edit_comments', 10, 1 ); function test_16502_cpt() { register_post_type( '16502', array( 'public' => true, 'label' => 'Test 16502' ) ); } function test_16502_adminbar( $wp_admin_bar ) { global $pagenow; $action = empty( $_GET['hide_quick_edit'] ) ? '1' : '0'; $url = add_query_arg( array_merge( $_GET, array( 'hide_quick_edit' => $action ) ), admin_url( $pagenow ) ); $wp_admin_bar->add_node( array( 'id' => 'test-16502', 'title' => $action ? 'Remove Quick Edit' : 'Add Quick Edit', 'href' => $url ) ); } function test_16502_hide_quick_edit( $actions ) { if ( isset( $_GET['hide_quick_edit'] ) && '1' === $_GET['hide_quick_edit'] ) unset( $actions['inline hide-if-no-js'] ); return $actions; } function test_16502_hide_quick_edit_comments( $actions ) { if ( isset( $_GET['hide_quick_edit'] ) && '1' === $_GET['hide_quick_edit'] ) unset( $actions['quickedit'] ); return $actions; }
- Navigate to
Plugins > Installed Plugins
and activate Test #16502. - Navigate to
Posts
. - Open DevTools and inspect a post's title.
- Below the post's title
<strong>
markup, there will be<div class="hidden" id="inline_XXXX">
element. - In the admin bar, click "Remove Quick Edit".
- 🐞 Repeat steps 4-5.
- Click "Add Quick Edit".
- Apply the PR.
- ✅ Repeat steps 3-7.
You may also test the following screens:
Posts > Categories
Posts > Tags
Pages
Test 16502
Expected Results
When reproducing a bug:
- ❌ The hidden "inline_XXXX" element should exist even though "Quick Edit" is removed.
When testing a patch to validate it works as expected:
- ✅ Posts (and other screens): The element should no longer exist when "Quick Edit" is removed.
Environment
- Server: Apache (Linux)
- WordPress: 6.1-alpha-53344-src
- Browser: Chrome 104.0.0.0
- OS: Windows 10
- Theme: Twenty Twenty-Two
- Plugins:
- Test #16502 1.0.0
Actual Results
When reproducing a bug/defect:
- ✅ Issue reproduced: The hidden "inline_XXXX" element still exists even though "Quick Edit" is removed.
When testing the bugfix patch:
- ✅ Posts: The hidden "inline_XXXX" element no longer exists when "Quick Edit" is removed.
- ✅ Pages: The hidden "inline_XXXX" element no longer exists when "Quick Edit" is removed.
- ✅ Custom Post Type (Test 15602): The hidden "inline_XXXX" element no longer exists when "Quick Edit" is removed.
- ❌ Tags: The hidden "inline_XXXX" element still exists even though "Quick Edit" is removed.
- ❌ Categories: The hidden "inline_XXXX" element still exists even though "Quick Edit" is removed.
Notes
- The patch works for Posts, Pages and Custom Posts Types, so this could be committed as-is.
- However, IMO, the patch should fix Tags and Categories too.
- Also, I didn't get to test with
Comments
, but I think the patch should cover this too.
#17
@
2 years ago
Thanks for the test report and feedback, @costdev.
As requested, I implemented the changes for the category and tags. I also tried to comment and got an error in the unit test for Ajax.
1) Tests_Ajax_EditComment::test_as_admin Undefined index: hook_suffix /var/www/src/wp-admin/includes/class-wp-screen.php:223 /var/www/src/wp-admin/includes/template.php:2601 /var/www/src/wp-admin/includes/class-wp-list-table.php:149 /var/www/src/wp-admin/includes/class-wp-comments-list-table.php:55 /var/www/src/wp-admin/includes/list-table.php:61 /var/www/src/wp-admin/includes/template.php:446 /var/www/src/wp-admin/includes/class-wp-comments-list-table.php:872 /var/www/src/wp-admin/includes/class-wp-list-table.php:1455 /var/www/src/wp-admin/includes/class-wp-comments-list-table.php:644 /var/www/src/wp-admin/includes/ajax-actions.php:1477 /var/www/src/wp-includes/class-wp-hook.php:307 /var/www/src/wp-includes/class-wp-hook.php:331 /var/www/src/wp-includes/plugin.php:517 /var/www/tests/phpunit/includes/testcase-ajax.php:265 /var/www/tests/phpunit/tests/ajax/EditComment.php:62 phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:51 2) Tests_Ajax_EditComment::test_editor_can_edit_orphan_comments Undefined index: hook_suffix /var/www/src/wp-admin/includes/class-wp-screen.php:223 /var/www/src/wp-admin/includes/template.php:2601 /var/www/src/wp-admin/includes/class-wp-list-table.php:149 /var/www/src/wp-admin/includes/class-wp-comments-list-table.php:55 /var/www/src/wp-admin/includes/list-table.php:61 /var/www/src/wp-admin/includes/template.php:446 /var/www/src/wp-admin/includes/class-wp-comments-list-table.php:872 /var/www/src/wp-admin/includes/class-wp-list-table.php:1455 /var/www/src/wp-admin/includes/class-wp-comments-list-table.php:644 /var/www/src/wp-admin/includes/ajax-actions.php:1477 /var/www/src/wp-includes/class-wp-hook.php:307 /var/www/src/wp-includes/class-wp-hook.php:331 /var/www/src/wp-includes/plugin.php:517 /var/www/tests/phpunit/includes/testcase-ajax.php:265 /var/www/tests/phpunit/tests/ajax/EditComment.php:110 phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:51 3) Tests_Ajax_GetComments::test_as_admin Undefined index: hook_suffix /var/www/src/wp-admin/includes/class-wp-screen.php:223 /var/www/src/wp-admin/includes/template.php:2601 /var/www/src/wp-admin/includes/class-wp-list-table.php:149 /var/www/src/wp-admin/includes/class-wp-comments-list-table.php:55 /var/www/src/wp-admin/includes/list-table.php:61 /var/www/src/wp-admin/includes/template.php:446 /var/www/src/wp-admin/includes/class-wp-comments-list-table.php:872 /var/www/src/wp-admin/includes/class-wp-list-table.php:1455 /var/www/src/wp-admin/includes/class-wp-comments-list-table.php:644 /var/www/src/wp-admin/includes/ajax-actions.php:1265 /var/www/src/wp-includes/class-wp-hook.php:307 /var/www/src/wp-includes/class-wp-hook.php:331 /var/www/src/wp-includes/plugin.php:517 /var/www/tests/phpunit/includes/testcase-ajax.php:265 /var/www/tests/phpunit/tests/ajax/GetComments.php:59 phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:51 4) Tests_Ajax_ReplytoComment::test_as_admin Undefined index: hook_suffix /var/www/src/wp-admin/includes/class-wp-screen.php:223 /var/www/src/wp-admin/includes/template.php:2601 /var/www/src/wp-admin/includes/class-wp-list-table.php:149 /var/www/src/wp-admin/includes/class-wp-comments-list-table.php:55 /var/www/src/wp-admin/includes/list-table.php:61 /var/www/src/wp-admin/includes/template.php:446 /var/www/src/wp-admin/includes/class-wp-comments-list-table.php:872 /var/www/src/wp-admin/includes/class-wp-list-table.php:1455 /var/www/src/wp-admin/includes/class-wp-comments-list-table.php:644 /var/www/src/wp-admin/includes/ajax-actions.php:1403 /var/www/src/wp-includes/class-wp-hook.php:307 /var/www/src/wp-includes/class-wp-hook.php:331 /var/www/src/wp-includes/plugin.php:517 /var/www/tests/phpunit/includes/testcase-ajax.php:265 /var/www/tests/phpunit/tests/ajax/ReplytoComment.php:73 phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:51
I found a similar error report #29933 If I make these changes, it will resolve my unit test errors. @SergeyBiryukov, could you please help me with this?
/wp-admin/includes/class-wp-screen.php on line 223 $id = $GLOBALS['hook_suffix']; Replace with $id = isset($GLOBALS['hook_suffix']) ? $GLOBALS['hook_suffix'] : "";
peterwilsoncc commented on PR #3152:
2 years ago
#19
I've merged in trunk to resolve the merge conflict following a4ec93f438988e1df0e81e93a43e8d706ba22089
peterwilsoncc commented on PR #3152:
2 years ago
#20
Hmm, resolving merge conflict resulted in the ajax failing in PHP 8.1, so I need to hold off merging for now. I'm checking to see if I did something silly.
#21
@
2 years ago
- Keywords changes-requested removed
The PR got enough approval for the merge. Please review and share your thought.
mukeshpanchal27 commented on PR #3152:
2 years ago
#22
We are near to Beta 1.
As note by @costdev https://github.com/WordPress/wordpress-develop/pull/3152#discussion_r962906194 If we update the WP_Screen::get()
code https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-admin/includes/class-wp-screen.php#L221-L225 then it works without adding the global $hook_suffix;
for the comment changes in 8.1
if ( $hook_name ) { $id = $hook_name; } else { $id = $GLOBALS['hook_suffix']; }
Replace with
if ( $hook_name ) { $id = $hook_name; } elseif ( wp_doing_ajax() && isset( $_REQUEST['action'] ) ) { $id = 'wp_ajax_' . esc_attr( $_REQUEST['action'] ); } elseif ( isset( $GLOBALS['hook_suffix'] ) ) { $id = $GLOBALS['hook_suffix']; } else { $id = ''; // Or just let it fail here. }
I'm happy to update it if it makes sense, or remove the quick edit changes for the comment and add a new issue in trac for it. Any thoughts, @peterwilsoncc @SergeyBiryukov?
Additionally, I checked some old tickets that have issues with hook_suffix
#38761 in which they update unit test code. Ping @johnbillion if you have any thoughts on this.
#23
@
2 years ago
- Milestone changed from 6.1 to Future Release
With 6.1 Beta 1 releasing tomorrow and this ticker still needing discussion and a decision, this is being moved to Future Release
.
If any maintainer/committer wishes to assume ownership of this during a specific cycle, feel free to update the milestone at that time.
#24
follow-up:
↓ 25
@
2 years ago
- Focuses performance removed
This ticket is not performance related so I am removing the tag.
#25
in reply to:
↑ 24
@
2 years ago
Replying to adamsilverstein:
This ticket is not performance related so I am removing the tag.
I believe the performance improvement mentioned in the ticket description was to avoid unnecessarily adding hidden fields with the Quick Edit data if they are not going to be used, though I agree the effect of that would likely be small, unless the post has a lot of terms, or something resource-heavy is attached to the add_inline_data
filter.
peterwilsoncc commented on PR #3152:
2 years ago
#26
@costdev @mukeshpanchal27 Are you able to refresh this now the hook fix has been merged?
2 years ago
#27
@mukeshpanchal27 Let me know whether you have time to refresh, or if you want me to handle it :slightly_smiling_face:
mukeshpanchal27 commented on PR #3152:
2 years ago
#28
@peterwilsoncc @costdev I have refreshed PR code. Can we set 6.1 milestone for this ticket?
2 years ago
#29
@mukeshpanchal27 that's an enhancement ticket so it'll have to wait for 6.2 at this point as all features/enhancements needed to be merged before 6.1 Beta 1.
mukeshpanchal27 commented on PR #3152:
2 years ago
#30
Thanks Jeff.
#31
@
2 years ago
- Milestone changed from Future Release to 6.2
Move to 6.2
consideration as PR is ready for final review and merge.
cc. @costdev @peterwilsoncc
This ticket was mentioned in Slack in #core by abhanonstopnews. View the logs.
2 years ago
This ticket was mentioned in Slack in #core by costdev. View the logs.
23 months ago
#34
@
22 months ago
Test Report
This report validates that the indicated patch addresses the issue (except Tags & Categories pages).
Patch tested: https://patch-diff.githubusercontent.com/raw/WordPress/wordpress-develop/pull/3152.diff
Environment
- OS: macOS 12.6 (21G115)
- Web Server: Nginx
- PHP: 7.4.33
- WordPress: 6.2-alpha-54642-src
- Browser: Google Chrome
- Theme: Twenty Twenty-Three
- Active Plugins:
- Test #16502
Actual Results
- ✅ Issue resolved with patch (except Tags & Categories pages).
Additional Notes
- ❌ Patch doesn't fix for 'Categories' & 'Tags' pages.
Supplemental Artifacts
For All Posts page:
Before: https://prnt.sc/EfHw1uVHEv4V
After1: https://prnt.sc/beU3ORq-0OPt
After2: https://prnt.sc/s-TKp0dcCzKH
For Categories page:
Before: https://prnt.sc/g6UVGd5_KKE2
After: https://prnt.sc/u6BI8F2oCz86
For Tags page:
Before: https://d.pr/i/MppgtQ
After: https://prnt.sc/hf_zU66u9JBK
#35
@
22 months ago
Test Report
This report validates that the indicated patch addresses the issue (excluding Tags and Categories pages).
Patch tested: https://patch-diff.githubusercontent.com/raw/WordPress/wordpress-develop/pull/3152.diff
Environment
- OS: macOS 11.2.3 (20D91)
- Web Server: Nginx
- PHP: 7.4.30
- WordPress: 6.2-alpha-54642-src
- Browser: Chrome 109.0.5414.119
- Theme: Twenty Twenty-Three
- Active Plugins:
- Test #16502
Actual Results
- ✅ Issue resolved with patch (excluding Tags and Categories).
Additional Notes
- ✅ All Posts Page: Issue resolved.
- ❌ Tags Page: Element still exists
- ❌ Categories Page: Element still exists
Supplemental Artifacts
All Posts:
Before - https://d.pr/i/y5u70O
After - https://d.pr/i/H5oaZQ
Tags Page:
Before - https://d.pr/i/YtmruC
After - https://d.pr/i/WD2VYA
Categories Page:
Before - https://d.pr/i/bDG1KJ
After - https://d.pr/i/BEEC3f
#36
@
22 months ago
@costdev @mukesh27 do you think we can commit without Tags and Categories pages?
Currently it fixes All posts page.
I am okay with/without Tags and Categories pages fix.
#37
@
22 months ago
Thanks @faguni22 @robinwpdeveloper for the test report.
I think for the category and tags we can add followup ticket so we get merge this ~12 years old ticket. WDYT @costdev?
#38
@
22 months ago
January 30, 2023 scrub:
- we have tested the patch and have the same results that the patch does not work for categories and tags
- can we determine whether a revised patch could fix categories and tags, or whether this can be committed on one aspect to get into 6.2, with a new ticket opening up for categories and tags fix.
@costdev do you have a view please?
#39
@
22 months ago
Hi all. Yes, the categories and tags will need some more consideration as there's possibly a little bit of refactoring to do to enable us to prevent the "inline_XXXX"
div from being output.
Until then, I think we can proceed with PR 3152 for 6.2 and continue the work for Categories and Tags in 6.3.
If committers agree, we can create the follow-up ticket once the PR has been committed and the ticket resolved as fixed
.
Pinging @peterwilsoncc and @SergeyBiryukov for their thoughts as two committers who have reviewed the PR.
#40
@
22 months ago
- Keywords needs-dev-note added
@costdev Splitting the patches up makes sense but if it's across milestone, let's set up a second ticket to avoid moving tickets around once they have commits on them.
My once concern with this change is if a plugin author has modified the actions but relies on the div being present for some custom code. Adding needs-dev-note accordingly.
#41
@
22 months ago
- Keywords needs-testing removed
That sounds good to me Peter!
As this ticket has multiple test reports that show that all non-Category/Tag functionality is working, I'll remove needs-testing
.
#43
@
22 months ago
Hi! I wonder if I am missing something, another patch possibly.
I looked up and tested the patch PR 3152 with WP v 6.1.1 and:
$actions['inline hide-if-no-js']
is set, so, inline data is still added.- Reply form in comments is moved across, and the result is that hidden data is still present, but forms are not working properly for the last item in comments list (please, look the video above).
- Browser is not requesting data to edit by ajax at the moment of click on the Quick edit link and just filling in the form with existing data which can be obsolete at the time of editing if, for example, several editors are fixing things at the same time. So, I think that the whole point of not rendering editing form is to be sure that the data is actual at the moment of editing.
- Browser will not render invisible content until it become visible, so, improvement in performance when unnecessary data will be removed is quite small.
Please, consider the purpose of the ticket and take a second look for what tables and how are affected.
#44
@
22 months ago
- Keywords dev-feedback added
Thanks @oglekler! I believe you've spotted a valid issue with the patch. It seems that the Quick Edit
and Reply
features both use markup generated by the wp_comment_reply()
function, and that the patch is missing consideration of the reply
action.
Adding isset( $actions['reply'] ) ||
to the start of the conditions at src/wp-admin/includes/class-wp-comments-list-table.php:867
appears to resolve the issue.
However, since this particular markup is not isolated to one feature (covers both Quick Edit
and Reply
), we might need to consider whether changing the Comments screen is necessary here.
For now, I'm thinking that we should remove the changes to src/wp-admin/includes/class-wp-comments-list-table.php
and investigate the Comments screen in a separate ticket.
Given that we'll be addressing Tags/Categories/Other Terms in #57596, I think the PR should probably just target src/wp-admin/includes/class-wp-posts-list-table.php
to remove unnecessary rendering in Posts/Pages/Custom Post Type list tables. After all, the ticket description only refers to post_row_actions
/get_inline_data()
.
@oglekler @mukesh27 @peterwilsoncc What do you think?
#45
@
22 months ago
- Keywords has-unit-tests added
I've attached some PHPUnit tests to cover a final patch.
These cover:
- A post with the
inline hide-if-no-js
action. - A page with the
inline hide-if-no-js
action. - A hierarchical custom post type with the
inline hide-if-no-js
action. - A non-hierarchical custom post type with the
inline hide-if-no-js
action. - A post without the
inline hide-if-no-js
action. - A page without the
inline hide-if-no-js
action. - A hierarchical custom post type without the
inline hide-if-no-js
action. - A non-hierarchical custom post type without the
inline hide-if-no-js
action.
#46
@
22 months ago
- Keywords commit added
PR 3152 has been updated to only contain changes for Posts/Pages/Custom Post Types and add the tests above so that we can address Tags/Categories and Comments in a later cycle.
This looks ready for commit
consideration to me.
Additional props: @mukesh27
This ticket was mentioned in Slack in #core by abhanonstopnews. View the logs.
22 months ago
#48
@
22 months ago
Does this result in a change to the position of elements within the DOM? It looks like the output is still within the same td cell but it would be good to confirm.
#49
@
22 months ago
- Milestone changed from 6.2 to 6.3
This ticket was discussed in the pre-6.2 Beta 1 bug scrub. Moving to 6.3 so that a committer can weigh in.
#50
follow-up:
↓ 54
@
22 months ago
- Keywords 2nd-opinion added; has-patch needs-dev-note commit removed
Having some doubts about this patch. It effectively disables one of the WordPress features: Quick Edit. As far as I know this feature is not used very often, but is quite helpful for Editors on sites with many users/authors.
The disabling is done indirectly, by "piggybacking" on other/existing methods. Don't think this is a good way to disable a feature. A better way would be to have a filter where extenders will be able to disable it directly.
In addition the current code may introduce bugs/regressions in plugins that replace the "Quick Edit" action link with another method of showing the UI, or reuse the quick edit data in some way.
#51
@
18 months ago
- Milestone changed from 6.3 to 6.4
I have doubts as well (#comment:43) we need clear understanding what need to be done and then compare it with patch. If we are improving performance, the result needs to be measured.
This is an enhancement, and we are in 12 days until Beta 1 after which we will not add new enhancement to the release, so, because there is no patch, I am moving this ticket to 6.4.
This ticket was mentioned in Slack in #core by oglekler. View the logs.
16 months ago
#53
@
16 months ago
- Keywords changes-requested added; has-testing-info has-unit-tests removed
- Milestone changed from 6.4 to Future Release
Because there is no movement lately, I am moving this ticket into Future release. If there will be a new patch, and it will be working correctly, it can go to the next available release.
#54
in reply to:
↑ 50
;
follow-up:
↓ 57
@
16 months ago
- Keywords has-patch added; dev-feedback changes-requested removed
- Milestone changed from Future Release to 6.4
Replying to azaozz:
Having some doubts about this patch. It effectively disables one of the WordPress features: Quick Edit. As far as I know this feature is not used very often, but is quite helpful for Editors on sites with many users/authors.
The disabling is done indirectly, by "piggybacking" on other/existing methods. Don't think this is a good way to disable a feature. A better way would be to have a filter where extenders will be able to disable it directly.
In addition the current code may introduce bugs/regressions in plugins that replace the "Quick Edit" action link with another method of showing the UI, or reuse the quick edit data in some way.
Indeed, removing $actions['inline hide-if-no-js']
via the post_row_actions
filter seems like a hacky way to disable the Quick Edit functionality.
Looking at the current PR, moving the get_inline_data()
call to the handle_row_actions()
method does not seem ideal either, because the former displays the markup directly, while the latter is supposed to return the result as a string and should not display anything.
16502.2.diff introduces two new filters to address both this ticket and #57596:
quick_edit_enabled_for_post_type
quick_edit_enabled_for_taxonomy
That seems like a cleaner way to do this, and should also resolve the backward compatibility concerns.
#56
@
15 months ago
Hey @azaozz,
What do you think about @SergeyBiryukov approach in 16502.2.diff? Does it address your concerns?
#57
in reply to:
↑ 54
@
15 months ago
- Keywords commit added; 2nd-opinion removed
Replying to SergeyBiryukov:
That seems like a cleaner way to do this, and should also resolve the backward compatibility concerns.
Yep, the new filters make sense imho.
Does it address your concerns?
Sure, LGTM now.
#58
follow-up:
↓ 59
@
15 months ago
Naming things nitpick -- quick_edit_enabled_for_posts
or quick_edit_enabled_for_post_objects
may be better.
add_filter( 'quick_edit_enabled_for_post_type', '__return_false' )
disables the feature for all posts so could be seen as misleading.
Given @SergeyBiryukov, @azaozz, @hellofromTonya and @oglekler are all very experienced contributes, it's also possible I'm over thinking this so I won't be upset if you disregard this comment :)
#59
in reply to:
↑ 58
;
follow-up:
↓ 60
@
15 months ago
Replying to peterwilsoncc:
add_filter( 'quick_edit_enabled_for_post_type', '__return_false' )
disables the feature for all posts so could be seen as misleading.
I was aiming for consistency with the existing use_block_editor_for_post_type
filter :)
Curious to see what others think.
#60
in reply to:
↑ 59
@
15 months ago
Replying to SergeyBiryukov:
I was aiming for consistency with the existing
use_block_editor_for_post_type
filter :)
That works for me.
#61
@
15 months ago
- Owner changed from chriscct7 to SergeyBiryukov
- Status changed from reviewing to accepted
@SergeyBiryukov commented on PR #3152:
15 months ago
#63
Thanks for the PR! An alternative approach was merged in r56611.
Ooops, second closing bracket is missing above.