WordPress.org

Make WordPress Core

Opened 3 weeks ago

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

Download all attachments as: .zip

Change History (19)

@audrasjb
3 weeks ago

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

#1 @audrasjb
3 weeks 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
3 weeks ago

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

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

Varied heights on colour picker inputs/buttons

#4 @audrasjb
3 weeks 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
3 weeks ago

  • Keywords needs-refresh added

@garrett-eclipse
3 weeks 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
3 weeks 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
3 weeks 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
3 weeks ago

#8 @afercia
3 weeks ago

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

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


2 weeks ago

@afercia
2 weeks ago

@afercia
2 weeks ago

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