Make WordPress Core

Opened 15 years ago

Closed 4 months ago

Last modified 4 months ago

#11302 closed enhancement (fixed)

Bulk editing posts should pre-fill fields with the same value / allow for removal

Reported by: pavelevap's profile pavelevap Owned by: francina's profile francina
Milestone: 6.5 Priority: normal
Severity: normal Version: 2.9
Component: Quick/Bulk Edit Keywords: needs-testing has-patch has-unit-tests commit
Focuses: accessibility, javascript, administration Cc:

Description

All I need is quickly move some posts from one category to another.

How to simulate this problem:

Create 2 posts in Uncategorized category.
Check both of them - Bulk actions - Edit.
There is no category selected even if both posts are in Uncategorized. When I check Category1 and click Update, both posts are now in Uncategorized and also in Category1. But no chance remove them from Uncategorized with Bulk actions.

Attachments (10)

bulk-edit-post-category.2.png (162.4 KB) - added by francina 4 years ago.
Screenshot of the bulk editing panel with undetermined state for checkboxes
screenshot-before-patch.png (404.7 KB) - added by ajmcfadyen 9 months ago.
Screenshot Before Patch https://patch-diff.githubusercontent.com/raw/WordPress/wordpress-develop/pull/4669.diff
screenshot-after-patch.png (402.5 KB) - added by ajmcfadyen 9 months ago.
Screenshot After Patch https://patch-diff.githubusercontent.com/raw/WordPress/wordpress-develop/pull/4669.diff
11302.diff (1.9 KB) - added by pbiron 8 months ago.
a fix for the same root cause identified in https://core.trac.wordpress.org/ticket/11302#comment:88, but in a more direct way
11302-patch-with-fix.png (33.3 KB) - added by oglekler 7 months ago.
11502.2.patch (8.6 KB) - added by oglekler 7 months ago.
11302.3.patch (4.3 KB) - added by ajmcfadyen 5 months ago.
11302.4.patch (4.4 KB) - added by ajmcfadyen 5 months ago.
11302.5.patch (4.5 KB) - added by ajmcfadyen 5 months ago.
11302.6.patch (4.4 KB) - added by ajmcfadyen 5 months ago.

Download all attachments as: .zip

Change History (121)

#1 @pavelevap
15 years ago

  • Cc pavelevap@… added

#2 @scribu
15 years ago

  • Component changed from UI to Administration
  • Keywords needs-patch added

Confirmed behaviour with 2.9-beta2.

#3 @scribu
15 years ago

  • Component changed from Administration to Quick Edit

Actually, no categories are checked!

#4 @scribu
15 years ago

  • Milestone changed from 2.9 to 3.0
  • Summary changed from Bulk actions for category change does not work to Can't removeCategories not checked in Quick edit
  • Type changed from defect (bug) to enhancement

The same goes for tags: the tag box will always be empty, even if both posts have one or more tags in common.

I think Quick Edit simply doesn't check similarities between posts at all, although it would be nice.

To get this working right will take some serious patching and a lot of testing. Punting to the next major release.

#5 @scribu
15 years ago

  • Summary changed from Can't removeCategories not checked in Quick edit to Bulk editing posts should pre-fill fields with the same value

#6 @nacin
14 years ago

  • Milestone changed from 3.0 to Future Release

No patch, punting.

#7 @scribu
14 years ago

Related: #12837

#8 @chased@…
13 years ago

I'd like to add that this seems to also appear when attempting to change Author with Bulk Edit. When doing so, changes are not saved.

#9 @helenyhou
12 years ago

Closed #21252 as a duplicate

#10 @SergeyBiryukov
11 years ago

Related: #24608

Version 0, edited 11 years ago by SergeyBiryukov (next)

#11 @helen
11 years ago

#24608 was marked as a duplicate.

#12 @helen
11 years ago

  • Summary changed from Bulk editing posts should pre-fill fields with the same value to Bulk editing posts should pre-fill fields with the same value / allow for removal

I could have sworn there was a comment to this effect somewhere already, but repeating it probably won't hurt.

There are two paths to allowing for removal of terms when it comes to UI + flow - that it can only be done when that term is already set for all posts in the selection (pre-filling with a check), or that we implement the indeterminate state (usually indicated by a checkbox with a dash through it) that allows the category to be removed from any posts that have that term set.

Implementation-wise, given that checkboxes don't send in the $_POST array when they aren't checked, a patch probably requires a good deal of thought. As noted in #8690, this is currently an add functionality, not replace. Indeterminate checkboxes are also not standardized in browsers or available in HTML.

Note that this only applies to hierarchical taxonomies. #19859 for non-hierarchical.

#13 @ubernaut
11 years ago

  • Cc ubernaut@… added

#14 @grahamarmfield
11 years ago

  • Cc graham.armfield@… added

#15 @joshcanhelp
10 years ago

This just recently came up in a project. The main issue is removing categories as they can be added in bulk with the current controls. Here are a few simpler options that came to mind:

1) Have the quick edit box react to whether you're already sorting by category in wp-admin. Above the list of posts, you have a category drop-down that shows posts within that category. The bulk edit function could key off of that and select the category you're currently viewing. If you submit and that category is gone, it's removed from the posts selected. This means you'd need to filter, select the posts, bulk edit, deselect, then save.

2) Another way to do this would be to have a separate category list for removal. It would show all categories that have posts assigned, all of them checked by default. You'd select the posts, bulk edit, deselect whatever categories you want to remove, then save. Any post that's selected would have the deselected categories removed.

Either one of those sound feasible?

#16 @ubernaut
10 years ago

personally i think the third mixed state/dashed indicator is the most elegant and conventional solution to this issue.

#17 @joshcanhelp
10 years ago

I definitely agree. I just figured one of these might be a good stop-gap solution (particularly the first since it's the least impactful on the UI as a whole).

The mixed-state checkbox, as Helen mentioned, is not available in HTML but there are good ways to implement with JS:

http://css-tricks.com/indeterminate-checkboxes/

#18 @ubernaut
10 years ago

interesting never actually knew that guess that's my new thing of the day. thanks for the link!

:)

Last edited 10 years ago by ubernaut (previous) (diff)

#19 @SergeyBiryukov
9 years ago

#33205 was marked as a duplicate.

#20 @Cyberchicken
9 years ago

@Helen: an old html trick to make checkboxes behave:
<input name="mycheckbox" value="nay" type="hidden">
<input name="mycheckbox" value="yes" type="checkbox">
You'll always be posted eithr a "nay" or a "yes".
Three-state require js for sure.

#21 @lau@…
9 years ago

I have a specific need and a simple suggested solution.

PROBLEM:
My store sells clothing so each season I add new clothes to the "New Arrivals" category and also to whatever category they will remain in over the long term e.g "Women's pants".
So every season I need to remove all the previous seasons clothes from the "New Arrivals" category, but I need them to remain in their other categories.

SOLUTION 1:
On the Edit Product Category page put a button at the bottom [ Remove all products from this Category ]

SOLUTION 2:
On the Product Categories list page add another Bulk Action "remove products from category"

SOLUTION 3:
In the short term I'd be happy if you could give me the sql to remove a categoryid from all products with that categoryid.

Thanks in advance
Lau

Last edited 9 years ago by lau@… (previous) (diff)

#22 @SergeyBiryukov
9 years ago

I've found the Batch-Move Posts plugin helpful as a workaround.

#23 follow-up: @ubernaut
9 years ago

so have we given up on the idea of making this a core feature? Seems to me a very basic UX feature that comes up in quite a few situations. I do also think that at some point (maybe in the past already) this becomes a competitive disadvantage versus other site building platforms. Relying on third party plugins to solve a common issue really seems less than ideal to me.

#24 in reply to: ↑ 23 @SergeyBiryukov
9 years ago

Replying to ubernaut:

so have we given up on the idea of making this a core feature?

I agree that it should be a core feature, we still need a patch though.

#25 @lau@…
9 years ago

I just tried the Batch-Move posts plugin but that only deals with post categories and posts. It doesn't deal with product categories and products.

I think I've figured out the SQL to do this. Can anybody tell me if I'm missing something?
In this case 493 is the term_id of the product category.

DELETE  term_relationships 
INNER JOIN term_taxonomy 
ON term_relationships.term_taxonomy_id = term_taxonomy.term_taxonomy_id
AND term_taxonomy.term_id = 493

UPDATE kry_term_taxonomy SET
count = 0
WHERE term_taxonomy.term_id = 493

ANOTHER SOLUTION which may be easier for you guys to implement as it eliminates a lot of complexity and potential for serious bugs and user misunderstanding is to have a second icon/link column in the Product Category page, next to the "Count" column. When clicked it takes you to a completely new page that lists all the products in the category (not the page that you go to when you click "count").
This new page has (at a minimum) a tickbox next to each product and two "Bulk Actions":
1) Remove from category
2) Add specified to category (you then have a dropdown with a list of categories at the top of the page)

I think this is probably the technically easiest solution and covers all the bases, even if some bases require a little extra user effort.
I think trying to do it through the bulk edit feature of the current product list page will be technically difficult for you guys and confusing for the user. The problem is how to indicate which categories are already allocated to the set of products, and especially the categories which are only allocated to a subset of the products (greyed out tickbox). I think that many users won't know what the greyed out tickbox represents and may tick or untick it incorrectly and then you get a lot of angry people who unwittingly removed or added a bunch of products to categories that they didn't mean to.

By creating the new page for products in a category you could even add an additional cool feature: you could use the term_relationships.term_order field to store the default order that the products should be displayed on the category page, and you could let the users specify the order by either:
1) a single field on each product row that you save using ajax to term_relationships.term_order
or
2) a funky feature to drag each product row into the proper order. (Best is to have both methods so that if you have 100 products in a category and you want to move a few from the top to much lower down don't have to do lots of scrolling/dragging.

Last edited 9 years ago by SergeyBiryukov (previous) (diff)

#26 @SergeyBiryukov
9 years ago

#34849 was marked as a duplicate.

This ticket was mentioned in Slack in #core by benoitchantre. View the logs.


8 years ago

#28 @Marcoevich
8 years ago

Hi guys I was wondering if there's any progress on this one?

#29 @ubernaut
8 years ago

doesn't seem like it :(

#30 @SergeyBiryukov
8 years ago

#35548 was marked as a duplicate.

#31 @tomybyte
8 years ago

No question, if you can bulk add a category, you just should be able to remove it.
I just had the experience to mark the wrong category in bulk edit for a lot oft posts. Just saw that while clicking on the submit button. But it was to late, I just had added the wrong category.
Now I have to change this for every post. Ok with quickedit it won't take so long, but anyway it is annoying not being able to correct such a mistake in bulk edit.

#32 @thinkluke
8 years ago

Agreed, this would be a nice core feature.

This ticket was mentioned in Slack in #forums by sergey. View the logs.


7 years ago

#34 @dd32
7 years ago

#40918 was marked as a duplicate.

#35 @dd32
7 years ago

#40801 was marked as a duplicate.

#36 @SergeyBiryukov
7 years ago

#41964 was marked as a duplicate.

#37 @Virtality Marketing Solutions
7 years ago

WordPress team, why is this feature taking so long? People have been asking for it for years and it makes managing products and large blogs a real pain. This is why people are now moving to solutions like Shopify. I'm a huge WordPress fan and want to stay one :-) Please get this one on deck please please :-)

#38 @Michalooki
7 years ago

If it is not possible for now to add this feature of pre-fill fields with the same value / allow for removal,
Will it be possible to to add some kind of extra field that allow to make a bulk removal of a category from a list of posts? This will help alot. Thanks.

#39 @ubernaut
7 years ago

Seems hard for me to believe, not actually being a programmer myself, that none of these people who keep bringing this up have the skills to implement this or that the powers that be arent sick of marking other tickets as duplicates.

Obviously, there is demand and we've had the UX worked out for years now. This is a very basic problem and i feel like it may signify a greater problem with the whole process of waiting till some one person or group who happens to have the capability to do something about it cares enough about this specific thing to do something.

Meanwhile, a great number of people pay the price through clunky and arduous UX. Bulk de-selection is not rocket science and we should not be treating it as such, imo.

#40 @dmsnell
7 years ago

After some conversations today I stumbled upon this ticket after thinking, "if the ability to bulk-remove categories isn't in WordPress already it must have some challenging long-term issues." Turns out I was right! Anyway I'd love to try and eye this ticket for Contributor Day at WordCamp US 2017 and on that hope would love to get some conversation/feedback started here so I can prepare for it.

Since it's been four years since issues with browser compatibility for indeterminate checkboxes was discussed I decided to look it up: all major browsers should support the tri-state box but it's still not possible to indicate that indeterminate state with HTML only - one must use JavaScript.

https://html.spec.whatwg.org/multipage/input.html#dom-input-indeterminate

Question one is how we feel about the tri-state checkbox in general. Is it the right idiom? I wasn't sure if the indeterminate state should imply that the category should be removed or if it would mean we aren't intending on changing the category. My conclusion was that unchecked should imply removing the category, checked should imply adding the category, and indeterminate should imply not changing anything.

Maybe on account of this confusion I had a hard time seeing the tri-state actually solving our wants. It's almost as if we need something else in addition to the checkbox.

Question two is how important it is to us that this all work without JavaScript. I believe that we should be able to figure out how to make this all work with the appropriate scripting before submitting the form, but does the form have to be able to work without it? The list of posts to the left of the category box appears to already rely on JavaScript for its functioning.

Question three is maybe less important for discussion because we can always "make it work" but how should we represent this on the API side. My guess is that we can probably add some new "categories to remove" attribute in the request.


It was hard for me to think up examples of existing UI for this purpose. LightRoom doesn't use checkboxes: they have tag names, tag names with a star at the end, and the absence of tag names to indicate "all items have this tag," "some items have this tag," and "no items have this [non-visible] tag." It doesn't list all of the available tags either, which is a bit simpler than our category bulk edit box.

After some doodling I came up with an idea which would indicate both the addition or not of a category as well as its removal: a hoverable x to the left of the checkmark. The video below is crude and a prototype and is only there for the idea. The picture shows one way we might be able to indicate that a category is on the removal list.

https://cldup.com/TMlmeM2_WU.gif https://cldup.com/ZYUaFYRQQ3.png


This makes me wonder too if a radio button isn't technically more appropriate. Of course the styling would need to be made nice but I think we're truly dealing with a tai-state field: add to all, remove from all, or leave them all as they are.

Could we style a radio button in a way that communicates these states the way we want?

Anyway, thanks to all who provide feedback here. Once I have a better idea of what we would want to build I can work on creating a patch.

#41 @ubernaut
7 years ago

Thanks, @dmsnell for your thoughtful response and willingness to chip in to try to make this happen. :) My vote on Q1 is for the Tri state (dashed checkbox) UI at this point. It is the only solution to this issue i've ever seen used anywhere else. You cannot set the third state it is always preset to indicate a mixed selection. Unchecking it sets all selected items to the false state, checking it sets all to true, not clicking simply leaves everything the way it is.

Not saying it's the best solution but it is the simplest, most intuitive one imo, at least that ive seen to date.

No opinion on Q 2 or 3 bc i cant code my way out of a paper bag, sorry.

Last edited 7 years ago by ubernaut (previous) (diff)

#42 @itecrs
6 years ago

There is no need for complicated solutions. All it takes is one more checkbox named "Remove previous categorization" and couple of lines of code. If you check this box, it will clear category info for selected products, and set new categorization as you select in list of categories. If you not check this box, it will work the way it does already.

#43 @pannelars
6 years ago

Still no plan for a release?? Miss this function

#44 @WHSajid
6 years ago

Looks like there's no solution to this yet. In the meantime, can I request a filter inside the bulk_edit_posts function? Possibly around the part where the new categories are being merged with the old ones? (Line 566-570 in includes/post.php) So that a simpler plugin can be developed (without creating an entire bulk edit plugin) to allow users to completely override existing categories. I know it's an ugly bandaid on a serious wound, but I think it can be a temporary relief.

#45 @ubernaut
6 years ago

there is a solution but nobody wants to code it up.

This ticket was mentioned in Slack in #core by francina. View the logs.


5 years ago

#47 @samba45
5 years ago

Possible solution, interrupt the SQL relationship between post and category with scripts from phpmyadmin.
Of the type:
Suppose we have the table prefix wp_blog, launching the command "delete from wp_blog_term_relationships where term_taxonomy_id = 1 and object_id in (select id from wp_blog_posts where post_type = 'post' and post_status = 'publish');" we break the correlation between the two elements (post and uncategorized) this action has value only on the post set to "published".
We report the counter value of the uncategorized category.
From phpmyadmin:
"Update wp_blog_term_taxonomy set count = 0 where term_taxonomy_id = 1;"
Method to use with caution, risk damaging SQL database, make backups before any action.
Result: Removes "uncategorized from posts"

Last edited 5 years ago by samba45 (previous) (diff)

#48 @Mte90
5 years ago

Looking also at the other ticket seems that we need to do a new UI to manage better the categories in the bulk mode specifically on to remove the categories itself from the posts.
So probably is more on javascript side than in PHP and we need to investigate also what is the best place for that code.
Maybe we need to see also the bulk edit mode why is not generating rightly the UI, maybe is something that should be splitted in two parts and one involve also the Design team?

#49 @johnbillion
5 years ago

  • Keywords needs-design added

Yeah this needs some design work and consideration of using the indeterminate state for checkboxes, similar to how Gmail does when bulk managing labels on emails.

#50 @tomluckies
4 years ago

Plugin for anyone looking for a solution in the meantime: https://wordpress.org/plugins/bulk-remove-posts-from-category/

This ticket was mentioned in Slack in #core by soulseekah. View the logs.


4 years ago

#52 @soulseekah
4 years ago

Hey, we're looking at this ticket at WCEU2020, if anyone is interested in contributing thoughts, code, etc. please hop onto Slack #core channel.

#53 @soulseekah
4 years ago

I think that using https://caniuse.com/#search=indeterminate is actually one of the best approaches with little overhead. But I'm not sure whether it works well or not, I had to hack it up a bit.

This is a feature plugin for this issue https://github.com/soulseekah/11302-core

Let's see if we can test this (especially UI/UX-wise) and get something more solid going. It will need at least one new filter, and edit.js will contain the JS blob that I put in my code (yes, yes, kittens died, inline JS, inline CSS, yes, sorry).

Cheers.

#54 @francina
4 years ago

Thanks @soulseekah - with a small delay of 3 months I tested this and on WordPress 5.6-alpha-48783, Chrome Version 84.0.4147.125
It works!

So, what's next? :-)

Is the indeterminate checkbox state supported by all the browsers that WordPress supports?

@francina
4 years ago

Screenshot of the bulk editing panel with undetermined state for checkboxes

#55 @francina
4 years ago

  • Focuses javascript administration added

Tested again on web site running WordPress 5.6 RC1, PHP 7.3.2, on MacOs BigSur.

Next steps

  1. Review code
  2. Turn it into a patch
  3. Cross-browser testing of the undetermined checkboxes styling (separate ticket?)

#56 @ubernaut
4 years ago

@francina looks great to me! :)

This ticket was mentioned in Slack in #core by francina. View the logs.


4 years ago

#58 @SergeyBiryukov
4 years ago

  • Milestone changed from Future Release to 5.7

Moving to the milestone to test the approach suggested in comment:53, which appears to have some consensus.

#59 @SergeyBiryukov
4 years ago

  • Owner set to francina
  • Status changed from new to assigned

This ticket was mentioned in Slack in #design by hellofromtonya. View the logs.


3 years ago

#61 @francina
3 years ago

  • Milestone changed from 5.7 to Future Release

This ticket was mentioned in Slack in #core by francina. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by francina. View the logs.


3 years ago

#64 @SergeyBiryukov
2 years ago

#55757 was marked as a duplicate.

This ticket was mentioned in Slack in #core by francina. View the logs.


15 months ago

This ticket was mentioned in Slack in #core by francina. View the logs.


12 months ago

#67 @oglekler
11 months ago

  • Milestone changed from Future Release to 6.4

Moving it to 6.4 to try to tackle it in the next release.

This ticket was mentioned in Slack in #design by chaion07. View the logs.


10 months ago

#69 @francina
9 months ago

Can I help move the ticket forward? What do you need? I have tested the existing patch from @soulseekah and have used it in production since 2020 👼

This is still the plan, as far as I understand:

  1. Review code
  2. Turn it into a patch
  3. Cross-browser testing of the undetermined checkboxes styling (separate ticket?)

@costdev can you assist? Thanks!

This ticket was mentioned in PR #4669 on WordPress/wordpress-develop by @costdev.


9 months ago
#70

  • Keywords has-patch added; needs-patch removed

This pre-fills post categories either with a checked checkbox, or a line indicating an indeterminate status.

#71 @costdev
9 months ago

Testing Instructions

These steps indicate how to test PR 4669.

Steps to Test

  1. Apply the patch.
  2. Run npm run build:dev.
  3. Assign some posts to various categories. Make sure that some posts are in one category, some are in another, and some are in both.
  4. Navigate to Posts > All Posts.
  5. You may need to perform a hard refresh of the page to ensure new styles will load as expected during the later steps.
  6. Check the checkbox beside one or more posts.
  7. Select Edit from the Bulk Actions dropdown, then click Apply.
  8. Look at each category's checkbox.

Expected Results

When testing a patch to validate it works as expected:

  • ✅ If all posts are assigned to a given category, that category's checkbox should be checked.
  • ✅ If some of the posts are assigned to a given category, that category's checkbox should have a line indicating an indeterminate status.
  • ✅ If none of the posts are assigned to a given category, that category's checkbox should appear empty (not checked and has no line).

#72 @oglekler
9 months ago

  • Keywords needs-testing added

This ticket was mentioned in Slack in #core-test by oglekler. View the logs.


9 months ago

#74 follow-up: @ajmcfadyen
9 months ago

Test Report

Env / Setup:

Theme: twentythirteen
Plugins: N/A
WordPress: 6.4-alpha-56267-src
OS: macOS Monterey
Localhost: wp-env
Browser: Chrome, Firefox, Safari

Setup Steps:

  • Apply the patch.
  • Run npm run build:dev.
  • Assign some posts to various categories. Make sure that some posts are in one category, some are in another, and some are in both.
  • Navigate to Posts > All Posts.
  • You may need to perform a hard refresh of the page to ensure new styles will load as expected during the later steps.
  • Check the checkbox beside one or more posts.
  • Select Edit from the Bulk Actions dropdown, then click Apply.
  • Look at each category's checkbox.

Results, Confirmed (✅)

✅ If all posts are assigned to a given category, that category's checkbox is checked.
✅ If some of the posts are assigned to a given category, that category's checkbox has a line indicating an indeterminate status.
✅ If none of the posts are assigned to a given category, that category's checkbox appears (not checked and has no line).

Last edited 9 months ago by ajmcfadyen (previous) (diff)

#75 in reply to: ↑ 74 @ubernaut
9 months ago

love it :)

This ticket was mentioned in Slack in #core-test by ajmcfadyen. View the logs.


9 months ago

#77 @oglekler
9 months ago

  • Keywords commit added

I will not add screenshots, since I tested the patch and it works as expected. But to see the change, a hard reload is necessary.

I am marking this patch for review to commit.

#78 @mukesh27
9 months ago

  • Keywords needs-testing removed

Thanks @costdev for PR and test report.

We get ✅ in test report. @costdev are you going to commit this one?

#79 @hellofromTonya
9 months ago

Is this ready for commit? It still has needs-design keyword on it.

cc @oglekler

#80 follow-up: @oglekler
9 months ago

  • Keywords needs-design removed

@hellofromTonya, forgive me,
We don't need the design. It looks just as everyone expected it to. So, there are no doubts about this. See the screenshot above.

#81 in reply to: ↑ 80 @hellofromTonya
9 months ago

Replying to oglekler:

@hellofromTonya, forgive me,
We don't need the design. It looks just as everyone expected it to. So, there are no doubts about this. See the screenshot above.

Thanks for confirming. Okay, that clears PR 4669 for commit consideration.

#82 @costdev
9 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 56712:

Quick/Bulk Edit: Pre-fill category fields with their status.

This pre-fills category fields in the Quick/Bulk Edit form with their current status.

When bulk editing, if only some of the selected items are in a given category, the category's checkbox will display a line to indicate an indeterminate status.

Props pavelevap, scribu, chasedsiedu, helen, joshcanhelp, ubernaut, Cyberchicken, laumindproductscomau, SergeyBiryukov, Marcoevich, tomybyte, thinkluke, virtality-marketing-solutions, Michalooki, dmsnell, itecrs, pannelars, WHSajid, samba45, Mte90, johnbillion, tomluckies, soulseekah, francina, oglekler, ajmcfadyen, mukesh27, costdev.
Fixes #11302.

@costdev commented on PR #4669:


9 months ago
#83

Committed in r56712.

This ticket was mentioned in Slack in #core by peterwilsoncc. View the logs.


8 months ago

#85 @hellofromTonya
8 months ago

  • Keywords has-patch commit removed
  • Milestone changed from 6.4 to 6.5
  • Resolution fixed deleted
  • Status changed from closed to reopened

[56712] will be reverted shortly for 6.4.1. Why? It introduced an issue reported in #59837 where bulk edits removes all categories and reassigns them to Uncategorized.

Reopening this ticket and moving to 6.5 to resolve the issue and then give it plenty of time for discussion, testing, and possible iteration.

#86 @peterwilsoncc
8 months ago

In 57093:

Quick/Bulk Edit: Prevent assigning posts to default categories during bulk edit.

During a bulk edit of posts with different categories, the categories for the edited posts would be reset to the default category: uncategorized by default.

This reverts [56712] to resolve the issue.

Props peterwilsoncc, hellofromtonya, jorbin.
Fixes #59837.
See #11302.

#87 @peterwilsoncc
8 months ago

In 57094:

Quick/Bulk Edit: Prevent assigning posts to default categories during bulk edit.

During a bulk edit of posts with different categories, the categories for the edited posts would be reset to the default category: uncategorized by default.

This reverts [56712] to resolve the issue.

Merges [57093] to the 6.4 branch.

Props peterwilsoncc, hellofromtonya, jorbin.
Fixes #59837.
See #11302.

#88 @joedolson
8 months ago

The problem with the patch originally committed is that the variable indeterminatePostCategoryField is overwritten each time it's used, so it only ever applies to the list listed indeterminate input. Declaring the variable inside the conditional fixes this issue.

// Compute initial states.
$( '.inline-edit-categories input[name="post_category[]"]' ).each( function() {
	// Clear indeterminate states.
	$( '<input type="hidden" name="indeterminate_post_category[]">' ).remove();

	if ( categories[ $( this ).val() ] == checkedPosts.length ) {
		// If the number of checked categories matches the number of selected posts, then all posts are in this category.
		$( this ).prop( 'checked', true );
	} else if ( categories[ $( this ).val() ] > 0 ) {
		// If the number is less than the number of selected posts, then it's indeterminate.
		$( this ).prop( 'indeterminate', true );
		var indeterminatePostCategoryField = $( '<input type="hidden" name="indeterminate_post_category[]">' );

		// Set indeterminate states for the backend.
		indeterminatePostCategoryField.val( $( this ).val() );
		$( this ).after( indeterminatePostCategoryField );
	}
} );

There are probably other methods to solve this; but this is the cause of the issue that required the revert.

#89 @joedolson
8 months ago

Before this is added back in, I'd like it to get some additional accessibility work. The state of the checkbox is announced as half checked (NVDA/Chrome), but that's not sufficient information to understand what's going on. I think that these indeterminate checkboxes need additional labeling to indicate what state means, e.g. "Some selected posts have this category".

#90 @joedolson
8 months ago

  • Focuses accessibility added

@pbiron
8 months ago

a fix for the same root cause identified in https://core.trac.wordpress.org/ticket/11302#comment:88, but in a more direct way

#91 @pbiron
8 months ago

to test 11302.diff, checkout r57092, and then apply the patch.

#92 @oglekler
7 months ago

I tried the updated patch (there is "'" needs to be removed) but I don't think that it all what we need. See the screenshot above. I believe that the problem is that the task wasn't clear enough.

  1. If categories aren't changed — they shouldn't be changed.
  2. If some categories are applied, the post should still have categories that it had previously — If category wasn't touched, it should remain its previous relation with all edited posts.
  3. If some categories are excluded, they need to be removed from all posts.

#93 @oglekler
7 months ago

Update: Patch is working. There is just another '"' is missed after name argument. But I am suggesting to add this check as well:

if ( ! $( this ).parent().find( 'input[name="indeterminate_post_category[]"]' ).length ) {
	// Set indeterminate states for the backend.
	$( this ).after( '<input type="hidden" name="indeterminate_post_category[]" value="' + $( this ).val() + '">' );
}

Otherwise, if you are opening Bulk Edit then canceling it and opening it again, hidden inputs are starting to duplicate.

Last edited 7 months ago by oglekler (previous) (diff)

@oglekler
7 months ago

#94 @oglekler
7 months ago

  • Keywords needs-testing has-patch added

This ticket was mentioned in Slack in #core by oglekler. View the logs.


7 months ago

#96 @oglekler
7 months ago

  • Keywords needs-e2e-tests added

This ticket was discussed during a bug scrub (see the link above).

This ticket needs to be tested properly with full list of possibilities to avoid previous mistake: to think about possible scenarios, write them down and then test according to them.

This ticket also needs e2e tests.

Add props: @webcommsat @pbiron @jorbin

#97 @joedolson
7 months ago

The patch still needs the accessibility work mentioned in comment 89, so let's make sure to address that as well before moving this forward.

This ticket was mentioned in Slack in #core by abhanonstopnews. View the logs.


6 months ago

#99 @webcommsat
6 months ago

Highlighted at today's bug scrubs (December 19, 2023) for testing. Also accessibility work needed.

@ajmcfadyen
5 months ago

#100 @ajmcfadyen
5 months ago

Here's an updated patch with accessible label text for the indeterminate checkboxes: https://core.trac.wordpress.org/attachment/ticket/11302/11302.3.patch

if ( ! $( this ).parent().find( 'input[name="indeterminate_post_category[]"]' ).length ) {
	// Set indeterminate states for the backend.
	$( this ).attr( 'label', wp.i18n.__( 'Some selected posts have this category' ) ).after( '<input type="hidden" name="indeterminate_post_category[]" value="' + $( this ).val() + '">' );
}

@ajmcfadyen
5 months ago

@ajmcfadyen
5 months ago

@ajmcfadyen
5 months ago

#101 @ajmcfadyen
5 months ago

Here's a follow-up to that patch to remove the label when the checkbox is interacted with & to switch the label attribute to an aria-label: https://core.trac.wordpress.org/attachment/ticket/11302/11302.6.patch. Hopefully, that covers accessibility. I've tested using voiceover. @joedolson is that what you had in mind?

I looked at the category checklist walker and potentially adding the indeterminate checkbox status server-side but wasn't sure if that was needed. I considered removing the aria-label in the inline-edit-save button click function but didn't think it would help much. Also, I added the :indeterminate pseudo selector to the onChange function that removes the hidden indeterminite fields because it only needs to run on indeterminite inputs.

Regarding testing, when bulk-editing posts:

Verify that the checked, unchecked, and indeterminate checkbox states in the post-category term list correspond to the terms on the posts being edited. (https://core.trac.wordpress.org/ticket/11302#comment:71)

  • If all posts have a term the checkbox of that term should be checked
  • If none of the posts selected for editing have the term, it should be unchecked
  • If some of the posts have the term, the checkbox should be indeterminate

Confirm that saving via the bulk editor functions as expected. (https://core.trac.wordpress.org/ticket/59837, https://core.trac.wordpress.org/ticket/11302#comment:92)

  • Verify that terms can be added or removed in bulk
  • Confirm that terms are not overwritten when saving. Unchanged terms should remain unchanged.

Hope that helps! Sorry for attaching so many versions

#102 @joedolson
5 months ago

Thanks, @ajmcfadyen! I'll do some testing of this. It still needs e2e tests, as well.

This ticket was mentioned in PR #5919 on WordPress/wordpress-develop by @joedolson.


5 months ago
#103

  • Keywords has-unit-tests added

Trac ticket: 11302

#104 @joedolson
5 months ago

  • Keywords needs-e2e-tests removed

Added three unit tests: one for no change, one for a category added, and one for a category removed; and I adjusted the label text so that the term name is included. The aria-label did a good job of indicating that the checkbox applied to some posts, but had lost the category text.

There are probably other unit tests that could be added, but these cover the obvious scenarios, anyway.

This ticket was mentioned in Slack in #core-test by joedolson. View the logs.


5 months ago

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


5 months ago

#107 follow-up: @huzaifaalmesbah
4 months ago

Test Report

Environment

  • WordPress: 6.5-alpha-56966-src
  • PHP: 8.2.15
  • Server: nginx/1.25.3
  • Database: mysqli (Server: 8.0.36 / Client: mysqlnd 8.2.15)
  • Browser: Chrome 121.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Four 1.0
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.0.1

Screenshots

Before Apply Patch After Apply Patch ✅
https://i.ibb.co/bRD1Kvp/Screenshot-2024-02-10-at-12-02-53-AM.png https://i.ibb.co/x2M30dn/Screenshot-2024-02-10-at-12-01-21-AM.png

#108 in reply to: ↑ 107 @ubernaut
4 months ago

Replying to huzaifaalmesbah:

looks great!

#109 @joedolson
4 months ago

  • Keywords commit added

This is effectively the same patch as was committed previously, but with the addition of a bug fix, unit tests, and an accessibility fix. I think we should get it in. Take 2!

#110 @joedolson
4 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 57580:

Quick/Bulk Edit: Pre-fill category fields with their status.

Pre-fill category fields in the Quick/Bulk Edit form with their current status.

When bulk editing, if only some of the selected items are in a given category, the category's checkbox will display a line to indicate an indeterminate status.

Originally committed in [56172], but reverted due to a bug that removed all categories. Updated commit fixes the bug, adds unit tests, and improves the accessibility of the indeterminate state checkboxes.

Props pavelevap, scribu, chasedsiedu, helen, joshcanhelp, ubernaut, Cyberchicken, laumindproductscomau, SergeyBiryukov, Marcoevich, tomybyte, thinkluke, virtality-marketing-solutions, Michalooki, dmsnell, itecrs, pannelars, WHSajid, samba45, Mte90, johnbillion, tomluckies, soulseekah, francina, oglekler, ajmcfadyen, mukesh27, costdev, hellofromTonya, peterwilsoncc, joedolson, pbiron, oglekler, webcommsat, jorbin, ajmcfadyen, huzaifaalmesbah.
Fixes #11302.

Note: See TracTickets for help on using tickets.