Make WordPress Core

Opened 18 months ago

Last modified 6 weeks ago

#56496 new defect (bug)

Twenty Twenty-Two: Update comment block markup

Reported by: mikachan's profile mikachan Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch needs-refresh 2nd-opinion
Focuses: accessibility Cc:

Description

The comment block markup in Twenty Twenty-Two is using an outdated version of the block: <!-- wp:post-comments /-->

In the Site Editor, the block shows the following notice: You're currently using this block in legacy mode.

This should be updated to use the latest version of the comments block, e.g. wrapped in the <!-- wp:comments --> tag.

Change History (17)

This ticket was mentioned in PR #3172 on WordPress/wordpress-develop by mukeshpanchal27.


18 months ago
#1

  • Keywords has-patch added

#2 @mukesh27
18 months ago

  • Milestone changed from Awaiting Review to 6.1
  • Version trunk deleted

The PR 3172 fixes the issue.

Let me know if any changes are needed.

#3 @poena
18 months ago

The theme supports WordPress 5.9 and newer. This change removes the comments area on WordPress installations below 6.1, because the <!-- wp:comments --> block does not exist.

In this related issue, I suggested that we could try to include the old block for old WP versions, and the new block for newer WP versions, by conditionally registering different block patterns and update the templates to use that patterns.

#4 @mrfoxtalbot
18 months ago

I am not if this is related to the issue we are discussing here but I have noticed that the .logged-in-as notice saying Logged in as username. Log out? Required fields are marked * has a negative top margin that makes it overlap with the Comments Block's title. Changing the top margin to 0 fixed the problem.

(Twenty TwentyVersion: 2.0, WordPress 6.0.2 & Gutenberg version 14.1.0)

https://cloudup.com/cKtb92a6CXV

#5 @desrosj
18 months ago

  • Milestone changed from 6.1 to Future Release

Related ticket: #53617. See specifically ticket:53617#comment:20.

I'm going to move this to Future Release as I'm not really sure how to handle this and it likely will require some changes upstream as well.

#6 @poena
18 months ago

  • Type changed from enhancement to defect (bug)

This is an accessibility related bug, not an enhancement, so it should not be "blocked" by beta 1?

I am not disagreeing in the sense that a future release is more realistic, but the accessibility-ready tag has already been added to the theme besides this issue.

Last edited 18 months ago by poena (previous) (diff)

#7 @desrosj
17 months ago

To be clear, I was not moving this to Future Release because of the type of issue, but rather the problem is more complicated than just updating the syntax.

I personally don't think that conditionally loading a different syntax based on the version of WordPress being used is a good solution. Where does that end?

In my opinion, the block editor should be more defensive about handling older syntax and mapping them into newer, more compatible versions.

@poena Could you expand on why this is an accessibility bug just so I correctly understand that aspect?

#8 @poena
17 months ago

With the old comments block, the heading order on single posts is incorrect.
This makes it more difficult to navigate using headings and to determine which content on the page that is most important, and to determine the context of the comments area.
This is a minor issue.

With the new comments block, that heading level can be adjusted in the block settings, which resolves the heading order.


I would prefer that too. I am concerned that this type of workaround in the theme sets a precedence for how other themes should handle upgrades.
But I don't understand how it would work when a user updates the theme, but does not update WordPress.
Unless the suggested solution is to update the block in minor releases of older WordPress versions.

Last edited 17 months ago by poena (previous) (diff)

#9 @poena
17 months ago

More context and pull request can be found here https://core.trac.wordpress.org/ticket/55172

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


17 months ago

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


17 months ago

#12 @ryokuhi
17 months ago

  • Focuses accessibility added
  • Keywords needs-refresh added
  • Milestone changed from Future Release to 6.1.1

From #56798:

From wordpress-develop pull request 3136:

The Post Comments block uses an h3 heading on it, this causes a11y issues, as discussed in Gutenberg issue 43203 (and previously in #55172), on the templates where it's inserted, since there are no existing h2 in them. The new Comments block doesn't have the same problem because it allows you to change the heading as you need, but it's only available with Gutenberg or WP >= 6.1. To solve this issue, we are doing the following:

  • We are creating a hidden block pattern that inserts either of the blocks depending on if the new block is registered and the WP version is >= 6.1
  • For users that will get the deprecated version of the block, we are using the render_block hook to swap the h3 with an h2, and solve the a11y for them
  • Users that will get the newer version of the block, will have the h2 instead.

To test this:

  • Run this branch, check a page with comments on it, you should be seeing the headings using h2. On the editor, you should be seeing the new Comments block (there is no deprecation message present). You may need to tweak this line in hidden-comments.php, since this branch is still not a 6.1 version of WP:
    if (
      WP_Block_Type_Registry::get_instance()->is_registered( 'core/comments' ) &&
      version_compare( $GLOBALS['wp_version'], '6.1', '>=' )
    ) {
    
  • Run the code from this branch on a 6.0 environment, you should be seeing the old block, with the deprecation notice in the editor. In the frontend, you should still see the headings on the comments block using h2, instead of h3.

Aside from a11y, note that this PR also improves User Experience of the TT2 theme: Users with WP >= 6.1 will get the Comments block in editable mode automatically, rather than seeing the legacy mode and the upgrade notice.

This was raised in #55172, but deemed not a blocker to adding the Accessibility Ready tag. Props to @poena when merged for raising this issue.

After discussing ticket #56798 during the accessibility team's weekly bug scrub, as suggested by @poena, we decided to close that ticket in favor of this one. Props to @desrosj for reporting this discussion from the wordpress-develop Pull Request to Trac.
We also decided to move this ticket to the 6.1.1 milestone, to add the accessibility focus (as this is actually an accessibility issue), and to ask for a patch refresh that takes into consideration the above.

#13 @ryokuhi
17 months ago

#56798 was marked as a duplicate.

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


16 months ago

#15 @JeffPaul
16 months ago

  • Milestone changed from 6.1.1 to Future Release

With no confirmation on approach, I'm punting out of the 6.1.1 milestone to Future Release. Once folks can settle on approach, then it can get pulled back into a numbered milestone... thanks!

#16 @poena
6 weeks ago

  • Keywords 2nd-opinion added

I no longer believe that it is reasonable to fix this, neither in the theme or in the already deprecated block.

Users who activate the theme on 6.1 or later can solve the problem by replacing the block or by clicking on the "legacy" / update button that the editor provides.

What we can do is add information about it to the readme file and the documentation.

#17 @mikachan
6 weeks ago

I no longer believe that it is reasonable to fix this, neither in the theme or in the already deprecated block.

I agree, I don't think we should fix this in the theme anymore. Users can solve the problem via the editor if needed.

What we can do is add information about it to the readme file and the documentation.

Sounds good. Thanks @poena!

Note: See TracTickets for help on using tickets.