Customize: fix the color hue picker HTML and accessibility

[afercia]

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

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:

[subrataemfluence]

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)

[subrataemfluence]

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

[celloexpressions]

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

[subrataemfluence]

JavaScript diff file

[subrataemfluence]

PHP diff file

[subrataemfluence]

@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?

[dlh]

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]

@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.

[afercia]

Issue created: ​https://github.com/Automattic/Iris/issues/64

[jorbin]

We shouldn't allow perfect to be the enemy of better. https://core.trac.wordpress.org/attachment/ticket/42078/42078.diff contains some improvements that we should commit while ​https://github.com/Automattic/Iris/issues/64 is worked on.

Trac ticket: #42078

I tested the changes as mentioned by @afercia on the ticket and found the first three points to be addressed.

About the 4th point (the hidden input field should not be hidden, and should be used as an alternative way to enter a hue value), I think it makes sense to provide an alternative way to manipulate the values.

But I think @joedolson would be a better judge for that.

[mleraygp]

It seems that the current PR only fixes the Twenty Seventeen theme, but aren't there some other themes using similar hue pickers?

Do we need to update other themes as well? It seems that ​Twenty Nineteen for example also has a hue customizer

@mleray Yes, other instances without labels also need to be fixed. I've added changes for Twenty Nineteen and Twenty Twenty; all other core instances already pass the label parameter.

@himanshupathak95 I think we should commit this without worrying about hidden input. Given that the issue has been open on the Iris repository for 8 years, I don't think it's likely that will be updated any time soon.

[joedolson]

I think that this is commit ready with the additional changes for Twenty Nineteen and Twenty Twenty added.

Waiting on changes from Iris doesn't seem productive, given the number of years since that was last progressed. It may make sense to consider that abandoned, and fork it or see if Automattic will move it into the WordPress.org organization so we can maintain it. @jorbin

[joedolson]

In 60478:

Customize: Use explicit labels in hue picker.

Add for and id attributes to the customizer hue picker, so labels are explicitly associated. Move picker outside of label element. Add label value for Twenty Seventeen, Twenty Nineteen, and Twenty Twenty.

Props afercia, subrataemfluence, dlh, jorbin, sainathpoojary, abcd95, mleraygp, joedolson.
Fixes #42078.

This was closed in https://core.trac.wordpress.org/changeset/60478