WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 23 months ago

Last modified 19 months ago

#39096 closed defect (bug) (fixed)

Placeholder Text in input field not fully visible

Reported by: Presskopp Owned by: westonruter
Milestone: 4.8 Priority: normal
Severity: normal Version: 3.8
Component: Customize Keywords: has-ui-feedback has-patch has-screenshots
Focuses: Cc:

Description

Example is the color picker in Customizer (using Twenty Seventeen)

The fiels shows "Hex Valu" (more or less in different browsers).

This seems to come from

 .wp-picker-container input[type="text"].wp-color-picker {
	width: 65px; 

in \wp-admin\css\color-picker.css, L.102

Attachments (10)

39096.jpg (14.1 KB) - added by Presskopp 2 years ago.
39096.patch (620 bytes) - added by stormrockwell 2 years ago.
fixed width, added padding for responsive issue, added wp-picker-clear css to fix responsive issues
39096.2.patch (707 bytes) - added by stormrockwell 2 years ago.
same as previous patch but better styling for the clear button
39096-2-patch.jpg (64.4 KB) - added by stormrockwell 2 years ago.
photo of the 39096.2 patch
39096.diff (730 bytes) - added by Presskopp 2 years ago.
39096.3.patch (1.2 KB) - added by tejas5989 2 years ago.
39096-3-patch.png (11.6 KB) - added by tejas5989 2 years ago.
39096.4.diff (1.4 KB) - added by westonruter 2 years ago.
39096.5.diff (1.5 KB) - added by westonruter 2 years ago.
39096.4.patch (1.7 KB) - added by sagarprajapati 2 years ago.

Download all attachments as: .zip

Change History (53)

@Presskopp
2 years ago

#1 @pento
2 years ago

  • Keywords needs-patch ui-feedback added
  • Milestone changed from Awaiting Review to Future Release
  • Version changed from trunk to 3.8

Thanks for the bug report, @Presskopp! I can see the same behaviour.

The width value causing this was introduced in [26435].

@stormrockwell
2 years ago

fixed width, added padding for responsive issue, added wp-picker-clear css to fix responsive issues

@stormrockwell
2 years ago

same as previous patch but better styling for the clear button

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


2 years ago

#3 @stormrockwell
2 years ago

  • Keywords has-patch added; needs-patch removed

@stormrockwell
2 years ago

photo of the 39096.2 patch

#4 @karmatosed
2 years ago

  • Keywords has-ui-feedback commit added; ui-feedback removed
  • Milestone changed from Future Release to 4.7.2

This fix looks good, but I do wonder if we need a more flexible solution for translation going forward? It does fix the initial issue though.

#5 @Presskopp
2 years ago

The string is longer in some languages, yes.

Example: Хексадекадна вредност (Macedonian)

#6 follow-up: @westonruter
2 years ago

  • Keywords 2nd-opinion added; commit removed

Instead of the placeholder being a label, what about a sample value? Like #21759b…? Then it won't vary by language and it would give users a cue to what the thing is that they should be entering there. Users may not even know what a “hex color” is, but they would probably recognize #21759b as some color code thing.

@Presskopp
2 years ago

#7 @Presskopp
2 years ago

1) thinking about what @westonruter proposed
2) made a new patch
3) waiting for feedback

@tejas5989
2 years ago

#8 @tejas5989
2 years ago

I have added the patch with the two changes regarding the Design and the color Hexa sample code. I have also attached screenshot for your review.

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


2 years ago

#10 in reply to: ↑ 6 @afercia
2 years ago

Replying to westonruter:

Instead of the placeholder being a label, what about a sample value? Like #21759b…?

I'd totally second this, since placeholders should not be used as replacements for field labels. From the specs:

The placeholder attribute represents a short hint (a word or short phrase) intended to aid the user with data entry when the control has no value. A hint could be a sample value or a brief description of the expected format.
...
Note: Use of the placeholder attribute as a replacement for a label can reduce the accessibility and usability of the control ...

Other references:

Nielsen Norman Group: Placeholders in Form Fields Are Harmful

The Paciello Group: HTML5 Accessibility Chops: the placeholder attribute

Roger Johansson: The HTML5 placeholder attribute is not a substitute for the label element

#11 follow-up: @westonruter
2 years ago

So what is the best sample value for a placeholder? Perhaps having a full value like #21759b would be misleading. What about #FF0... or something that makes it clear that it is not supplied yet. I can imagine someone who is new to computers might interpret a grayed-out placeholder value as just a regular value being already supplied or that the input is disabled.

#12 in reply to: ↑ 11 @afercia
2 years ago

Replying to westonruter:

So what is the best sample value for a placeholder? Perhaps having a full value like #21759b would be misleading. What about #FF0... or something that makes it clear that it is not supplied yet.

Yep, that would be ideal. Not sure about the best sample though. Maybe #addc01? :) Seriously, sometimes the hex format is represented as #RRGGBB but I guess that would be a bit too technical for users.

#13 follow-up: @westonruter
2 years ago

@afercia I meant to actually include the ellipsis in the placeholder. Also, R and G are not valid hexadecimal numbers which range 0-F :)

#14 in reply to: ↑ 13 @afercia
2 years ago

@westonruter I know it's not a valid value :) I meant sometimes the format is represented in that way because RRGGBB represent the red, green and blue components of the color. In the same way, when we use dd/mm/yyyy that's not a valid value, but it represents the date format.

#15 follow-up: @westonruter
2 years ago

@afercia So maybe #RGB… would be a good placeholder?

#16 in reply to: ↑ 15 @afercia
2 years ago

Replying to westonruter:

@afercia So maybe #RGB… would be a good placeholder?

Hm not sure :) That could be a bit misleading, it made me immediately think to the rgb format.

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


2 years ago

#18 follow-up: @SergeyBiryukov
2 years ago

If we know the current/default color (the screenshot suggests that we do), can we use its HEX value as a placeholder, instead of some random value?

@westonruter
2 years ago

#19 @westonruter
2 years ago

@SergeyBiryukov good point. The control does have a defaultValue parameter that is obtained from the setting's default. See 39096.4.diff.

#20 follow-up: @westonruter
2 years ago

There is a chance that the underlying setting associated with the control would lack a default being defined, and in that case no placeholder would be shown.

#21 in reply to: ↑ 20 @SergeyBiryukov
2 years ago

Replying to westonruter:

There is a chance that the underlying setting associated with the control would lack a default being defined, and in that case no placeholder would be shown.

That's what I got when testing 39096.4.diff on trunk with Twenty Seventeen :)

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


2 years ago

#23 @westonruter
2 years ago

  • Milestone changed from 4.7.3 to 4.7.4

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


2 years ago

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


2 years ago

This ticket was mentioned in Slack in #design by swissspidy. View the logs.


2 years ago

#27 @swissspidy
2 years ago

  • Milestone changed from 4.7.4 to 4.7.5

#28 in reply to: ↑ 18 ; follow-up: @melchoyce
2 years ago

Replying to SergeyBiryukov:

If we know the current/default color (the screenshot suggests that we do), can we use its HEX value as a placeholder, instead of some random value?

+1.

There is a chance that the underlying setting associated with the control would lack a default being defined, and in that case no placeholder would be shown.

Is this because a theme or plugin might not define a default, or something else? In that case, could we show something more obviously a placeholder? Like: ex: #ff0000 (Specifically using "ex:" here to say "this is an example, not a real value")

#29 @joyously
2 years ago

I like that ex: #ff00bb kind of thing, as long as that makes sense in other languages.
If not that, I think the #RRGGBB is very useful, even though it's English words that those are initial letters for.
This ticket doesn't take into account that the color picker is very limited in that it only accepts hex values, and no alpha, and no color keywords. These enhancements could come along and then this placeholder is moot.

#30 in reply to: ↑ 28 @westonruter
2 years ago

Replying to melchoyce:

Is this because a theme or plugin might not define a default, or something else?

That's right, as @SergeyBiryukov found in 21.

Replying to joyously:

I like that ex: #ff00bb kind of thing, as long as that makes sense in other languages.

That's my concern, that ex: or e.g. wouldn't have a short abbreviation in other languages.

If not that, I think the #RRGGBB is very useful, even though it's English words that those are initial letters for.
This ticket doesn't take into account that the color picker is very limited in that it only accepts hex values, and no alpha, and no color keywords. These enhancements could come along and then this placeholder is moot.

I think #RRGGBB is probably the best fallback placeholder text, even if R and G are not valid hex digits, as it's a placeholder not a default value to be re-used.

@westonruter
2 years ago

#31 @westonruter
2 years ago

  • Keywords needs-testing added; 2nd-opinion removed

Ok, 39096.5.diff implements default #RRGGBB. Give it a shake and I'll commit.

#32 @sagarprajapati
2 years ago

Hi @westonruter

I have checked your latest patch it is working fine on desktop device but it is not working on mobile devices. I have updated the patch and it's working on mobile devices as well as desktop devices. Please review it.

Thanks

#33 @mayurk
2 years ago

  • Keywords has-screenshots added; needs-testing removed

Hi @westonruter , @sagarprajapati , @swissspidy

I have checked latest patch https://core.trac.wordpress.org/attachment/ticket/39096/39096.4.patch and it's working on desktop browsers (chrome, safari, firefox) as well as mobile devices (iPhone 5, iPhone 6).

I have attached some images for review. Please check it.

http://i.imgur.com/vP3HRD0.png
http://i.imgur.com/dvlNxQ0.png
http://i.imgur.com/kc0JLxT.png
http://i.imgur.com/RmQ7cUy.png
http://i.imgur.com/MS7QQ3e.png

Thanks

#34 @westonruter
2 years ago

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

In 40471:

Customize: Use fixed-width illustrative placeholder for hex field in color picker to prevent truncation.

Props stormrockwell, sagarprajapati, Presskopp, afercia, tejas5989, westonruter, mayurk for testing.
Fixes #39096.

#35 @westonruter
2 years ago

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

Re-opening for consideration in 4.7.5

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


23 months ago

#37 @desrosj
23 months ago

  • Milestone changed from 4.7.5 to 4.8

#38 @westonruter
23 months ago

  • Keywords fixed-major removed
  • Resolution set to fixed
  • Status changed from reopened to closed

Closing since in trunk.

#39 @afercia
19 months ago

Seems to me a couple of CSS rules introduced here break the responsiveness of the input field. In the responsive view, it should become a bit bigger (as the button does). Instead now it looks like this:

https://cldup.com/_jEL4ud8fE.png

Not sure this is really needed:

	.wp-picker-container input[type="text"].wp-color-picker {
		margin-right: 6px;
		padding: 3px 5px;
	}

I'm going to fix this in the refreshed patch for #39662. Please ping me if I'm missing something.

#40 @afercia
19 months ago

OK I realize now the left and right padding are necessary, but they should be adjusted and top and bottom not changed.

Last edited 19 months ago by afercia (previous) (diff)

#41 @westonruter
19 months ago

@afercia Would you open a new ticket with a patch?

#42 @westonruter
19 months ago

@afercia or I suppose the fix can just be added as part of #39662.

#43 @afercia
19 months ago

@westonruter

I'm going to fix this in the refreshed patch for #39662.

Note: See TracTickets for help on using tickets.