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

fix(number-field): negative numbers handled #3673

Merged
merged 9 commits into from Sep 29, 2023

Conversation

blunteshwar
Copy link
Contributor

Description

Number field was not correctly handling negative numbers when step was a floating point number

Related issue(s)

Motivation and context

How has this been tested?

  • Test case 1
    1. Go here
    2. Do this
  • Test case 2
    1. Go here
    2. Do this

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Checklist

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • If my change required a change to the documentation, I have updated the documentation in this pull request.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have reviewed at the Accessibility Practices for this feature, see: Aria Practices

Best practices

This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against main.

@github-actions
Copy link

github-actions bot commented Sep 26, 2023

Tachometer results

Chrome

number-field permalink

Version Bytes Avg Time vs remote vs branch
npm latest 622 kB 227.79ms - 230.98ms - unsure 🔍
-1% - +1%
-1.31ms - +2.66ms
branch 618 kB 227.52ms - 229.90ms unsure 🔍
-1% - +1%
-2.66ms - +1.31ms
-

slider permalink

Version Bytes Avg Time vs remote vs branch
npm latest 461 kB 181.42ms - 184.06ms - unsure 🔍
-1% - +1%
-1.52ms - +1.60ms
branch 457 kB 181.88ms - 183.52ms unsure 🔍
-1% - +1%
-1.60ms - +1.52ms
-
Firefox

number-field permalink

Version Bytes Avg Time vs remote vs branch
npm latest 622 kB 564.31ms - 602.57ms - unsure 🔍
-5% - +4%
-29.86ms - +26.18ms
branch 618 kB 564.81ms - 605.75ms unsure 🔍
-4% - +5%
-26.18ms - +29.86ms
-

slider permalink

Version Bytes Avg Time vs remote vs branch
npm latest 461 kB 447.92ms - 478.04ms - unsure 🔍
-9% - +1%
-43.90ms - +3.38ms
branch 457 kB 465.02ms - 501.46ms unsure 🔍
-1% - +10%
-3.38ms - +43.90ms
-

@Rajdeepc Rajdeepc changed the title fix(number-field): negetive numbers handled fix(number-field): negative numbers handled Sep 26, 2023
@Rajdeepc Rajdeepc added bug Something isn't working Component: Number Field labels Sep 26, 2023
Copy link
Collaborator

@Westbrook Westbrook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty good to me. If you could add a test to confirm that the original issue is now address this will be good to go!

Copy link
Contributor

@Rajdeepc Rajdeepc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blunteshwar if you are able to add a test we will be able to merge this

@blunteshwar
Copy link
Contributor Author

@blunteshwar if you are able to add a test we will be able to merge this

done!

Co-authored-by: Rajdeep Chandra <rajrock38@gmail.com>
@Rajdeepc
Copy link
Contributor

@Westbrook If we are good to go, let's land this 🛩️

Copy link
Collaborator

@Westbrook Westbrook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:!

@Westbrook Westbrook merged commit 62553dd into main Sep 29, 2023
47 checks passed
@Westbrook Westbrook deleted the blunteshwar/numberField-issue-incorrect-value branch September 29, 2023 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants