Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#53803 closed defect (bug) (fixed)

Customizer: Radio button on background image position selector visible on focus

Reported by: walbo's profile walbo Owned by: peterwilsoncc's profile peterwilsoncc
Milestone: 5.8.1 Priority: normal
Severity: normal Version: 5.8
Component: Customize Keywords: has-screenshots has-patch commit fixed-major
Focuses: accessibility, css Cc:

Description

After 5.8 the radio buttons on the background image position selector show on focus. Seems like the css style added by the new customizer widget screen affects the image position selector.

Steps to reproduce:

  1. Use a theme that has background image option. Ex twenty-twenty-one
  2. Go to customizer > Background Image
  3. Upload an image and change the position. Radio button is visible.

Expected behavor:
The radio button is hidden.

Attachments (6)

background-position.gif (2.7 MB) - added by walbo 4 years ago.
background-position-controls-chrome.png (266.5 KB) - added by sabernhardt 4 years ago.
background-position-controls-chrome-classic-widgets-active.png (590.5 KB) - added by sabernhardt 4 years ago.
radio buttons not visible with Classic Widgets plugin activated (WP5.8)
53803.patch (435 bytes) - added by ravipatel 4 years ago.
Radio input focus css added based on ref class
53797.patch (10.8 KB) - added by ravipatel 4 years ago.
@walbo I have created patch with ui-helper-hidden-accessible, now css and class both patch avaiable.
53803.2.patch (1.9 KB) - added by ravipatel 4 years ago.
@walbo now added a right ticket patch, chanegs class as well in file.

Change History (20)

@sabernhardt
4 years ago

radio buttons not visible with Classic Widgets plugin activated (WP5.8)

#1 @sabernhardt
4 years ago

  • Focuses css added
  • Milestone changed from Awaiting Review to 5.8.1

The .screen-reader-text:focus styles in block-library/style.css override the clip value (and other properties).

@ravipatel
4 years ago

Radio input focus css added based on ref class

#2 @sabernhardt
4 years ago

Thanks for the patch! Unfortunately, the input uses the screen-reader-text class purposely to avoid display:none or visibility:hidden.

A negative top value might help for a quick fix:

.background-position-control .screen-reader-text:focus {
    top: -9999px;
}

I'd like to try pulling the radio button out of the label tag (using for and id attributes), but that is too much to change comfortably in a minor release.

#3 @sabernhardt
4 years ago

We also could reconsider assigning the ui-helper-hidden-accessible class in the markup (ticket:22058#comment:103), at least for now.

#4 @walbo
4 years ago

As you say @sabernhardt it should probably be refactored in a major release. I did a quick test locally and ui-helper-hidden-accessible will work as a fix for 5.8.1.

Will create a patch tomorrow unless @ravipatel updates his patch.

@ravipatel
4 years ago

@walbo I have created patch with ui-helper-hidden-accessible, now css and class both patch avaiable.

#5 @walbo
4 years ago

Thanks @ravipatel. However it looks like your last patch was meant for a different ticket.

This ticket was mentioned in PR #1523 on WordPress/wordpress-develop by walbo.


4 years ago
#6

  • Keywords has-patch added; needs-patch removed

#7 follow-up: @sabernhardt
4 years ago

  • Focuses accessibility added

On second thought, we need to remove the screen-reader-text:focus styles in Gutenberg instead. These radio buttons might not be the only focusable elements in Core with that class, and users' websites could have something similar on the front end.

PR 30141 apparently added the screen-reader-text styles for the Archives block's optional dropdown label. The focus styles are intended mainly for skip links, not just anything with the class.

#8 @walbo
4 years ago

Thanks @sabernhardt

I'll create a issue upstream in Gutenberg repo.

@ravipatel
4 years ago

@walbo now added a right ticket patch, chanegs class as well in file.

#9 @sabernhardt
4 years ago

Thanks for opening the issue GB33724 and PR 33726

#10 in reply to: ↑ 7 @mukesh27
4 years ago

Replying to sabernhardt:

On second thought, we need to remove the screen-reader-text:focus styles in Gutenberg instead. These radio buttons might not be the only focusable elements in Core with that class, and users' websites could have something similar on the front end.

PR 30141 apparently added the screen-reader-text styles for the Archives block's optional dropdown label. The focus styles are intended mainly for skip links, not just anything with the class.

Agreed with @sabernhardt second thoughts.

Thanks for PR @walbo

#11 @peterwilsoncc
4 years ago

  • Keywords commit added

Testing with 53803.2.patch applied:

  • Radio buttons hidden within customizer editor
  • Radio buttons hidden on deprecated wp-admin/themes.php?page=custom-background

I think this is good for the minor release fix.

--

As mentioned elsewhere on this ticket, it probably needs a refactor in a major release. Current best practice is to place the form controls over the replacement control with opacity: 0 to ensure haptic feedback remains on touch devices and it doesn't look like that is the case here.

#12 @peterwilsoncc
4 years ago

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

In 51637:

Customize: Hide native control on background position field.

Hide the browser's native radio button on the custom background position selector in the Customizer and the legacy background screen. This fixes an issue causing both to display.

Props mukesh27, ravipatel, sabernhardt, walbo.
Fixes #53803.

#13 @peterwilsoncc
4 years ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for merging in to the 5.8 branch.

#14 @desrosj
4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 51640:

Customize: Hide native control on background position field.

Hide the browser's native radio button on the custom background position selector in the Customizer and the legacy background screen. This fixes an issue causing both to display.

Props mukesh27, ravipatel, sabernhardt, walbo, peterwilsoncc.
Merges [51637] to the 5.8 branch.
Fixes #53803.

Note: See TracTickets for help on using tickets.