Make WordPress Core

Opened 2 months ago

Closed 7 weeks ago

#60535 closed defect (bug) (fixed)

Twenty Twenty-Four: Unintended focus outline is applied in the Site Editor

Reported by: wildworks's profile wildworks Owned by: youknowriad's profile youknowriad
Milestone: 6.5 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch commit
Focuses: accessibility, css Cc:

Description

Improvements have been made to the focus outline style for Twenty Twenty-Four in [57554]. This changeset adds the following styles, which appear to affect all focusable elements in the site editor.

.wp-site-blocks *:focus{
  outline-width:2px;outline-style:solid
}

Attachments (6)

table.png (7.4 KB) - added by wildworks 2 months ago.
Outline when focusing on a cell in a table block
paragraph.png (13.0 KB) - added by wildworks 2 months ago.
Outline when focusing on paragraph block
60535.diff (545 bytes) - added by sabernhardt 2 months ago.
lowers the specificity of Twenty Twenty-Four's focus style
60535.2.diff (535 bytes) - added by sabernhardt 2 months ago.
using :where(.wp-site-blocks *:focus)
65-beta2-table-block-tt4-with-patch.png (61.6 KB) - added by poena 2 months ago.
Screenshot of 60535.2.diff: The table block in the Site Editor, with a cell selected and no visible focus style
Twenty Twenty-Four theme with WooCommerce.png (31.4 KB) - added by oglekler 8 weeks ago.

Download all attachments as: .zip

Change History (30)

@wildworks
2 months ago

Outline when focusing on a cell in a table block

@wildworks
2 months ago

Outline when focusing on paragraph block

#1 @poena
2 months ago

  • Focuses accessibility css added

So what is the expected way for any users to add focus styles?

The "elements" in theme.json includes links and buttons. There are so many more focusable elements, so its not a good enough replacement.

Do we really need to enqueue the style.css file in block themes, or should the specificity in the editor styles solve this?

#2 @wildworks
2 months ago

I'm wondering if it's necessary to override the browser's default focus outline style in the first place. This is because I believe that the browser's default focus style should meet accessibility standards. If we want to avoid the dot style, how about just removing the focus style itself from theme.json?

If we want to explicitly increase the thickness of the default focus outline, we might have no choice but to load style.css only on the front end or define a style like the one below.

{
	"$schema": "https://schemas.wp.org/trunk/theme.json",
	"version": 2,
	"styles": {
		"elements": {
			"button": {
				":focus": {
					"css": "outline-width:2px;outline-style:solid"
				}
			}
		},
		"blocks": {
			"core/navigation-link": {
				"elements": {
					"link": {
						":focus": {
							"css": "outline-width:2px;outline-style:solid"
						}
					}
				}
			},
			"core/comments": {
				"css": "& input:focus { outline-width:2px;outline-style:solid }"
			}
		}
	}
}

#3 @poena
2 months ago

This is not only a problem for this theme, but for all block themes where someone wants to change the focus style.

#4 @sabernhardt
2 months ago

Considering that the purpose of the thicker style is to make focused elements more visible, I would call the focus style in the site editor more of a "happy accident" than a mistake.

However, it is inconsistent with the post editor, so it probably should be changed.

The theme could check whether .wp-site-blocks also includes one of the editor classes. The editor's class attribute (for now) includes block-editor-block-list__layout and edit-site-editor-canvas__block-list.

.wp-site-blocks:not(.block-editor-block-list__layout) *:focus

Or if raising the specificity more is a problem, it could use :where()

.wp-site-blocks:where(:not(.block-editor-block-list__layout)) *:focus

If the editor's content.css is changed instead, html .rich-text:focus would be enough to override Twenty Twenty-Four's style.

@sabernhardt
2 months ago

lowers the specificity of Twenty Twenty-Four's focus style

#5 @sabernhardt
2 months ago

  • Component changed from Themes to Bundled Theme
  • Keywords has-patch added

A less specific selector in the theme could avoid the extra outline in the site editor without needing to use an editor-related class name.

#6 @poena
2 months ago

  • Milestone changed from Awaiting Review to 6.6

#7 @poena
2 months ago

  • Milestone changed from 6.6 to 6.5

#8 @wildworks
2 months ago

I think a lower selector makes sense to me too. On the editor side, no unnatural outlines appear to occur.

What do you think about setting the specificity to zero to ensure that only the browser default styles are overridden?

:where(.wp-site-blocks *:focus) {outline-width:2px;outline-style:solid}

#9 @youknowriad
2 months ago

I agree that lower selector makes sense. Did we consider removing .wp-site-blocks as well? Is that really necessary?

#10 @poena
2 months ago

Thank you for your help, everyone.

I used .wp-site-blocks to prevent the style from affecting the admin toolbar links, log out link, and skip link. Which are outside (before) <div class="wp-site-blocks">.

Updating the style in the theme is a short term fix, it might solve this exact situation but does not prevent other themes and users from breaking the Site Editor focus styles.

#11 @youknowriad
2 months ago

Updating the style in the theme is a short term fix, it might solve this exact situation but does not prevent other themes and users from breaking the Site Editor focus styles.

I agree with the logic but for me the issue is the usage of the specific .wp-site-blocks because with the same logic above, you could say the admin bar should have a higher specificity for these things.

This style is like a "reset" so IMO, it should have the lowest specificity possible.

@sabernhardt
2 months ago

using :where(.wp-site-blocks *:focus)

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


2 months ago

@poena
2 months ago

Screenshot of 60535.2.diff: The table block in the Site Editor, with a cell selected and no visible focus style

#13 @poena
2 months ago

  • Keywords commit added

I have not found a way to make it work without the wp-site-blocks class.
And I don't think the theme should enqueue the style CSS file only to increase the outline width; so I would like to re-add the commit keyword for patch 60535.2.diff.

#14 @oglekler
8 weeks ago

Hi @poena this can have a huge impact on the front end, and if I am using a mouse to hover, the hover behavior should be enough if I am not using a keyboard to navigate (how these things can be saparated - is a question too). I found a couple of places where this new style looks really weird, but with different themes and places, this can have an impact in many different and unexpected places.

Last edited 8 weeks ago by oglekler (previous) (diff)

#15 @poena
8 weeks ago

I did not understand:

  • The CSS does not change the :hover styles and the :active styles.
  • Nor does it change other themes.

#16 @poena
8 weeks ago

That WooCommerce tabs are problematic to style is not a problem with the CSS proposed for this ticket, but with the plugin.
You can see that even in Woo's own theme "Storefront", Hello Elementor, Astra, Kadence, and GeneratePress, the focus outline for the tabs on the single product page is only visible on three sides.

WooCommerce itself includes extra CSS for the tabs for several of the bundled themes: but not for Twenty Twenty-Four.

#17 @oglekler
8 weeks ago

Thank you, @poena, I just panicked a bit when I saw this focus behaviour because one time we had global change in images appearance and at some sites I was maintaining back then all images become stretched. Of course, I was testing before an update, but still it was a surprise.

Can we use :where() as @sabernhardt has suggested? It most likely will allow plugins to keep their own focus behaviour on elements they specified.

#18 @poena
8 weeks ago

I tried removing the outline width on links with the :active state, but it did not hide the outline when I right click on a link (Chrome and Firefox on macOS).

It only kind of worked on left click mouse down. On mouse up, both the outline and text decoration are visible again.

I just added this in theme.json under styles > elements > link

":active": {
	"outline": {
		"width": "0"
	}
},

#19 @wildworks
8 weeks ago

The reason why focus styles are applied even when you right-click an element is probably because right-click implicitly puts focus on the element. If this was intended, I don't think it's a bug. For example, Twenty Twenty-One also has its own focus style, which can be applied in the right-click menu.

https://github.com/WordPress/wordpress-develop/blob/ec37b57fb98a28539f9427f81c3cf268d46561ad/src/wp-content/themes/twentytwentyone/style.css#L1594-L1601

If the right-click behavior is not what we want, hou about using focus-visible?

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


7 weeks ago

#21 @wildworks
7 weeks ago

I believe that the current focus style is a big problem on the editor side and needs to be fixed in some way in WP6.5. If we need more time to investigate the impact on consumers or find a more ideal approach, how about reverting [57554]?

#22 @poena
7 weeks ago

I added the commit keyword 8 days ago but I do not have commit access myself.

So perhaps the core or theme commiters would like to add their opinions.

#23 @youknowriad
7 weeks ago

I'm going to commit 60535.2.diff I think it's a good compromise for now.

#24 @youknowriad
7 weeks ago

  • Owner set to youknowriad
  • Resolution set to fixed
  • Status changed from new to closed

In 57739:

Bundled Theme: Fix focus outline in Twenty Twenty-Four in the editor.

Improvements made to the focus outline style for Twenty Twenty-Four caused a regression in the focus outlines of the block editor. This commit solves the regressions by reducing the CSS specificity while keeping the improvements.

Follow-up to [57554].

Props wildworks, poena, sabernhardt, youknowriad, oglekler.
Fixes #60535.

Note: See TracTickets for help on using tickets.