Royal-Navy / design-system

Build web applications that meet the Royal Navy service standards
https://storybook.design-system.navy.digital.mod.uk
Apache License 2.0
103 stars 32 forks source link

feat(NumberInputE): Add float step support #2924

Closed m7kvqbe1 closed 2 years ago

m7kvqbe1 commented 2 years ago

Related issue

Closes #1128

Overview

Add support for stepped floats to NumberInputE.

Link to preview

https://61c1ab613913be2c341df66a--numberinputefloats.netlify.app

Reason

Component would not work correctly with float steps.

Work carried out

Developer notes

If we let users enter typed input and there is a step, should we be validating their typed input?

Not doing so allows them to circumvent the step.

jpveooys commented 2 years ago

Hmm I'm still unable to type a number with a decimal point though (or do we have a separate issue for that?)

m7kvqbe1 commented 2 years ago

Hmm I'm still unable to type a number with a decimal point though (or do we have a separate issue for that?)

Ah ok interesting it lets you add the decimal point in to an existing number but won't accept that character if it's the last character (e.g. 1.). I am investigating this now.

sonarcloud[bot] commented 2 years ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

95.8% 95.8% Coverage
0.0% 0.0% Duplication

k9dh3zij commented 2 years ago
  1. If there's already a decimal point, can we stop the user from typing a second one? (as we're already restricting them from typing any 'invalid' characters)

  2. There seems to be some unexpected rounding going on:

Example:

Expected behaviour (with a step of 0.25):

Unexpected behaviour:

3.

If we let users enter typed input and there is a step, should we be validating their typed input? Not doing so allows them to circumvent the step.

Good question. I think this should be an option. Looking at this : "the value must be a positive number - integer or float -- or the special value any, which means no stepping is implied, and any value is allowed".

I tried 'any' and it defaulted the step to 1 while accepting floating values. However, I don't see why someone shouldn't be able to define a step of (say) 100, but accept typed integers which are not multiples of 100. This feels like bad UX (eg. if valid inputs range from 10,000 to 50,000 we don't want a step of 1, but may want to accept all integers between 10,000 and 50,000 as valid inputs).

m7kvqbe1 commented 2 years ago

The following tickets have been created off the back of @k9dh3zij feedback:

https://github.com/defencedigital/mod-uk-design-system/issues/2948 https://github.com/defencedigital/mod-uk-design-system/issues/2949 https://github.com/defencedigital/mod-uk-design-system/issues/2950