WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#15378 closed enhancement (fixed)

Post formats should have built-in index pages

Reported by: nacin Owned by:
Milestone: 3.1 Priority: normal
Severity: normal Version: 3.1
Component: Themes Keywords:
Focuses: Cc:

Description

We currently set rewrite to false.

If we turn rewriting on, it enables themes to create an index of, say, videos. We can also put that rewriting on a filter, if we wanted.

They can already leverage the taxonomy-$taxonomy-{$term->slug}.php template.

Leaving this out of the 3.1 milestone for now, but I've already gotten a few questions on this from some theme people.

Patch incoming.

Attachments (3)

15378.diff (1.0 KB) - added by nacin 3 years ago.
15378.2.diff (4.9 KB) - added by nacin 3 years ago.
Patch doesn't handle flushing rewrite rules on theme switch (#14849). Uses a filter for the base, instead of a UI option. Also, the array_flip and such makes for dense logic. But it does handle i18n slug situations both generating and handling links.
15378.3.diff (6.8 KB) - added by nacin 3 years ago.
Includes rudimentary UI support.

Download all attachments as: .zip

Change History (28)

nacin3 years ago

comment:1 nacin3 years ago

  • Keywords has-patch added

comment:2 nacin3 years ago

Note that with_front = true. That makes sense to me, at least...

comment:3 nacin3 years ago

Whoops, we'll need a query var there. Also, even though we're not doing conditional registration of the taxonomy, we should be only rewriting if current_theme_supports.

That said, maybe we should conditionally register the taxonomy? We didn't for nav menus but that's because basically every theme supports widgets.

comment:4 nacin3 years ago

  • Milestone changed from Awaiting Review to 3.1

Moving for consideration, per some discussion.

comment:5 mikeschinkel3 years ago

  • Cc mikeschinkel@… added

comment:6 sbressler3 years ago

  • Cc sbressler@… added

comment:7 voyagerfan57613 years ago

  • Cc WordPress@… added

comment:8 mfields3 years ago

  • Cc michael@… added

I think that this is a great idea.

comment:9 Otto423 years ago

+1. Having a nice way to display all "gallery" formatted posts would be useful.

comment:10 follow-up: nacin3 years ago

Here's what I'm thinking:

query var: two options, format, or the more conservative post_format. Not really a big deal as this would be hidden when pretty permalinks are enabled.

rewrite slug: obvious one would be format. That said:

I think I'd like to place a filter specifically on the rewrite slug. If that sounds unorthodox, keep in mind we have UI options on the permalinks page for category/tag base. *That* said, I'd actually entertain the idea of adding a UI option for format too though, assuming the active theme supports it. I know we shouldn't add options, but in this case, format is particularly ambiguous (more so than tag and category) and artificial, and I certainly see people wanting to change it to something that fits their style.

If that's backed by the rest of the core team, I'll code it up.

comment:11 in reply to: ↑ 10 ; follow-up: markmcwilliams3 years ago

You might find the obvious one to be format but in URL format, I *personally* really wouldn't want to visit somesite.com/format/aside(s)/ as I don't think it sounds right. I'd probably rather go for somesite.com/type/aside(s)/ but thats going to introduce even more terminology! :(

On the other hand I'd rather it be somesite.com/aside(s)/, somesite.com/galler(y/ies)/ or somesites.com/audio(s)/ as they all just make more sense to me, as for others who knows ... it's a personal opinion! Thoughts?

comment:12 in reply to: ↑ 11 filosofo3 years ago

Replying to markmcwilliams:

On the other hand I'd rather it be somesite.com/aside(s)/, somesite.com/galler(y/ies)/ or somesites.com/audio(s)/ as they all just make more sense to me, as for others who knows ... it's a personal opinion! Thoughts?

+1 to that. Clients are always asking to remove the category prefix as it is, and is seems like most of the things out there providing, e.g., gallery functionality do it under /gallery/ and the like.

Also, it makes sense to hide the format from "pretty" permalinks, because format is a private notion (in every relevant sense), and "pretty" permalinks are in the realm of public, human-readable notions.

comment:13 follow-up: nacin3 years ago

So, been thinking about this -- I like the idea of being able to omit the prefix and then actually using /gallery/ etc. This is actually exactly what Matt plans to do on ma.tt and I agree.

Because it's a set number of formats, adding the rewrite rules is not a burden.

However, this opens up a UX issue. If we offer an option, then leaving it blank will be different functionality from leaving the tag or cat base blank. Thoughts?

I *don't* think we can do this rewriting without the base by default.

comment:14 follow-up: Otto423 years ago

The wildcard base URLs are reserved for Pages. I don't think we should pre-empt them by default.

Really you're looking at two different things here:
a) Having format index pages (/format/aside).
b) Allowing taxonomy based URLs to have no base.

Two different things, and I'd separate them into separate tickets. Just make this do /format/whatever for now, come back and think about no-bases for later.

comment:15 in reply to: ↑ 13 Otto423 years ago

Replying to nacin:

So, been thinking about this -- I like the idea of being able to omit the prefix and then actually using /gallery/ etc. This is actually exactly what Matt plans to do on ma.tt and I agree.

Is doing now. Although that's implemented with a custom Page Template and an advanced taxonomy query, to get both the old gallery category as well as the new gallery post_formats.

nacin3 years ago

Patch doesn't handle flushing rewrite rules on theme switch (#14849). Uses a filter for the base, instead of a UI option. Also, the array_flip and such makes for dense logic. But it does handle i18n slug situations both generating and handling links.

nacin3 years ago

Includes rudimentary UI support.

comment:16 nacin3 years ago

(In [16589]) Only link to the Gallery category if the post is in the category. (It may be a format.) see #15506, see #15378.

comment:17 in reply to: ↑ 14 gazouteast3 years ago

  • Cc gazouteast added

Replying to Otto42:

The wildcard base URLs are reserved for Pages. I don't think we should pre-empt them by default.

Really you're looking at two different things here:
a) Having format index pages (/format/aside).
b) Allowing taxonomy based URLs to have no base.

Two different things, and I'd separate them into separate tickets. Just make this do /format/whatever for now, come back and think about no-bases for later.

Otto - very surprised to be reading this from you - forward-looking, the problem will be even worse than the one caused by capital_P_dangit. You're opening up a 404-avalanche risk.

If 3.1 rolls with the /format/ layer embedded in the permalinks, then 3 months later 3.1.1 rolls with it removed (or optional) - how many millions of pages on WordPress based sites are suddenly going to have masses of 404s reporting in Google analytics, and get de-indexed? Not to mention breaking of internal and external links to pages and taxonomies in a "we had it, now we don't" game of hide and seek regarding the /formats/ layer.

I'm very surprised @greenshady hasn't weighed in on this point yet - this type of problem projection is usually his forte.

Gaz

comment:18 nacin3 years ago

(In [16705]) Post format indexes. First pass. see #15378.

comment:19 nacin3 years ago

Okay. Indexes are in. Notes:

  • Needs some testing, but these are L10n safe.
  • get_post_format_slugs() and how it gets used (one time it gets flipped) feels odd. Feel free to improve. Also looks like I missed phpdoc on that.
  • I hooked into request, but now I think we should target pre_get_post instead. That would allow for any query to use post_format=gallery, rather than post_format=post-format-gallery. Maybe even lower into WP_Query too.
  • Please triple-check my logic. Tested well, but I might have missed something, and there might be room for improvement in some spots.
  • We need to update Twenty Ten at some point soon.

comment:20 greenshady3 years ago

Error with _post_format_link() when using the default permalink structure:

Warning: str_replace() expects at least 3 parameters, 2 given in C:\xampplite\htdocs\wp-includes\post.php on line 5173

This code is used post.php:

return add_query_arg( 'format', str_replace( 'post-format-', $term->slug ), $link );

str_replace() requires three parameters:

return add_query_arg( 'format', str_replace( 'post-format-', '', $term->slug ), $link );

Changing the code to that changes the term links to: http://localhost/?post_format=post-format-aside&format=aside. I'm not sure if that's the desired result, so I didn't want to upload a patch yet.

comment:21 nacin3 years ago

(In [16837]) Use correct query var. see #15378.

comment:22 nacin3 years ago

  • Keywords needs-testing added; has-patch removed

comment:23 anointed3 years ago

  • Cc anointed added

+1 on this. Please consider for 3.1 so we're not left hanging when designing custom themes.

comment:24 ryan3 years ago

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

comment:25 markmcwilliams3 years ago

  • Keywords needs-testing removed

No more testing needed if it's fixed? :P

Note: See TracTickets for help on using tickets.