WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 14 months ago

Last modified 11 months ago

#13578 closed defect (bug) (fixed)

wp_link_pages isn't fully functional and doesn't include require style information

Reported by: brianlayman Owned by: SergeyBiryukov
Milestone: 3.6 Priority: normal
Severity: normal Version: 2.9.2
Component: Themes Keywords: has-patch 3.6-early needs-codex
Focuses: Cc:

Description

Since paging within a post was dropped as a quicktag, wp_link_pages hasn't been kept up to date with the new themes and has had some major flaws since it was first created. 3.0 has a number of enhancements to this function already, but it still has several flaws. These changes will allow theme authors to use this built in function and have to not overwrite it their themes.

The attached patch is already deployed at b5media. And makes the following changes:
*It allows you to specify the separator for numbers.
*The seperator is now only use between two numbers instead of always being appended to the end of a number.
*The function no longer produces an empty anchor tag when next or prev is not to be displayed. Up to now it left the link in but didn't include any inner text.
*Adds the css classes pag_nav_num, pag_nav_first, pag_nav_current, pag_nav_prev, pag_nav_next & pag_nav_last allowing style specifications for all output conditions

I think this should give everyone the freedom to do whatever they want with this function now while being mostly backwards compatible. I do drop the empty links, but I can't see how anyone would have styled specifically for them any way.

Attachments (5)

post_template-wp_link_pages.diff (7.5 KB) - added by brianlayman 4 years ago.
Code and comment changes to fix the functionality of wp_link_pages
wp_link_pages.diff (4.8 KB) - added by kobenland 2 years ago.
Enhancements of wp_link_pages()
13578.diff (4.7 KB) - added by obenland 15 months ago.
13578.2.diff (4.8 KB) - added by SergeyBiryukov 14 months ago.
13578.3.diff (4.8 KB) - added by SergeyBiryukov 14 months ago.

Download all attachments as: .zip

Change History (35)

brianlayman4 years ago

Code and comment changes to fix the functionality of wp_link_pages

comment:1 nacin3 years ago

  • Keywords has-patch added; theme wp_linkpages css removed
  • Milestone changed from Awaiting Review to Future Release

comment:2 chipbennett3 years ago

  • Cc chip@… added

comment:3 anointed2 years ago

  • Cc anointed added

comment:4 kobenland2 years ago

  • Cc obenland@… added

comment:5 kobenland2 years ago

  • Keywords dev-feedback added

Hi there,

today I had a conversation with Edward Caissie and Chip Bennett over at the theme-review mailing list about wp_link_pages() and how to improve the markup rendered or returned by this function.

I prepared a proposal on how we could make wp_link_pages() way more accessible for theme authors just by the addition of some filters and Brian's idea to make the separator overridable. It would be awesome if a member of the dev community could give me feedback on the attached diff. Basically I just added the 'separator' parameter, cleaned up some code and made the links as well as the entire output filterable. This would probably make this ticket a 'proposal' but I don't feel comfortable (yet) to mess around with the properties here too much.

BTW: When I tested the function I ran into issues with the $more variable, as it was always NULL and prevented the previous/next output in my install. It works just fine when I take out all instances of $more - has anyone else had the same issue?

kobenland2 years ago

Enhancements of wp_link_pages()

comment:6 scribu2 years ago

The 'wp_link_pages_link' filter should be moved into the proposed get_multipage_link() function: #16973

comment:7 kobenland2 years ago

Sure. May I create another patch upon your last patch there, or would you rather do it?

comment:8 scribu2 years ago

Go nuts. :)

comment:9 brianlayman2 years ago

I'd like to re-emphasize the point of this ticket as well. 2.9.2 was out when this was written and I was working with three different themes that each had their own implementation of the link pages code because the existing code in WordPress didn't produce code that could be themed, but it did allow complete replacement. Adding the separator specification was a nice extra but the real point of this patch was to get the css class specifications in there that would eliminate the need for the alternate implementations. If WP produced code that allowed you to full theme each part of the links, there would be a whole lot of themes that could be several big functions lighter.

comment:10 ardathksheyna22 months ago

  • Cc ardathksheyna added

comment:11 archon81022 months ago

#21001 and #21002 were marked as duplicates of this bug, though #21002 contains a new feature request that's not part of this bug.

@brianlayman Do you think #21002 is useful enough to warrant a patch from you or another contributor and add to the scope of this ticket or should it remain separate and re-opened?

Any chance of these patches making it to the next major version of WP?

comment:12 archon81022 months ago

  • Cc admin@… added

comment:13 brianlayman22 months ago

I definitely agree that the current page should be stylable. That's the point of 21002 right? I'll put in an updated version of my original patch. I took a good bit of time to test that patch and had it running in the field. We've had two more years of themes going out there having build their own numbering functions. I was hoping to avoid that. I hope it goes in with 3.5.

comment:14 follow-up: chipbennett17 months ago

  • Version changed from 3.0 to 2.9.2

Changed version to 2.9.2, as it appears the ticket was submitted prior to 3.0 release?

Also: is there any particular reason not to act on this ticket? This is a much-needed patch for Theme developers.

comment:15 Rarst17 months ago

To enumerate my fresh experience with this function (I was trying to achieve functionality of pagination component in Twitter Bootstrap):

  • no way to add elements outside of link
  • no way to retrieve links as HTML list
  • no output filter

In a nutshell markup is impossible to properly customize and what markup you get is hard to style.

Given that this is on the list of required functions to be used in theme per guidelines - it's in serious need of improvement.

comment:16 follow-up: scribu17 months ago

Under these circumstances, I think requiring themes to use this function is a mistake. The guidelines should mandate that multi-page functionality has to exposed in the theme, not how it should be implemented.

Version 0, edited 17 months ago by scribu (next)

comment:17 in reply to: ↑ 16 chipbennett17 months ago

Replying to scribu:

Under these circumstances, I think requiring themes to use this function is a mistake. The guidelines should mandate that multi-page functionality has to be exposed in the theme, not how it should be implemented.

That is something to be discussed on the Theme-Reviewers mail-list, or on the Make/Themes site, rather than in this Trac ticket. But that change goes against a fundamental principle of the Theme Review guidelines: that Themes support core implementation of features.

Thus, I would rather the Theme Review Team push for improving the core implementation, rather than saying that it is acceptable for Themes to roll their own, just because the core implementation isn't perfect.

comment:18 in reply to: ↑ 14 helenyhou17 months ago

  • Keywords 3.6-early added

Replying to chipbennett:

Also: is there any particular reason not to act on this ticket?

Not really, besides that bringing it up right this moment is probably just about the worst time you could pick (attempting to wrap up 3.5 and all that). This seems like a sane thing to do (as a concept, have not looked at patch). Will mark it for early in the hopes that somebody actually manages to go through that list, but if not, please please bring it up again after allowing for a little post-release break, probably after the holidays. :)

comment:19 Rarst16 months ago

  • Cc contact@… added

comment:20 toscho15 months ago

  • Cc info@… added

comment:21 F J Kaiser15 months ago

  • Cc 24-7@… added

comment:22 travisnorthcutt15 months ago

  • Cc travis@… added

comment:23 helen15 months ago

  • Keywords needs-testing added
  • Milestone changed from Future Release to 3.6

Patch still applies, although you have to specify the file. It looks okay to me, just some nitpicky code standard things, but somebody who uses this function regularly and finds the need for this type of control should probably actually test it and the added filters :)

obenland15 months ago

comment:24 obenland15 months ago

Refreshed patch

comment:25 lancewillett14 months ago

  • Cc lancewillett added

comment:26 goto1014 months ago

  • Cc dromsey@… added

comment:27 Rarst14 months ago

+1 for patch. New filters allow to wrap individual links as well as overwrite output completely, which gives freedom to do whatever with it.

comment:28 alex-ye14 months ago

  • Cc nashwan.doaqan@… added

SergeyBiryukov14 months ago

SergeyBiryukov14 months ago

comment:29 SergeyBiryukov14 months ago

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

In 23653:

Make wp_link_pages() filterable. Add 'separator' argument. Simplify the function logic. props obenland, brianlayman. fixes #13578.

comment:30 DrewAPicture11 months ago

  • Keywords needs-codex added; dev-feedback needs-testing removed

New filter and new args in [23653]

Note: See TracTickets for help on using tickets.