Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wp_link_pages should not be inside entry-content #1294

Open
dshanske opened this issue May 21, 2018 · 10 comments · May be fixed by #1304
Open

wp_link_pages should not be inside entry-content #1294

dshanske opened this issue May 21, 2018 · 10 comments · May be fixed by #1304

Comments

@dshanske
Copy link

wp_link_pages should not be inside entry-content, as this is not content.

@grappler
Copy link
Collaborator

This has been so since the initial commit.

@dshanske Where would you place wp_link_pages()?

@dshanske
Copy link
Author

In the entry footer function is my suggestion. It is not content, it is navigation. I know it has been where it is for a long time, but it is still not content.

@dshanske
Copy link
Author

The length of time it has been there is why I started the issue to discuss.

@grappler
Copy link
Collaborator

I know it has been where it is for a long time, but it is still not content.

Sorry I was not verbose enough. 😞 I wanted to document the fact in the issue so that future me would could just read the comment without having to find the commit again. As it was part of the initial commit means that there are no issues or pull requests with reasoning why it should be added within entry-content.

I have been looking for examples in the wild that place wp_link_pages() differently but it seems like everyone is copying one of the earlier implementations.

I have been playing around with the idea and I am not fully convinced. I see wp_link_pages() as similar to the dots under a slider to switch to a specific slider. The page numbers are specific to the content.

I don't see wp_link_pages() as the same as the_post_navigation().

@miklb
Copy link

miklb commented May 23, 2018

My 2¢ is that wp_link_pages() is a <nav> element. In general I wouldn't write:

<article>
<p>content</p>
<nav>
stuff
</nav>
</article>

If not a nav then at minimum it would be a footer of the article which still wouldn't put it in the entry-content div.

EDIT I'm making the assumption of a nav element based on the def of wp_link_pages "Displays page links for paginated posts"

@Ruxton
Copy link

Ruxton commented May 23, 2018

Everyone puts it in the content, because it's always been there but from a structured data perspective, it arguably belongs outside the content block. I'd argue based purely on semantics it belongs in the footer, as a nav element.

also, wp_link_pages should cause <link rel="next" and <link rel="prev" to get added to head with appropriate pages.

@peiche
Copy link

peiche commented Jun 28, 2018

@grappler I don't know if linking to other starter themes is a no-no (if it is I apologize), but Sage 9 puts wp_link_pages outside the content.

<div class="entry-content">
  @php the_content() @endphp
</div>
<footer>
  {!! wp_link_pages(['echo' => 0, 'before' => '<nav class="page-nav"><p>' . __('Pages:', 'sage'), 'after' => '</p></nav>']) !!}
</footer>

grappler added a commit to grappler/_s that referenced this issue Jul 2, 2018
@grappler grappler linked a pull request Jul 2, 2018 that will close this issue
@grappler
Copy link
Collaborator

grappler commented Jul 2, 2018

Thank you @peiche for the link to Sage 9. It is interesting to see how other people do it.

I have created a PR #1304 to fix this. What do you think?

@justintadlock
Copy link

Not that I use this particular theme, but I thought I'd chime in here since I came across this topic.

wp_link_pages() may not be "content" in and of itself, but it's intricately tied to the actual content. It outputs the pagination of the post content. A better name for the function would've been post_content_pagination().

The <!--nextpage--> quicktag triggers this, which is retrieved from $post->post_content.

Depending on the theme design, where to place the call is going to change. I don't see enough reason to break from convention here.

@miklb
Copy link

miklb commented Jul 11, 2018

that's what the article footer is for. Semantics > convention

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants