WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.4k stars 4.16k forks source link

RangeControl component: disable when value is out of bounds #60633

Open MaggieCabrera opened 5 months ago

MaggieCabrera commented 5 months ago

Description

As reported by @afercia here, The RangeControl component right now is inconsistent with the value next to it if it allows for a different range than the slider does. This is happening right now on spacing controls like margin and padding, since the slider's range is 0-300 but the input allows any value to infinity (and negative numbers possibly soon). Changing the range of the slider is not possible so lets disable it when the value in the input is out of bounds.

Step-by-step reproduction instructions

  1. Insert a group block
  2. Change the margin to 4000, focus the RangeControl slider
  3. the slider shows the value is 300

Screenshots, screen recording, code snippet

319572841-cd4ea15a-2c04-4148-95bc-d3f1a0e51dd0

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

mirka commented 5 months ago

I discussed this with @daniguardiola for a second opinion, and we agreed that this a UI smell that tells us we're using a slider for something that shouldn't use a slider. Like the HTML attribute type="range" suggests, a slider UI is for selecting a value from a defined range, and is inappropriate for an input that doesn't have a defined range. @DaniGuardiola also noted that it could be extra confusing for a user that immediately interacts with the slider, and gets the wrong message that they're not allowed to choose values outside of that range.

If you still want to do some custom ad hoc overriding of component internals to achieve this (e.g. in BoxControl like you've mentioned earlier), let me know and I can help you with that. But we encourage y'all to reconsider showing the slider at all, since it goes against the standard mental model of what a slider is. And for that reason we probably don't want to bake this behavior into our components.

cc @WordPress/gutenberg-design

mirka commented 5 months ago

Also removing the Bug tag, since in the RangeControl component the number input and slider will always maintain the same range, as defined by the min/max props. The behavior and screenshot in the OP is a custom implementation that uses the RangeControl component in slider-only mode, which moves the responsibility of the value/range orchestration to the consumer.

jasmussen commented 5 months ago

a slider UI is for selecting a value from a defined range, and is inappropriate for an input that doesn't have a defined range.

The slider is a greatly ergonomical tool to set margins and paddings, I'd hate for us to have to remove it, that's why I think if you're intentionally typing in negative numbers, or very high numbers that break those bounds, disabling the slider would be a good way to allow both, and adhere to the spirit of the statement. Most of the time you shouldn't have huge values, or use negative values, that's why we'd require typing them in, and providing a bounded range is a useful additional guardrail.

I remember replacing headings with SVG fonts back in the day before web-fonts, and moving the actual text content -9999px to the left to still be legible by screen readers. This is a CSS practice that we want to be able to support. But it seems unreasonable to me to remove the slider as a result.

afercia commented 5 months ago

But we encourage y'all to reconsider showing the slider at all,

I agree with @mirka. UI controls aren't absolute entities that are always good. They are good only when they correctly represent the value they are meant to control. Only in this case they provide a good user experience.

jameskoster commented 5 months ago

Would it be strange if the range min/max updated based on the input value? So if you manually enter -999 then the range would be -999 to 300.

A provocation; should the order of the range / input should be visually flipped? If the slider is supposed to be the more ergonomic control, then shouldn't it be emphasised over the input, which is for edge case/custom values? If this is not the case then I tend to agree it's probably not adding a lot of value.

MaggieCabrera commented 5 months ago

Would it be strange if the range min/max updated based on the input value? So if you manually enter -999 then the range would be -999 to 300.

Then 300 would still be a limit on the max value