Twenty Eleven: fix "Older Posts" label for archive pages with respect to order#9135
Twenty Eleven: fix "Older Posts" label for archive pages with respect to order#9135shrivastavanolo wants to merge 12 commits intoWordPress:trunkfrom
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
| <h3 class="assistive-text"><?php _e( 'Post navigation', 'twentyeleven' ); ?></h3> | ||
| <div class="nav-previous"><?php next_posts_link( __( '<span class="meta-nav">←</span> Older posts', 'twentyeleven' ) ); ?></div> | ||
| <div class="nav-next"><?php previous_posts_link( __( 'Newer posts <span class="meta-nav">→</span>', 'twentyeleven' ) ); ?></div> | ||
| <div class="<?php echo esc_attr( $is_desc ? 'nav-previous' : 'nav-next' ); ?>"><?php next_posts_link( $is_desc ? $old_posts_text : $new_posts_text ); ?></div> |
There was a problem hiding this comment.
Instead of doing two consecutive checks, that are essentially the same, why not combine everything into one check? This applies for 2011, 2012 and 2013 that are very similar in this regard.
There was a problem hiding this comment.
@SirLouen Hey, I have made the suggested changes in the twentythirteen PR could you have a look please. I'm not able to tag you there for some reason.
ba1861e to
a64b13a
Compare
…develop into fix/archive-page-nav-labels-twentyeleven
SirLouen
left a comment
There was a problem hiding this comment.
I have not reviewed in detail, but I'm just wondering, can't we follow the same strategy as in 2011?
|
Hey @SirLouen! I initially implemented the code you shared since it did help simplify the logic. However, I later realized that while the output visually looked correct, it did not preserve the DOM order of Ref: <?php if ( get_query_var( 'order', 'DESC' ) === 'DESC' ) : ?>
<div class="nav-previous"><?php next_posts_link( $left_arrow . $old_posts_text ); ?></div>
<div class="nav-next"><?php previous_posts_link( $new_posts_text . $right_arrow ); ?></div>
<?php else : ?>
<div class="nav-next"><?php previous_posts_link( $old_posts_text . $right_arrow ); ?></div>
<div class="nav-previous"><?php next_posts_link( $left_arrow . $new_posts_text ); ?></div>
<?php endif; ?>This affects the meaningful sequence and focus order requirements as described in the WCAG 2.2 guidelines on Meaningful Sequence and Focus Order, which are important for accessibility. So I had to revert that commit. I am currently looking for ways to keep the code simple while ensuring the DOM order remains consistent and accessible. Let me know if you have any thoughts on how we can achieve both. Thanks again for the review! |
Ping me out if you figure out something without making the code too repetitive. |
There was a problem hiding this comment.
Again, the patch looks good. I suggested a few minor edits and an unrelated correction for the HTML comment.
The navigation appears the same as it did before the patch, regardless of order. The screenshots show the theme's :focus underline for the first link in the bottom page navigation (according to DOM order). Note that the top navigation does not appear on the first page.
I also checked French and Hebrew.
In default descending order, the markup has different spacing between elements, and wp_kses_post() removes the space after the href values. Otherwise, it is the same as before the patch in both the top and bottom areas. (I updated diffs after 0b4776d)
<nav id="nav-above">
<h3 class="assistive-text">Post navigation</h3>
- <div class="nav-previous"><a href="http://localhost/svn/src/tag/alice/page/3/" ><span class="meta-nav">←</span> Older posts</a></div>
- <div class="nav-next"><a href="http://localhost/svn/src/tag/alice/" >Newer posts <span class="meta-nav">→</span></a></div>
- </nav><!-- #nav-above -->
+ <div class="nav-previous"><a href="http://localhost/svn/src/tag/alice/page/3/"><span class="meta-nav">←</span> Older posts</a></div>
+
+ <div class="nav-next"><a href="http://localhost/svn/src/tag/alice/">Newer posts <span class="meta-nav">→</span></a></div>
+ </nav><!-- #nav-above --> <nav id="nav-below">
<h3 class="assistive-text">Post navigation</h3>
- <div class="nav-previous"><a href="http://localhost/svn/src/tag/alice/page/3/" ><span class="meta-nav">←</span> Older posts</a></div>
- <div class="nav-next"><a href="http://localhost/svn/src/tag/alice/" >Newer posts <span class="meta-nav">→</span></a></div>
- </nav><!-- #nav-above -->
+ <div class="nav-previous"><a href="http://localhost/svn/src/tag/alice/page/3/"><span class="meta-nav">←</span> Older posts</a></div>
+
+ <div class="nav-next"><a href="http://localhost/svn/src/tag/alice/">Newer posts <span class="meta-nav">→</span></a></div>
+ </nav><!-- #nav-below -->With ?order=ASC, the links' href attributes switch, without changing the appearance. These diff views highlight the link changes on the ASC page, before and after applying the patch, within the markup:
<nav id="nav-above">
<h3 class="assistive-text">Post navigation</h3>
- <div class="nav-previous"><a href="http://localhost/svn/src/tag/alice/page/3/?order=ASC" ><span class="meta-nav">←</span> Older posts</a></div>
- <div class="nav-next"><a href="http://localhost/svn/src/tag/alice/?order=ASC" >Newer posts <span class="meta-nav">→</span></a></div>
- </nav><!-- #nav-above -->
+ <div class="nav-previous"><a href="http://localhost/svn/src/tag/alice/?order=ASC"><span class="meta-nav">←</span> Older posts</a></div>
+
+ <div class="nav-next"><a href="http://localhost/svn/src/tag/alice/page/3/?order=ASC">Newer posts <span class="meta-nav">→</span></a></div>
+ </nav><!-- #nav-above --> <nav id="nav-below">
<h3 class="assistive-text">Post navigation</h3>
- <div class="nav-previous"><a href="http://localhost/svn/src/tag/alice/page/3/?order=ASC" ><span class="meta-nav">←</span> Older posts</a></div>
- <div class="nav-next"><a href="http://localhost/svn/src/tag/alice/?order=ASC" >Newer posts <span class="meta-nav">→</span></a></div>
- </nav><!-- #nav-above -->
+ <div class="nav-previous"><a href="http://localhost/svn/src/tag/alice/?order=ASC"><span class="meta-nav">←</span> Older posts</a></div>
+
+ <div class="nav-next"><a href="http://localhost/svn/src/tag/alice/page/3/?order=ASC">Newer posts <span class="meta-nav">→</span></a></div>
+ </nav><!-- #nav-below -->Full-page screenshots show three posts, with different published dates (matching the chapter number to the day of the month).
- Before patch, default order
- Before patch, ascending order
- With patch, default order
- With patch, ascending order
Twenty Eleven does not have any adjustment for links on small screens.
I ran each translation through wp_kses_post() to make sure they all display properly.
Co-authored-by: Stephen A. Bernhardt <sabernhardt@yahoo.com>
Co-authored-by: Stephen A. Bernhardt <sabernhardt@yahoo.com>
Co-authored-by: Stephen A. Bernhardt <sabernhardt@yahoo.com>
|
Similar to PR, PR
Fixes the issue: "Older Entries" and "Newer Entries" links are wrong when entries displayed in ascending order for the Twenty Eleven theme by adding dynamic labels according to the order query var (DESC or ASC). More info in trac ticket.
Trac ticket: https://core.trac.wordpress.org/ticket/10219
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.