WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 3 weeks ago

#22938 assigned enhancement

Presentation of hierarchical taxonomy in Media modal should be checkboxes rather than comma-separated tag list

Reported by: yeswework Owned by: wonderboymusic
Milestone: Future Release Priority: normal
Severity: normal Version: 3.5
Component: Media Keywords: needs-patch
Focuses: ui, javascript Cc:

Description

Since 3.5, using register_taxonomy_for_object_type on attachments, if the taxonomy used is hierarchical, whereas in the edit attachment UI you see the normal list of checkboxes, in the corresponding modal it is presented as a comma-separated list of slugs, as if it were a non-hierarchical taxonomy (tags rather than categories). I'm sure this is not a bug / mistake / oversight, but at best it's a little unintuitive (you need to have memorised the category slugs to add new ones) and worst a bit dangerous (risk of adding unwanted categories), and it would be great if in future it was presented here too as a list of checkboxes.

Attachments (3)

Screen Shot 2012-12-14 at 15.01.02.png (7.4 KB) - added by yeswework 2 years ago.
shows a hierarchical taxonomy presented like a tag list in 3.5 media modal
22938.patch (2.9 KB) - added by jessepollak 7 months ago.
Screenshot 2014-10-26 20.04.22.png (515.2 KB) - added by jessepollak 7 months ago.
Screenshot of checkbox attachment taxonomy chooser

Download all attachments as: .zip

Change History (20)

@yeswework2 years ago

shows a hierarchical taxonomy presented like a tag list in 3.5 media modal

comment:1 @knutsp2 years ago

  • Cc knut@… added

comment:2 @SergeyBiryukov2 years ago

  • Component changed from General to Administration

comment:3 @helen2 years ago

  • Component changed from Administration to Media
  • Keywords ui-focus added

comment:4 @mendelk2 years ago

  • Cc menkra@… added

comment:5 @ericlewis12 months ago

  • Keywords needs-patch added
  • Version set to 3.5

comment:6 @leemon9 months ago

Now the new 4.0 media library edit attachment modal shows categories as a comma-separated list of slugs, too. Why? This is a backward step.

comment:7 @DrewAPicture9 months ago

  • Milestone changed from Awaiting Review to 4.1

I'd like to come up with a solution for this in 4.1. Definitely seems pretty weird to relegate hierarchical taxonomies to non-hierarchical-type input boxes.

comment:8 @wonderboymusic8 months ago

  • Focuses javascript added
  • Owner set to wonderboymusic
  • Status changed from new to assigned

I will fall on this sword

comment:9 @ericlewis8 months ago

Select2 y'all

comment:10 @ircbot8 months ago

This ticket was mentioned in IRC in #wordpress-dev by DrewAPicture. View the logs.

comment:11 @slackbot7 months ago

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

comment:12 @jessepollak7 months ago

I have a patch for this incoming — I'll get it up later this evening. It's not perfect, so I'm hoping to get some feedback advice, then clean it up.

comment:13 follow-ups: @jessepollak7 months ago

  • Keywords has-patch dev-feedback needs-testing added; needs-patch removed

I'm attaching a patch & screenshot of an MVP of the fix for this. Right now, it displays the categories for a media item as a list of checkboxes and saves them. I have a few questions and TODOs:

  1. I use wp_terms_checklist() which in turn relies on Walker_Category_Checklist which is relatively inflexible. Primarily, it can only output inputs of the form name=post_category[] or tax_input[$taxonomy]. That's a different form than what's used to saved AttachmentCompat's in Backbone, so I have to str_replace to get it in the correct form. I know this is horrible, but I'm not sure what the best way to proceed is — should I rewrite Walker_Category_Checklist, so it can be more modular?
  1. wp_terms_checklist() outputs to the buffer, so I use ob_start() etc to get the content which is passed via AJAX. Is this acceptable or — again — should I refactor to make it possible to get it as a string rather than output?
  1. In the media.view.AttachmentCompat class, I add an if statement in the save method that handles the case where the input is a series of checkboxes (before this, they were always a single input of type=text or type=hidden). Is this acceptable or should I subclass media.view.AttachmentCompat specifically for ones that are taxonomies of some sort.
  1. I've only been testing this in the Attachment Edit & Create pages and with the category taxonomy. Can anyone think of other objects and/or taxonomies I should be testing on?
  1. I have done zero styling of the check box. Right now, on my checklist is: restrict height of container and add overflow: scroll, indent child categories, align checkbox and text, add visual border to contain checkboxes. Any other thoughts?

Appreciate any and all feedback :)

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

@jessepollak7 months ago

@jessepollak7 months ago

Screenshot of checkbox attachment taxonomy chooser

comment:14 @slackbot6 months ago

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

comment:15 @johnbillion6 months ago

  • Milestone changed from 4.1 to Future Release

comment:16 in reply to: ↑ 13 @husobj2 months ago

Replying to jessepollak:

  1. I use wp_terms_checklist() which in turn relies on Walker_Category_Checklist which is relatively inflexible. Primarily, it can only output inputs of the form name=post_category[] or tax_input[$taxonomy].

This may relate to issue #28053 - The saving of media modal fields doesn't cater for multiple form fields of the same name named with empty square brackets such as post_category[]. You would expect these to get submitted and handled as an array but only the last field gets submitted.

Would handling that issue perhaps help here?

comment:17 in reply to: ↑ 13 @boonebgorges3 weeks ago

  • Keywords needs-patch added; has-patch dev-feedback needs-testing removed

Thanks for the initial patch, jessepollak. Some thoughts:

  1. I use wp_terms_checklist() which in turn relies on Walker_Category_Checklist which is relatively inflexible. Primarily, it can only output inputs of the form name=post_category[] or tax_input[$taxonomy]. That's a different form than what's used to saved AttachmentCompat's in Backbone, so I have to str_replace to get it in the correct form. I know this is horrible, but I'm not sure what the best way to proceed is — should I rewrite Walker_Category_Checklist, so it can be more modular?

Yeah, definitely feel free to refactor, but it might be awkward in this case, since it looks like you need the $name to contain a reference to the attachment ID. You could make this work with the creative use of a filter, or perhaps by breaking the $name logic into a separate method of Walker_Category_Checklist and then subclassing it so as to override.

  1. wp_terms_checklist() outputs to the buffer, so I use ob_start() etc to get the content which is passed via AJAX. Is this acceptable or — again — should I refactor to make it possible to get it as a string rather than output?

Definitely yes. An 'echo' param would be good.

  1. In the media.view.AttachmentCompat class, I add an if statement in the save method that handles the case where the input is a series of checkboxes (before this, they were always a single input of type=text or type=hidden). Is this acceptable or should I subclass media.view.AttachmentCompat specifically for ones that are taxonomies of some sort.

What you've done in the patch seems OK to me. If this is the only change necessary, let's not overengineer.

  1. I've only been testing this in the Attachment Edit & Create pages and with the category taxonomy. Can anyone think of other objects and/or taxonomies I should be testing on?

I don't think you need to handle other objects with this ticket. For taxonomies, you should handle any taxonomy where 'hierarchical' => true.

Note: See TracTickets for help on using tickets.