Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: NumberField does not reflect the correct value in the UI when value is a negative floating number and step is a floating number #3508

Closed
1 task done
ckrac opened this issue Aug 3, 2023 · 2 comments · Fixed by #3673
Assignees

Comments

@ckrac
Copy link

ckrac commented Aug 3, 2023

Code of conduct

  • I agree to follow this project's code of conduct.

Impacted component(s)

sp-number-field

Expected behavior

The value passed to the value property should be reflected in the UI without any rounding.

Actual behavior

When the value is a negative floating number (eg. -2.4) and step is set to a float number, the value in the UI is rounded.

Screenshots

Screen Shot 2023-08-03 at 12 52 50 PM

What browsers are you seeing the problem in?

all browsers

How can we reproduce this issue?

  1. Go to https://opensource.adobe.com/spectrum-web-components/storybook/index.html?path=/story/number-field--default
  2. Update the step property value to 0.001 under storybook controls
  3. Update the value property value to -2.4 or any negative floating number
  4. The value shown in the UI is different than the actual value entered

Sample code that illustrates the problem

No response

Logs taken while reproducing problem

No response

@ckrac ckrac added bug Something isn't working triage An issue needing triage labels Aug 3, 2023
@ckrac ckrac changed the title [Bug]: [Bug]: NumberField does not reflect the correct value in the UI when value is a negative floating number and step is a floating number Aug 3, 2023
@najikahalsema
Copy link
Collaborator

While JS does not have floating point numbers and has issues with precision, but usually not to this low of a decimal place. We should check to see how we're rounding these numbers, because the output seems like it's using the ceil() function.

@najikahalsema najikahalsema added Spectrum Engineering and removed triage An issue needing triage labels Sep 19, 2023
@hunterloftis
Copy link
Contributor

This section has a few places that may not account for negatives:

https://github.com/adobe/spectrum-web-components/blob/main/packages/number-field/src/NumberField.ts#L474-L491

moduloStep is built with JS's remainder operator, which isn't quite a modulo operator and has different behavior with negatives:

overUnder is only checking for positive 1:

I'd recommend writing a few tests to check different combinations of steps and values to make sure they all fall out appropriately.

@Rajdeepc Rajdeepc linked a pull request Sep 20, 2023 that will close this issue
13 tasks
@blunteshwar blunteshwar self-assigned this Sep 25, 2023
@blunteshwar blunteshwar linked a pull request Sep 26, 2023 that will close this issue
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants