WordPress.org

Make WordPress Core

Opened 2 months ago

Closed 2 months ago

#48131 closed defect (bug) (fixed)

Height issue in wp-color-picker

Reported by: audrasjb Owned by: 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:
PR Number:

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 2 months 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 2 months ago.
before
Capture d’écran 2019-09-24 à 22.54.50.png (37.9 KB) - added by audrasjb 2 months ago.
after
Screen Shot 2019-09-24 at 2.43.52 PM.png (21.1 KB) - added by garrett-eclipse 2 months ago.
Varied heights on colour picker inputs/buttons
48131.2.diff (1.0 KB) - added by garrett-eclipse 2 months 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 2 months ago.
48131.3.diff (1.1 KB) - added by afercia 2 months ago.
color picker.png (49.3 KB) - added by afercia 2 months ago.

Download all attachments as: .zip

Change History (19)

@audrasjb
2 months ago

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

#1 @audrasjb
2 months 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
2 months ago

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

#3 @garrett-eclipse
2 months 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
2 months ago

Varied heights on colour picker inputs/buttons

#4 @audrasjb
2 months 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
2 months ago

  • Keywords needs-refresh added

@garrett-eclipse
2 months 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
2 months 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
2 months 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
2 months ago

#8 @afercia
2 months ago

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

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


2 months ago

@afercia
2 months ago

@afercia
2 months ago

#10 @afercia
2 months 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
2 months 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.