WordPress.org

Make WordPress Core

Opened 6 years ago

Last modified 3 months ago

#7463 new enhancement

Display multiple tag titles on tag unions and intersections

Reported by: ionfish Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.8
Component: Taxonomy Keywords: needs-patch
Focuses: template Cc:

Description

Currently, tag unions and intersections only display the title of the first tag in the query, rather than all of them. This patch adds a new template function, multiple_tag_titles, which displays one or more tag titles for tag intersections and unions, adding a final 'and' or 'or' conjunction where appropriate.

Having written this patch, I'm starting to think it's the wrong approach: multiple taxonomy titles for taxonomy unions and intersections would be preferable.

Attachments (4)

7463.diff (3.3 KB) - added by ionfish 6 years ago.
7463.1.diff (3.4 KB) - added by ionfish 5 years ago.
7463.2.diff (3.4 KB) - added by ionfish 5 years ago.
7463.3.diff (3.4 KB) - added by ionfish 5 years ago.

Download all attachments as: .zip

Change History (20)

ionfish6 years ago

comment:1 ryan6 years ago

  • Milestone changed from 2.7 to 2.8

Moving enhancements to 2.8.

comment:2 ionfish5 years ago

Patch is stale; attaching a new one applied against trunk.

ionfish5 years ago

comment:3 ionfish5 years ago

  • Version changed from 2.7 to 2.8

Just tested the patch against latest trunk, seems to still apply ok.

comment:4 Denis-de-Bernardy5 years ago

  • Keywords tested added; needs-feedback removed

_implode_nicely() would work with languages that read from right to left, or?

oh, and @since 2.7 should be @since 2.8 :-)

ionfish5 years ago

comment:5 ionfish5 years ago

Fixed the @since, cheers.

_implode_nicely() should work fine with RTL languages as long as they have list syntax that's similar enough to English (although guidance from someone with more experience in that area would be appreciated). One thing that strikes me now is that it probably shouldn't add spaces automatically, as it prevents one from just using a final comma rather than a connective word.

ionfish5 years ago

comment:6 Denis-de-Bernardy5 years ago

for the commas, see #7897

comment:7 ionfish5 years ago

The commas are run through () in the latest patch. I'm also wondering whether multiple_tag_titles() should be renamed to just tag_titles().

comment:8 Denis-de-Bernardy5 years ago

might be. also of potential issue is the filter's name. it might be that some plugins relied on its old (single) version, if any.

comment:9 ionfish5 years ago

In that case, would it be worth passing the results through both filters and deprecating single_tag_title()?

comment:10 Denis-de-Bernardy5 years ago

  • Keywords dev-feedback added

this is just a guess: for backwards compatibility sake, core devs might think it's preferable to change the behavior of the new function (and pass all of the tags through the single_tag_title filter (or whatever it's called). and if they think like that I'd actually agree with them.

comment:11 follow-up: ryan5 years ago

This adds extra queries per tag, which is why we didn't add this from the beginning. Reducing to one query would be nice. In query.php where we do the single tag query we could call get_terms() with either 'include' or 'slugin' (which doesn't exist yet).

Can we add this to single_tag_title()so themes don't have to update templates?

The i18n on this is ugly, but I don't know if there's a good way to avoid concatenating strings.

$glue = __(', '), $conjuction = __(' and ')

You can't use function return as a default. We usually default to false or null and then assign the default string if false inside the function.

comment:12 in reply to: ↑ 11 Denis-de-Bernardy5 years ago

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

Replying to ryan:

This adds extra queries per tag, which is why we didn't add this from the beginning. Reducing to one query would be nice.

you mean on the get_terms() and get_term_by() calls, right? if so, there's room to bulk grab tags in order to cache the result, ya.

comment:13 Denis-de-Bernardy5 years ago

  • Milestone changed from 2.8 to Future Release

moving to future from lack of refreshed patch

comment:14 ionfish5 years ago

Since it seems like a new patch would be slightly involved, I'll run one up for 2.9.

comment:15 scribu4 years ago

Related: #12891

comment:16 nacin3 months ago

  • Component changed from Template to Taxonomy
  • Focuses template added
Note: See TracTickets for help on using tickets.