carbon-design-system / carbon

A design system built by IBM
https://www.carbondesignsystem.com
Apache License 2.0
7.86k stars 1.81k forks source link

NumberInput component step prop did not support "any" #6930

Open hsharma1-ibm-maximo opened 4 years ago

hsharma1-ibm-maximo commented 4 years ago

What package(s) are you using?

7.12.0 But issue exist on latest version too.

Detailed description

Describe in detail the issue you're having.

The type of prop 'step' should be string and pattern prop should be exposed. 1.HTML5 shows validation message if user put any value into number input which is not a multiple of step For ex: If step=10 and value=10 and user enters 15 or 15.30 then on form submit it would shows validation message as 'Please enter a valid value. The two nearest valid values are 10 and 20' 2.So in this case if user do not want to show any validation message then we have pass step='any'. But in carbon step prop is of number type. So the type should be updated to string so that user could pass 'any' if does not want any client validation. Currently pattern prop is hard coded and it could be exposed as a properties where user can set his own pattern. As we have scenario where user can set decimal values like this step="any" pattern="^\d*(.\d{0,2})?$"

Is this issue related to a specific component? NumberInput

What did you expect to happen? What happened instead? What would you like to see changed? Step should accept "any" and "pattern" should be exposed as a prop. So that user can enter decimal value as per required format without any HTML5 violation.

What browser are you working in? Issue occurs on all browsers.

What version of the Carbon Design System are you using?

What offering/product do you work on? Any pressing ship or release dates we should be aware of?

Steps to reproduce the issue

  1. Enter a decimal value in numberinput and hover mouse over it browser will show invalid error.
  2. Enter a non decimal number which should not be a multiple of step value, and hover mouse over it browser will show invalid error.

Please create a reduced test case in CodeSandbox

Additional information

davidicus commented 4 years ago

I think you could hide the step buttons based off the step value. Is there a reason why Carbon limits this to a number only?

davidicus commented 4 years ago

@hsharma1-ibm-maximo in the meantime you can just pass in a decimal 0.001 and this will allow you to add floating point values without triggering validation error.

tw15egan commented 4 years ago

Most likely an oversight on our part and assuming the step would always be a number. If passing in any allows a user to bypass HTML5 form validation, then it makes sense to change the PropType to accommodate that 👍

Ref https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/number#step

davidicus commented 4 years ago

@tw15egan I have submitted a PR to add this functionality.

hsharma1-ibm-maximo commented 4 years ago

Hi @davidicus , will PR contain changes related to pattern requirement also ?

davidicus commented 4 years ago

Hi @davidicus , will PR contain changes related to pattern requirement also ?

The PR adds the ability to pass in "any" as a value to step.

hsharma1-ibm-maximo commented 4 years ago

As per current component implementaion pattern="^\d*(.\d{0,2})?$" is hardcoded in the render method and there is no provision to override that. We have a requirement where field can have values like 2:00 instead of 2.0, which might work if we have provision to override pattern so that we can provide our own custom pattern, we have mention the pattern requirement in description too.

davidicus commented 4 years ago

As per current component implementaion pattern="^\d*(.\d{0,2})?$" is hardcoded in the render method and there is no provision to override that. We have a requirement where field can have values like 2:00 instead of 2.0, which might work if we have provision to override pattern so that we can provide our own custom pattern, we have mention the pattern requirement in description too.

@hsharma1-ibm-maximo It is my understanding that this prop does not do anything in relation to a number input and should probably be removed. The number input is already restricted to valid floating point values which does not include a colon. This relates to html spec and is not dictated by the Carbon library.

Pattern would only be valid on password, text, or tel type inputs.

image

If you need to support colons then you are probably going to have to use a regular text input and restrict non number input on your own.

brandones commented 4 months ago

It would be great to have support for this. Right now the only way to support decimals in the number field is by setting a decimal step (such as 0.1), which is often undesirable, since it makes the up and down arrows increment/decrement by 0.1.3