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

Min/Max property of SpinButton has no effect when providing custom increment/decrement callbacks #12883

Closed
lukasbash opened this issue Apr 27, 2020 · 4 comments
Assignees
Labels
Component: SpinButton Issue Pinged Resolution: Won't Fix Not going to fix an issue due to various factors

Comments

@lukasbash
Copy link

Environment Information

  • Package version(s): 6.213.0
  • Browser and OS versions: Google Chrome 81.0.4044.92 on macOS 10.15.4

Please provide a reproduction of the bug in a codepen:

https://codepen.io/lukasbash/pen/pojPXEV?editors=1010

Actual behavior:

If the min and/or max properties are provided and onIncrement and/or onDecrement are provided, then the min/max values have no actual effect.

Expected behavior:

There are different points of view in my opinion:

  • either the increment/decrement callbacks should not be called if the limit exceeds the max value (this does actually only work if step is provided)
  • or the min/max value is passed to the relative handler (i.e. max to onIncrement and min to onDecrement)
  • or min/max and onIncrement/onDecrement are mutually exclusive (this would actually be my favorite, because how it works now, the first properties indicate uncontrolled, and the second ones a controlled behavior).

Priorities and help requested:

Are you willing to submit a PR to fix? No

Requested priority: Normal

Products/sites affected: -

@aneeshack4
Copy link
Contributor

@lukasbash Thanks for reaching out to us! So, like you said in the 3rd bullet point of expected behavior, min/max having no effect when onIncrement/onDecrement are provided is how these two patterns are mutually exclusive in practice.

@lukasbash
Copy link
Author

@aneeshack4 Thank you for the fast response! Well, if it is intended that it works like this, then this should be clearly documented.
I think it might even make sense to strictly exclude the types via XOR (so if you pass e.g. the min and onDecrement property, it indicates an error.

@aneeshack4
Copy link
Contributor

@joschect Maybe a line could be added about this to the Dos/Donts section of SpinButton's page? Besides that, @lukasbash we're currently focusing on improvements to performance and coherence, as you can see in #12770 detailing our v8 plans so while we don't plan to address this right now, when we get a chance to work on SpinButton in the future, this will be taken into consideration. Thank you for reaching out to us.

@aneeshack4 aneeshack4 added Needs Dev Input Resolution: Won't Fix Not going to fix an issue due to various factors and removed Needs: Attention labels Apr 27, 2020
@msft-github-bot
Copy link
Contributor

@joschect

Gentle ping that this issue needs attention.

@microsoft microsoft locked as resolved and limited conversation to collaborators Jun 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Component: SpinButton Issue Pinged Resolution: Won't Fix Not going to fix an issue due to various factors
Projects
None yet
Development

No branches or pull requests

4 participants