Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#48131 closed defect (bug) (fixed)

Height issue in wp-color-picker

Reported by: audrasjb's profile audrasjb Owned by: audrasjb's profile audrasjb
Milestone: 5.3 Priority: normal
Severity: normal Version: 5.3
Component: Administration Keywords: has-patch has-screenshots 5-3-admin-css-changes
Focuses: ui, accessibility Cc:

Description

In 5.3 beta 1, a regression appeared on WP core colorpickers.

We just need to use min-height instead of height in colorpicker related CSS rule and it would fix the issue.

Adding accessibility focus since it's related to previous accessibility tickets.

Attachments (8)

48131.diff (404 bytes) - added by audrasjb 5 years ago.
Replace height with min-height in color picker related CSS rule
Capture d’écran 2019-09-24 à 22.54.22.png (30.5 KB) - added by audrasjb 5 years ago.
before
Capture d’écran 2019-09-24 à 22.54.50.png (37.9 KB) - added by audrasjb 5 years ago.
after
Screen Shot 2019-09-24 at 2.43.52 PM.png (21.1 KB) - added by garrett-eclipse 5 years ago.
Varied heights on colour picker inputs/buttons
48131.2.diff (1.0 KB) - added by garrett-eclipse 5 years ago.
Address the height discrepancy on desktop & mobile, and increase width of color picker result slightly on desktop so hex code isn't cut off.
48131 5.2.3.png (24.4 KB) - added by afercia 5 years ago.
48131.3.diff (1.1 KB) - added by afercia 5 years ago.
color picker.png (49.3 KB) - added by afercia 5 years ago.

Download all attachments as: .zip

Change History (19)

@audrasjb
5 years ago

Replace height with min-height in color picker related CSS rule

#1 @audrasjb
5 years ago

  • Keywords has-patch has-screenshots added; needs-patch removed
  • Owner set to audrasjb
  • Status changed from new to assigned

My bad: Screenshot 1 is after the patch and screenshot 2 if before the patch :)

The patch works fine on my side. Would need double sign-off before commit.

#2 @audrasjb
5 years ago

Thanks @ipstenu for reporting that bug from dotorg forums and for @garrett-eclipse for testing the issue.

#3 @garrett-eclipse
5 years ago

Thanks @audrasjb testing here that corrected the issue of the colour wrapping too far.

One thing to note though in my testing was the three form elements there all have different heights (screen coming).
The .wp-color-result is 24px, then the .color-picker-hex is 30px and finally the .wp-picker-default is 25px. To me that seems odd and would expect them to all be in line with one another for that control. Should that be addressed here or a new ticket created?

@garrett-eclipse
5 years ago

Varied heights on colour picker inputs/buttons

#4 @audrasjb
5 years ago

Thanks for the feedback @garrett-eclipse !
Let's handle all height issues in this ticket, I think it should be easier to track :-)

#5 @audrasjb
5 years ago

  • Keywords needs-refresh added

@garrett-eclipse
5 years ago

Address the height discrepancy on desktop & mobile, and increase width of color picker result slightly on desktop so hex code isn't cut off.

#6 @garrett-eclipse
5 years ago

  • Keywords needs-testing needs-design-feedback added; needs-refresh removed

Sounds good @audrasjb attached 48131.2.diff to address this, it has three changes;

  1. Make height consistent on desktop
  2. Unset on mobile so height stays consistent
  3. Increase the .color-picker-hex width so it's contents don't get cutoff.

Marking for testing, and would like a designer to review to ensure there's no unforeseen issues.

#7 @afercia
5 years ago

Thanks all :) I think this would be better fixed by keeping the current height value and adding min-height: 0 to override the newly inherited min height.

Seems to me a broader approach would be by reviewing the CSS "small" classes for inputs and buttons. @kjellr do you think it would make sense to adjust them in order to have a height of 24 pixels?

In this specific case:

  • the input field could use the small-text class
  • the button-small class should be slightly adjusted to have a total height of 24 pixels.

For reference, I'm attaching a screenshot of the color picker input and buttons on WP 5.2.3 with Twenty Sixteen active.

@afercia
5 years ago

#8 @afercia
5 years ago

  • Keywords 5-3-admin-css-changes added

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


5 years ago

@afercia
5 years ago

@afercia
5 years ago

#10 @afercia
5 years ago

  • Keywords needs-testing needs-design-feedback removed

Thanks everyone!

There's the need to fix some line-heights with better consistency with the latest CSS for the input fields. I went ahead and prepared color picker.png.

Tested on the Custom Background / Header legacy pages and in the Customiser. Chrome and Firefox. It now also scales decently with text zoom.

See screenshot above.

#11 @afercia
5 years ago

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

In 46360:

Accessibility: Improve and modernize user interface controls: Improve new styles for the color picker.

Props audrasjb, garrett-eclipse.
Fixes #48131.

Note: See TracTickets for help on using tickets.