Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 12 months ago

#53115 closed enhancement (fixed)

Twenty Twenty: number inputs have too much padding

Reported by: helgatheviking's profile helgatheviking Owned by: audrasjb's profile audrasjb
Milestone: 6.0 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch commit assigned-for-commit
Focuses: css Cc:

Description

The number input has so much padding that the browser's up/down buttons are not clickable.

With padding, the cursor never turns into a pointer cursor
https://share.getcloudapp.com/2Nuwpo4W

With padding removed, the cursor turns into a pointer and the up/down buttons are clickable
https://share.getcloudapp.com/7KuPWNDO

Note this gif is using WooCommerce.. which is applying a width to the input

.single-product form.cart input {
    width: 5em;
}

However, tweaking the width doesn't seem to have an impact one way or the other.

Attachments (3)

number-input-woocommerce-ltr-firefox.png (9.5 KB) - added by sabernhardt 3 years ago.
WooCommerce number input after patch (LTR) in Firefox
number-input-woocommerce-rtl-firefox.png (10.3 KB) - added by sabernhardt 3 years ago.
RTL number inputs have the increment buttons on the other side
53115.patch (773 bytes) - added by sabernhardt 3 years ago.
slight edit: adding zero before decimal point

Download all attachments as: .zip

Change History (31)

#1 @SergeyBiryukov
3 years ago

  • Component changed from General to Bundled Theme
  • Summary changed from Twenty Twenty number inputs have too much padding to Twenty Twenty: number inputs have too much padding

#2 @sabernhardt
3 years ago

  • Focuses css added

Except for a few random times, I have this problem in Firefox 88 (Windows 10 Home 20H2) when there is padding on the number input. However, I can use the increment buttons in Chromium-based browsers: Chrome, Edge, Opera, Vivaldi.

If this is a Firefox bug, it might be worth considering input[type="number"] {-moz-appearance: textfield} to hide those buttons so no one tries using them unsuccessfully. Using the arrow keys still works.

(Regarding the unrelated issue about the input width, WooCommerce already plans to make that selector more specific.)

#3 @helgatheviking
3 years ago

I was about to re-post this issue because it drives me nuts! Good thing I searched first. :)

Looks like you can set the padding to

padding-right: .5rem;

and the buttons become clickable again.

https://share.getcloudapp.com/6qu8jPX4

I hate SVN with the fire of 10,000 suns so here's a pull request at github:
https://github.com/WordPress/WordPress/pull/551

The RTL is a bit of a guess... I assumed that the number input buttons are on the other side in RTL?

#4 @sabernhardt
3 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 5.8

@sabernhardt
3 years ago

WooCommerce number input after patch (LTR) in Firefox

@sabernhardt
3 years ago

RTL number inputs have the increment buttons on the other side

#5 @sabernhardt
3 years ago

  • Owner set to sabernhardt
  • Status changed from new to accepted

@helgatheviking Thanks for the patch! That works for me, both with the WooCommerce quantity inputs and with a number input within a Custom HTML block. (Yes, the padding adjustment belongs on the other side for RTL because the increment buttons are on the opposite side.)

I tested on Windows browsers: Firefox, Chrome, Edge, Opera, Vivaldi

For consistency, though, it would be good to add a zero before the decimal for these values within both stylesheets:

input[type="number"] {
	padding-right: 0.5rem;
}

@sabernhardt
3 years ago

slight edit: adding zero before decimal point

#6 @sabernhardt
3 years ago

  • Keywords needs-testing added

53115.patch has the same change as PR 551, but with the leading zeros.

#7 @sabernhardt
3 years ago

  • Milestone changed from 5.8 to 5.9

Unless a committer thinks the patch is ready now, this can be tested and reviewed in the next release cycle.

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


2 years ago

#9 @chaion07
2 years ago

Hi @helgatheviking! Thank you so much for reporting this. We discussed this ticket during a recent bug-scrub and based on the discussion, we feel the need for testing to confirm. So far the patch looks good. Eventually it would be ready for commit. Cheers!

Props: @poena

This ticket was mentioned in Slack in #core-test by justinahinon. View the logs.


2 years ago

#11 @justinahinon
2 years ago

I tried to reproduce the initial issue, but was not able to do so.

Environment

  • WordPress:
  • Plugins: WooCommerce
  • Theme: Twenty Twenty
  • OS: macOS Monterey 12.0
  • Browsers: Firefox Developer Edition 95.0b5 & Google Chrome 95.0.4638.69

Testing instructions

  • Install and activate Twenty Twenty Theme
  • Install and activate WooCommerce
  • Create a sample product
  • Go to the product page, and check if you can use the Add to Cart input buttons

#12 @sabernhardt
2 years ago

This was a problem with Firefox/Windows, but I do not experience trouble with the increment buttons using Firefox 94 (or other Windows browsers). @helgatheviking Do you still have trouble with the buttons?

Even if the browser fixed the button issue, it may be worth editing the padding anyway so the buttons are not indented as much or to accommodate large numbers in fixed-width inputs.

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


2 years ago

#14 @sabernhardt
2 years ago

Very little time remains for adding bug fixes to the 5.9 release. If a committer would like to include this in Beta 1, feel free to adjust the milestone.

#15 @sabernhardt
2 years ago

  • Milestone changed from 5.9 to Future Release

#16 @helgatheviking
2 years ago

@sabernhardt Tested today with FireFox 94.0.1 on Windows and can no longer reproduce the bug, but do agree a little less padding to reduce the indentation on the up/down buttons would still be nice.

#17 @sabernhardt
2 years ago

  • Milestone changed from Future Release to 6.0
  • Type changed from defect (bug) to enhancement

The WordPress admin CSS already has something similar in forms.css for small-text and tiny-text inputs.

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


2 years ago

This ticket was mentioned in Slack in #core-test by sabernhardt. View the logs.


2 years ago

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


2 years ago

#21 @ugyensupport
2 years ago

Tested with browsers, it looks good.

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


2 years ago

#23 @nayana123
2 years ago

Tested with cross-browsers, it's looking good now.

#24 @sabernhardt
2 years ago

  • Keywords commit added; needs-testing removed

#25 @sabernhardt
2 years ago

  • Owner sabernhardt deleted
  • Status changed from accepted to assigned

#26 @audrasjb
2 years ago

  • Keywords assigned-for-commit added

Since tests look good, let's commit this right now.

#27 @audrasjb
2 years ago

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

In 53120:

Twenty Twenty: Improve padding for number input type.

This changes improves lateral padding for number input types in Twenty Twenty bundled theme.

Props helgatheviking, sabernhardt, chaion07, poena, justinahinon, ugyensupport, nayana123.
Fixes #53115.

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


12 months ago

Note: See TracTickets for help on using tickets.