WordPress.org

Make WordPress Core

Opened 2 months ago

Last modified 4 days ago

#42078 new defect (bug)

Customize: fix the color hue picker HTML and accessibility

Reported by: afercia Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 4.7
Component: Customize Keywords: has-screenshots has-patch
Focuses: accessibility Cc:

Description

The hue color picker control used in the Customizer, for example with Twenty Seventeen, has an empty <label> element and incorrect markup:

https://cldup.com/tjBoOqM20j.png

the actual markup:

<div class="customize-control-content">
	<label>
		<span class="screen-reader-text"></span>
		<input class="color-picker-hue" type="text" data-type="hue" style="display: none;">
		<div class="iris-picker iris-only-strip ... >{ rest of the iris color picker here}</div>
	</label>
</div>
  • the label is actually empty
  • it should not wrap the hidden input field and should use for/id attributes as per the WordPress accessibility standard
  • the iris picker should not be inside the label
  • the hidden input field should not be hidden, and should be used as an alternative way to enter a hue value

Worth noting the normal color picker does use an input field as alternative to make sure everyone, with any device or ability, can enter a value:

https://cldup.com/syZnWhJHkC.png

Attachments (6)

42078.diff.zip (1.1 KB) - added by subrataemfluence 2 months ago.
Includes: 42078-class-wp-customize-color-control.php.diff and 42078-customize-controls.js.diff
Screenshot.png (22.0 KB) - added by subrataemfluence 2 months ago.
42078-customize-controls.js.diff (349 bytes) - added by subrataemfluence 6 weeks ago.
JavaScript diff file
42078-class-wp-customize-color-control.php.diff (1.4 KB) - added by subrataemfluence 6 weeks ago.
PHP diff file
42078.diff (3.3 KB) - added by dlh 5 days ago.
42078.debug.diff (3.1 KB) - added by afercia 4 days ago.

Download all attachments as: .zip

Change History (13)

#1 @afercia
2 months ago

  • Summary changed from Customizer: fix the color hue picker HTML and accessibility to Customize: fix the color hue picker HTML and accessibility

#2 @subrataemfluence
2 months ago

  • Keywords has-patch added

Hi afercia,

Please let me know whether the patch works. I have added a maxlength attribute to color-picker-hue control and set a value of 3. Now the actual markup looks like (no longer wrapped inside a label):

<div class="customize-control-content">
   <input class="color-picker-hue" type="text" data-type="hue" maxlength="3">
   <div class="iris-picker iris-only-strip ... >{ rest of the iris color picker here}</div>
</div>

However, looks like there is another issue. When dragging the hue slider, value gets updated in color-picker-hue textbox but doing it other way round, i.e. changing value in the textbox does not update/move slider handle. when I press tab, the value resets in the textbox with current slider value.

WordPress version is 4.8.2
Tested in Chrome and Firefox (Ubuntu 14.04)

Last edited 6 weeks ago by subrataemfluence (previous) (diff)

@subrataemfluence
2 months ago

Includes: 42078-class-wp-customize-color-control.php.diff and 42078-customize-controls.js.diff

#3 follow-up: @celloexpressions
6 weeks ago

  • Milestone changed from Awaiting Review to Future Release
  • Version set to 4.7

@subrataemfluence could you please upload your patch as a .diff file, not .zip?

@subrataemfluence
6 weeks ago

JavaScript diff file

#4 in reply to: ↑ 3 @subrataemfluence
6 weeks ago

@celloexpressions I created the zip because there are two files modified. I have uploaded following two separate diff files.

42078-customize-controls.js.diff and 42078-class-wp-customize-color-control.php.diff

Please let me know if this is the way you wanted it.

From my earlier comment:

However, looks like there is another issue. When dragging the hue slider, value gets updated in color-picker-hue textbox but doing it other way round, i.e. changing value in the textbox does not update/move slider handle. when I press tab, the value resets in the textbox with current slider value.

I am trying to fix the above and create a patch.

Thank you!

Replying to celloexpressions:

@subrataemfluence could you please upload your patch as a .diff file, not .zip?

Last edited 6 weeks ago by subrataemfluence (previous) (diff)

@dlh
5 days ago

#5 @dlh
5 days ago

42078.diff builds on the previous patches with approaches for almost all of the issues described.

the label is actually empty

This appears to be a bug in Twenty Seventeen, which doesn't pass a label to the control. I assume that no label is used because the control is meant to be visually part of the "Color Scheme" control above it.

The patch adds a label along with JavaScript that applies the screen-reader-text class to maintain the existing appearance.

it should not wrap the hidden input field and should use for/id attributes as per the WordPress accessibility standard

The patch moves the hidden input field out of the label and adds the attributes.

the iris picker should not be inside the label

The change to the label in the patch should also cause the Iris picker to appear outside the label.

the hidden input field should not be hidden, and should be used as an alternative way to enter a hue value

I encountered the same difficulty as @subrataemfluence here: The field responds to the slider, but not to manual input. If I had to guess, I would say the _addInputListeners() function in iris.min.js isn't handling the values correctly, but that's only a guess.

@afercia
4 days ago

#6 @afercia
4 days ago

@dlh @subrataemfluence thanks. Yep, seems Iris doesn't handle the case of a hue value being changed from the input. I've played a bit with https://github.com/Automattic/Iris/blob/add/hue-only-mode/dist/iris.js which should be the version currently used in core. Seems to me it wouldn't be so hard to make _addInputListeners() pass the value, the problem now is that the color.js toString() always transform the input value in a hex color.

I'm going to open an issue on the Iris GitHub repo, anyone willing to contribute and follow there is very welcome.

In the meantime, 42078.debug.diff keeps the label and input field visible for debugging purposes.

Note: See TracTickets for help on using tickets.