WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#30589 closed enhancement (fixed)

Comments navigation template tags

Reported by: obenland Owned by: obenland
Milestone: 4.4 Priority: normal
Severity: normal Version: 4.1
Component: Themes Keywords: has-patch commit
Focuses: template Cc:

Description

We've introduced posts navigation template tags in #29808. Adding equivalent tags for comments would round off the API, giving theme authors tags that work similarly with both posts and comments, with similar output.

Unfortunately we missed the boat to have Twenty Fifteen take advantage of it, but that won't keep future themes from benefitting from the functions.

Attachments (3)

30589.diff (3.3 KB) - added by obenland 5 years ago.
30589.twentyfifteen.diff (2.0 KB) - added by obenland 4 years ago.
Proof-of-concept: Custom code reduction in Twenty Fifteen.
30589.2.diff (3.3 KB) - added by obenland 4 years ago.

Download all attachments as: .zip

Change History (24)

@obenland
5 years ago

#1 @obenland
4 years ago

  • Milestone changed from Future Release to 4.2

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


4 years ago

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


4 years ago

@obenland
4 years ago

Proof-of-concept: Custom code reduction in Twenty Fifteen.

#4 @obenland
4 years ago

@johnbillion asked me to upload a patch to show what it would have saved in Twenty Fifteen.

While this is not a patch that will ever be landing in Twenty Fifteen (it wouldn't be backwards compatible), it certainly will help in Twenty Sixteen and beyond.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


4 years ago

#6 follow-up: @afercia
4 years ago

We quickly discussed this in the Accessibility team meeting yesterday, our concern is about 2 different issues:

1.
the total number of navigation landmarks in a typical page.
This should be raised in a separate ticket but just consider e.g. Twenty Fifteen with one main menu, one social menu, a few widgets in the sidebar... there would be 13-14 landmarks, with nested landmarks and 5 navigation landmarks. See screenshots:
https://cldup.com/n6o4tdWKr3.png
and
https://cldup.com/VzDECLO8uL.png
Just too many landmarks.

2.
The usage of a nav element for comments navigation. There's some debate about this, we're open to discussion but please consider what the specification says:
http://www.w3.org/TR/html5/sections.html#the-nav-element
Note: Not all groups of links on a page need to be in a nav element — the element is primarily intended for sections that consist of major navigation blocks.

What is "major" is matter of discussion of course. Specifically for comments, they're a secondary content by definition. Whether you consider them a "section" or an "aside", they're related to the main content but they're not the main content. Thus, a secondary content having a navigation with a semantic meaning of major doesn't look right to me.
As I said there are different opinions, we're open to discussion and we'd really appreciate everyone's feedback, I'd just like to quote Mr. Bruce Lawson:

You need to ask yourself why you’re marking something up as nav. I do it to help users of assistive technology easily find important navigation, eg site-wide stuff. If you mark up every link that goes somewhere in the site as nav, it’s harder for such users to find the really important stuff.

-- http://html5doctor.com/avoiding-common-html5-mistakes/#comment-16949

#7 in reply to: ↑ 6 ; follow-up: @obenland
4 years ago

Replying to afercia:

We quickly discussed this in the Accessibility team meeting yesterday, our concern is about 2 different issues:

  1. the total number of navigation landmarks in a typical page.

Right, five does feel like a lot. I guess it depends on what theme you're using (as always). A page with one main navigation, one single post navigation, and one comment navigation would be reasonable again. I have no strong feelings there. But keep in mind, that since we add it to the nav widget, there's really no easy way to limit it, even if we omitted it in a template tag.

  1. The usage of a nav element for comments navigation.

Yeah, I'd interpret that a bit more loosely. I think the purpose of that is to avoid having all anchors wrapped in a nav element, because you use anchors to navigate. I like the example with the footer links that follows your spec quote, to illustrate that.

Also: Thank you everyone at a11y for taking the time to review it!

#8 in reply to: ↑ 7 @afercia
4 years ago

Replying to obenland:

Replying to afercia:

We quickly discussed this in the Accessibility team meeting yesterday, our concern is about 2 different issues:

  1. the total number of navigation landmarks in a typical page.

Right, five does feel like a lot.

Thanks very much for your feedback. To clarify (my English!): I meant navigational landmarks:
http://www.w3.org/TR/wai-aria/roles#landmark_roles_header

application
banner
complementary
contentinfo
form
main
navigation
search

they're all navigational landmarks. Screen readers will use them, for example with NVDA you press the key "d" and you can "jump" to the sections marked with those roles. That's what landmarks are meant for, a mechanism to quickly skip over blocks of content in the page. This helps users a lot.

Additionally, some HTML5 elements are mapped to landmark roles, they have an implicit landmark role:
<main> -> role=main
<header> -> role=banner
<nav> -> role=navigation
<aside> -> role=complementary
etc.

Screen readers will grab from the page both elements with a landmark role and elements that have an implicit landmark role. In our example they're 13-14 not just 5. Between them 5 are <nav>s.

Our concern is that the "jump" mechanism provided to users by landmarks is useful just if landmarks are reasonably limited in number and when it's easy to remember that a page has 4-5-6 main landmark sections and what kind of content I can find there.

Having my screen reader announce there are 5 navigation areas, then 5-6 or even more (nested!) complementary areas, 1 main, 1 banner, 1 search, 1 contentinfo etc. doesn't help me :)

As a user, I would just want to know that pressing "d" no more than 5-6 times I can quickly go through the whole page and get the main content or quickly reach the main navigation or the complementary content area.

It depends on the theme used, yes. The screenshots in my previous comment come from Twenty Fifteen, based on _s (for reference, I know you know that :). But having core adding even more landmarks makes this issue harder to solve.

TL;DR sorry, will stop here :) I would strongly recommend to reconsider this issue. We're all here to help users and, if possible, to help developers. But users first.

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


4 years ago

#10 @andy.saint
4 years ago

Would it be possible to add this for Image navigation also?

As for too many nav elements on the page, could you not just add an extra parameter so the developer can determine which tag to use for the container element? Like with the wp_nav_menu function.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


4 years ago

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


4 years ago

#13 @helen
4 years ago

  • Milestone changed from 4.2 to Future Release

It seems that this would be best paired with #31315. I am not sure there's anybody who's up for owning this in 4.2, and as far as I see it's not an urgent matter.

#14 @obenland
4 years ago

  • Milestone changed from Future Release to 4.4

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


4 years ago

#16 @obenland
4 years ago

  • Owner set to obenland
  • Status changed from new to accepted

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


4 years ago

#18 follow-up: @obenland
4 years ago

  • Keywords commit added

@afercia: I'm close to getting this in so we can stop defining custom template tags in themes for that.

How much of a problem is the navigation landmark really, especiialy in light of default themes (and _s-based themes) using the same markup already?

#19 in reply to: ↑ 18 ; follow-up: @afercia
4 years ago

Replying to obenland:

@afercia: I'm close to getting this in so we can stop defining custom template tags in themes for that.

@obenland hi, my personal opinion is still that this is more helpful for theme authors than for final users, as your sentence above implies :) I'd recommend to pair it with #31315 as @helen suggested, because WordPress should never output markup without giving developers the ability to easily change it.

in light of default themes (and _s-based themes) using the same markup already?

it always depends on the content structure and the total number of navigational landmarks in a page so there's not an ultimate answer. Using one more <nav> is not a problem per se but it may be, especially if developers are going to use it thinking that it's always right "because WordPress does it this way" :)

#20 in reply to: ↑ 19 @obenland
4 years ago

Replying to afercia:

I'd recommend to pair it with #31315 […], because WordPress should never output markup without giving developers the ability to easily change it.

That is the plan, see comment:15:ticket:31315.

@obenland
4 years ago

#21 @obenland
4 years ago

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

In 34367:

Introduce template functions for comment navigation:

  • get_the_comments_navigation() and the_comments_navigation() for navigation to the next and previous page of comments.
  • get_the_comments_pagination() and the_comments_pagination() for paginated navigation between pages of comments. Uses paginate_comments_links().

This reduces the need for themes to define their own sets of comment navigation
functions, like Twenty Fifteen and others.
Completes the navigation template tag API started in #29808 for posts.

Fixes #30589.

Note: See TracTickets for help on using tickets.