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

Consider adding H2 Heading on single posts or pages when comments are enabled #43203

Closed
annezazu opened this issue Aug 12, 2022 · 20 comments
Closed
Labels
[Block] Comments Affects the Comments Block - formerly known as Comments Query Loop [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Enhancement A suggestion for improvement.

Comments

@annezazu
Copy link
Contributor

What problem does this address?

Pulling out of this trac issue on adding an "accessibility ready" tag for the Twenty Twenty-Two Theme. From @carolinan who did the audit:

On single posts or pages, when comments are enabled, the site title is an H1, the post title is an H1, but the next title is the "Leave a Reply" title and it is an H3, so the H2 level is skipped.
If there are comments, the "One response to “title” is present, but it is also an H3:
H1 Site Title
H1 Sample Page
H3 One response to “Sample Page”
H3 Leave a Reply

What is your proposed solution?

From @carolinan in that ticket:

There are a few different ways to solve this, but I think that the best option, that would work for both older versions of WP and 6.1, is to add a new heading above the comments area.

Further from DM with @joedolson:

Structurally, it would be better to have an additional heading, because of what this structure looks like once there are comments present. With comments, there's an additional heading "One response to "{post title}"", which is also an H3. That heading should be an h2, and should be present whether there are comments or not, so it provides structural context.

cc @SantosGuillamot @ockham do you have thoughts here, considering your work with the Comments block? This isn't a severe, deal breaking issue but jumping from H1 to H3 is generally discouraged.

@annezazu annezazu added [Type] Enhancement A suggestion for improvement. [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Block] Comments Affects the Comments Block - formerly known as Comments Query Loop labels Aug 12, 2022
@annezazu annezazu changed the title Consider adding H3 Heading on single posts or pages when comments are enabled Consider adding H2 Heading on single posts or pages when comments are enabled Aug 12, 2022
@ockham
Copy link
Contributor

ockham commented Aug 14, 2022

IIRC this was discussed on another PR where @c4rl0sbr4v0 had a bit more context.

@cbravobernal
Copy link
Contributor

I will take a look at it, as it was mentioned on this PR by @justintadlock .

Also, on that PR, we set H2 as the default one: https://github.com/WordPress/gutenberg/pull/40419/files#diff-f1c094255127dfb8013473531eabaa07f7cd0029bc4bdcb90db3b7745d9c2e25

I also checked Core code, and it puts H2 as a default:
https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/blocks/comments-title.php

@ockham
Copy link
Contributor

ockham commented Aug 16, 2022

I will take a look at it, as it was mentioned on this PR by @justintadlock .

Ah, that's exactly what I was thinking of (but couldn't remember where to find it).

Also, on that PR, we set H2 as the default one: https://github.com/WordPress/gutenberg/pull/40419/files#diff-f1c094255127dfb8013473531eabaa07f7cd0029bc4bdcb90db3b7745d9c2e25

I also checked Core code, and it puts H2 as a default: https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/blocks/comments-title.php

Ah, indeed! Thanks Carlos!

I feel we need to provide a bit more context:

Twenty Twenty-Two uses the Post Comments block to render comments. However, per WP 6.0, we've deprecated that block in favor of the Comments Query Loop block (which we've since renamed to (just) Comments). And in that block, the One response to “Sample Page” header is indeed an H2!

I actually just had an idea or two on how we might resolve the issue; I'll try it out and will post an update in a bit!

@ockham
Copy link
Contributor

ockham commented Aug 16, 2022

Okay, I think there might be a few different solutions. I'll provide a bit more context for starters (as it helps inform those solutions):

The Post Comments block is really just a thin wrapper around comments_template. Since we’re calling it without arguments, it uses Core's theme-compat comments.php template to render comments — which happens to have a H3 hardcoded for the "One response to…" heading. (I'll add that the theme-compat comments.php has a deprecation note which I hadn't previously noticed.)


OTOH, in more recent versions of Gutenberg, we've actually merged the Post Comments block into the Comments block, and added a banner in the editor on top of the legacy Post Comments block that allows users to convert it to Comments with one click:

Bildschirmfoto 2022-08-16 um 17 15 56

(The copy is a bit verbose BTW -- my bad. Maybe @annezazu @carolinan have some suggestions?)

This works well enough when the user loads a template with an existing Post Comments block in the editor -- they'll see the prompt which will allow them to easily convert the block to a Comments block (which comes with the desired H2). But of course, no blocks are converted if the user doesn't edit the containing template in the Site Editor.


So I think the optimal solution would be if TT2 could be updated to use the Comments block (instead of the Post Comments block): This would turn the "One response to…" heading into an H2 and provide better UX for handling comments in the Site Editor. However, I don't think that's possible, since the Comments (Query Loop) block was only part of WP 6.0, whereas TT2 is also available on sites running older versions of WP. And while it's obviously possible to have conditional logic in PHP themes, I don't think we can have a block theme that conditionally uses different blocks depending on their availability in different WP versions (correct me if I'm wrong).

So instead, we can tackle the problem at the (PHP) comments_template() level (as called by the Post Comments block). This would likely mean including a copy of Core's theme-compat comments.php (with the slight modification to use an H2 rather than an H3 for "One response to...") with the TT2 theme. I don't love the idea of including more PHP code with a block template, but it's probably the most effective way of fixing the issue.

(Fortunately, we won't have to do anything like that for newer themes, now that the Comments (Query Loop) block is also available.)

Curious to hear y'all's thoughts on this.

@ockham
Copy link
Contributor

ockham commented Aug 16, 2022

This would likely mean including a copy of Core's theme-compat comments.php (with the slight modification to use an H2 rather than an H3 for "One response to...") with the TT2 theme.

I just verified that this works, i.e. in wordpress-develop, run

cp ./src/wp-includes/theme-compat/comments.php ./src/wp-content/themes/twentytwentytwo/
sed -i '' 's/h3/h2/g' ./src/wp-content/themes/twentytwentytwo/comments.php

@ockham
Copy link
Contributor

ockham commented Aug 16, 2022

Hmm, I just realized that this doesn't really solve the a11y issue when there are no comments though, as there will be only the "Leave a reply" H3 -- right after the post title H1 😕

@annezazu
Copy link
Contributor Author

(The copy is a bit verbose BTW -- my bad. Maybe @annezazu @carolinan have some suggestions?)

Hah it is! Perhaps this?

Comments block: You’re currently using the legacy version. What you see is a placeholder with the styling based on the current theme. For the latest updates, switch the block to its editable mode.

As for this broader issue, who else can we involve to perhaps move this work forward/consider more ideas? I'll tag in @gziolo for now in case he has any thoughts. While this isn't necessarily a blocker for the theme being accessibility ready, it feels important to get right.

@ockham
Copy link
Contributor

ockham commented Aug 23, 2022

(The copy is a bit verbose BTW -- my bad. Maybe @annezazu @carolinan have some suggestions?)

Hah it is! Perhaps this?

Comments block: You’re currently using the legacy version. What you see is a placeholder with the styling based on the current theme. For the latest updates, switch the block to its editable mode.

Much better 👍 Thanks a lot. I'll file a PR with this change (maybe with a slight tweak to the "latest updates" part, since as a user, I wouldn't find that the most compelling reason to click that button 😄).

As for this broader issue, who else can we involve to perhaps move this work forward/consider more ideas? I'll tag in @gziolo for now in case he has any thoughts. While this isn't necessarily a blocker for the theme being accessibility ready, it feels important to get right.

I was also thinking about this a bit more over the past week. I think I'd been caught up a bit too much with the feature parity question (getting the legacy block to make the "One response to..." heading an H2, as it already is for the editable version). While that's part of the puzzle, it doesn't solve the problem if there are no comments.

To solve this larger problem, maybe it's best to look how other themes have been addressing this? (Specifically Core's Twenties themes -- assuming that at least the more recent ones are a11y compliant.)

TT1 uses H2 for both "1 comment" and "Leave a comment":

image

Same for Twenty Twenty:

image

Looks like an elegant enough solution for me that also works if there are no comments 😄

So how about we follow this pattern for TT2?

@annezazu
Copy link
Contributor Author

Adjust away! Thanks for being open to feedback on the message either way. I think this makes a ton of sense to follow the same pattern as other themes.

@ockham
Copy link
Contributor

ockham commented Aug 24, 2022

Okay, so it's possible to implement this suggestion by using my earlier solution and tweaking it a bit. The following is a patch against WordPress:

diff --git a/wp-content/themes/twentytwentytwo/comments.php b/wp-content/themes/twentytwentytwo/comments.php
new file mode 100644
index 0000000000..b11dd8b3fe
--- /dev/null
+++ b/wp-content/themes/twentytwentytwo/comments.php
@@ -0,0 +1,66 @@
+<?php
+// Do not delete these lines.
+if ( ! empty( $_SERVER['SCRIPT_FILENAME'] ) && 'comments.php' === basename( $_SERVER['SCRIPT_FILENAME'] ) ) {
+	die( 'Please do not load this page directly. Thanks!' );
+}
+
+if ( post_password_required() ) { ?>
+		<p class="nocomments"><?php _e( 'This post is password protected. Enter the password to view comments.' ); ?></p>
+	<?php
+	return;
+}
+?>
+
+<!-- You can start editing here. -->
+
+<?php if ( have_comments() ) : ?>
+	<h2 id="comments">
+		<?php
+		if ( 1 == get_comments_number() ) {
+			printf(
+				/* translators: %s: Post title. */
+				__( 'One response to %s' ),
+				'&#8220;' . get_the_title() . '&#8221;'
+			);
+		} else {
+			printf(
+				/* translators: 1: Number of comments, 2: Post title. */
+				_n( '%1$s response to %2$s', '%1$s responses to %2$s', get_comments_number() ),
+				number_format_i18n( get_comments_number() ),
+				'&#8220;' . get_the_title() . '&#8221;'
+			);
+		}
+		?>
+	</h2>
+
+	<div class="navigation">
+		<div class="alignleft"><?php previous_comments_link(); ?></div>
+		<div class="alignright"><?php next_comments_link(); ?></div>
+	</div>
+
+	<ol class="commentlist">
+	<?php wp_list_comments(); ?>
+	</ol>
+
+	<div class="navigation">
+		<div class="alignleft"><?php previous_comments_link(); ?></div>
+		<div class="alignright"><?php next_comments_link(); ?></div>
+	</div>
+<?php else : // This is displayed if there are no comments so far. ?>
+
+	<?php if ( comments_open() ) : ?>
+		<!-- If comments are open, but there are no comments. -->
+
+	<?php else : // Comments are closed. ?>
+		<!-- If comments are closed. -->
+		<p class="nocomments"><?php _e( 'Comments are closed.' ); ?></p>
+
+	<?php endif; ?>
+<?php endif; ?>
+
+<?php comment_form(
+	array(
+		'title_reply_before' => '<h2 id="reply-title" class="comment-reply-title">',
+		'title_reply_after'  => '</h2>'
+	)
+); ?>

I'll file a PR against wordpress-develop.

@joedolson
Copy link
Contributor

Link it against #55172 and I can review & commit it.

@carolinan
Copy link
Contributor

carolinan commented Aug 25, 2022

I would like for other options to be explored, only because I am concerned that this will set a precedence for including a .php file which should not be needed in a block theme, in a default theme.
The default themes are often used as examples.

@carolinan
Copy link
Contributor

carolinan commented Aug 25, 2022

Untested: By moving the comments area to a pattern, the blocks can be included conditionally depending on WordPress version.
The caveat is that it would only help installations where the single and page templates have not already been altered (markup saved to the database), and new installations.

@ockham
Copy link
Contributor

ockham commented Aug 25, 2022

Untested: By moving the comments area to a pattern, the blocks can be included conditionally depending on WordPress version.

Ah, I wasn't aware of that technique -- thank you @carolinan!

The caveat is that it would only help installations where the single and page templates have not already been altered (markup saved to the database), and new installations.

Still a net win I guess 😄

@annezazu
Copy link
Contributor Author

Thanks for your guidance here @carolinan <3

@ockham
Copy link
Contributor

ockham commented Aug 29, 2022

I've given this some more thought (see WordPress/wordpress-develop#3136), but I don't think we can entirely avoid customizing the comments template via PHP. I've thus now filed WordPress/wordpress-develop#3150. Curious to hear y'all's thoughts!

@carolinan
Copy link
Contributor

I'd still prefer a new heading block. The text could just be "Comments". Even if that heading is hidden visually with a screen reader text.

@ockham
Copy link
Contributor

ockham commented Sep 5, 2022

I'd still prefer a new heading block. The text could just be "Comments". Even if that heading is hidden visually with a screen reader text.

Fair enough 😄 But I'm afraid I'll have to put this on the backburner myself, since I need to focus on backporting code into Core, as the Feature Freeze is approaching fast 😅 Overall, all the attempted solutions to this issue had some rather complex implications (owed mostly to the different Comments block variants), so I'm afraid that it'll still take a while to get it right -- even with the extra heading block...

@carolinan
Copy link
Contributor

I think that is the right priority :)
I wonder if this issue should be closed and discussions can continue in the trac issue and PR's for Twenty Twenty-Two?

@annezazu
Copy link
Contributor Author

annezazu commented Sep 6, 2022

Great point. I'll close this out. Thank you all for discussing this!

@annezazu annezazu closed this as completed Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Comments Affects the Comments Block - formerly known as Comments Query Loop [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

5 participants