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.

Sign up for GitHub

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

Jump to bottom

fix: number-field range coercion and step constrols #5289

Merged
merged 5 commits into from Oct 22, 2021
Merged

fix: number-field range coercion and step constrols #5289

merged 5 commits into from Oct 22, 2021

Conversation

robarbms
Copy link
Contributor

Pull Request

📖 Description

Fixes for:

  • Number-field trying to coerce field value between min an max while typing. Now happens on blur.
  • Step up and step buttons not working.
  • Better processing of step functions. step up will start at the min and step down will start at the max.
  • Cleaned up all tests.
  • Removed unnecessary dependency on displayText.

🎫 Issues

👩‍💻 Reviewer Notes

📑 Test Plan

✅ Checklist

General

  • I have included a change request file using $ yarn change
  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

⏭ Next Steps

@robarbms robarbms self-assigned this Oct 18, 2021
@robarbms robarbms mentioned this pull request Oct 18, 2021
Closed
9 tasks
This was linked to issues Oct 19, 2021
NumberField inc/dec buttons don't work anymore #5246
Closed
fix: range coercion in number field makes data entry difficult #5249
Closed
@EisenbergEffect EisenbergEffect mentioned this pull request Oct 20, 2021
Closed
@hkusulja
Copy link

Any progress on this approval?

chrisdholt
chrisdholt approved these changes Oct 21, 2021
View reviewed changes
Copy link
Member

@chrisdholt chrisdholt left a comment

Choose a reason for hiding this comment

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

Thanks @robarbms!

@hkusulja
Copy link

@EisenbergEffect please check about 5 pending reviewers...

@chrisdholt
Copy link
Member

@EisenbergEffect please check about 5 pending reviewers...

I just sent this to @EisenbergEffect as well for a sanity check. We'll publish this out today once we get things in. Thanks for your patience!

@robarbms robarbms merged commit c1d594d into master Oct 22, 2021
@robarbms robarbms deleted the users/robarb/number-field-steps branch October 22, 2021 15:52
@vnbaaij vnbaaij mentioned this pull request Oct 23, 2021
Closed
@chigivigi chigivigi mentioned this pull request Oct 27, 2021
Closed
nicholasrice pushed a commit that referenced this pull request Nov 17, 2021
@robarbms @chrisdholt @nicholasrice
fix: number-field range coercion and step constrols (#5289)
061d178 
* Improving number field interactions and tests

* Change files

* Updating documentation

Co-authored-by: Chris Holt <chhol@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EisenbergEffect EisenbergEffect EisenbergEffect approved these changes

@scomea scomea scomea approved these changes

@chrisdholt chrisdholt chrisdholt approved these changes

@eljefe223 eljefe223 eljefe223 approved these changes

@awentzel awentzel Awaiting requested review from awentzel

@Falkicon Falkicon Awaiting requested review from Falkicon

@janechu janechu Awaiting requested review from janechu janechu is a code owner

@nicholasrice nicholasrice Awaiting requested review from nicholasrice

Assignees

@robarbms robarbms

Labels
None yet
Projects
None yet
Milestone
No milestone
7 participants
@robarbms @hkusulja @chrisdholt @EisenbergEffect @nicholasrice @scomea @eljefe223