Make WordPress Core

Opened 2 years ago

Last modified 5 days ago

#56496 accepted defect (bug)

Twenty Twenty-Two: Update readme and docs to inform user how to change comment block

Reported by: mikachan's profile mikachan Owned by: joedolson's profile joedolson
Milestone: 6.7 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: needs-patch
Focuses: accessibility Cc:

Description (last modified by joedolson)

Ticket modified from original intent; instead update documentation so that users have a source of information explaining how to switch the comment block to the current version.

===

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 (25)

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


2 years ago
#1

  • Keywords has-patch added

#2 @mukesh27
2 years 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
2 years 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
2 years 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
2 years 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
2 years 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 2 years ago by poena (previous) (diff)

#7 @desrosj
2 years 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
2 years 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 2 years ago by poena (previous) (diff)

#9 @poena
2 years 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.


2 years ago

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


2 years ago

#12 @ryokuhi
2 years 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
2 years ago

#56798 was marked as a duplicate.

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


2 years ago

#15 @JeffPaul
2 years 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
9 months 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
9 months 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!

#18 @sabernhardt
4 months ago

  • Keywords needs-patch added; has-patch needs-refresh 2nd-opinion removed

This would need a new patch for the readme.

Since users ideally would replace/update the block, we probably could stop editing CSS for the legacy version of the block (closing #57542 and removing that part from the PR on #59334). Having the block look broken can encourage replacing it.

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


3 months ago

#20 @joedolson
3 months ago

  • Description modified (diff)
  • Milestone changed from Future Release to 6.7
  • Owner set to joedolson
  • Status changed from new to accepted
  • Summary changed from Twenty Twenty-Two: Update comment block markup to Twenty Twenty-Two: Update readme and docs to inform user how to change comment block

#21 @joedolson
3 months ago

@poena I've taken ownership of this, but if you want to make these changes, please do. I don't actually know what the steps are to change the block, so you're probably better qualified to write this...

#22 @poena
3 months ago

Here is an example text, for review:

Comment block support

If you are using WordPress version 6.1 or newer, you are recommended to update
the comment block to the latest version.

Instructions:
Open the Site Editor from your WordPress admin area.
Select "Templates" from the menu in the Site Editor sidebar.
Select the template and open it in the Site Editor.
Locate the comments block.
If the comments block has a button with the text
"Switch to editable mode", activate the button.
Save the template.

If the button is not present, than you may have already updated the block,
and you do not need to make any changes.

Repeat these instructions for the following templates:

  • Pages
  • Page (No Separators)
  • Single Posts
  • Single Post (No Separators)

This ticket was mentioned in Slack in #core-themes by karmatosed. View the logs.


3 months ago

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


2 months ago

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


5 days ago

Note: See TracTickets for help on using tickets.