SpinButton default max of 100, misleading optional property

[mattmazzola]

Environment Information

• Package version(s): [email protected]
• Browser and OS versions: Google Chrome is up to date
  Version 78.0.3904.108 (Official Build) (64-bit)

Please provide a reproduction of the bug in a codepen:

There is misleading documentation.
The docs claim the default is 10, but it appears to be 100.
The types also make the property optional implying that if you don't supply a max it will have no max value other than the assumed MAX_INTEGER or something.

The issue is that not giving a max property means the max is 100.

See codepen. Taken from https://developer.microsoft.com/en-us/fabric#/controls/web/spinbutton then removed all extra unecessary code related to bug.

Just type in 2343245 into the input and press tab. It sets to 100.

https://codepen.io/mattmazzola-the-reactor/pen/KKwwgKN?editors=1010

Actual behavior:

Spin button limits input to 100 when it shouldn't. There is other weird behavior where if you type a large number and click outside of the input onto the webpage background the number stays unmodified, but if you enter 2342345 and press tab, enter, or click on another Office Fabric control the input is reset to 100.

This change in behavior based on type of Dismissal reminds me of another bug related to picker dimissal changing. #10892

My work around seems to be forcing highest max possible.
max={Number.MAX_SAFE_INTEGER}

Update: looks like issue is same with min property also limiting to 0

Expected behavior:

Number should be not have a max if the max property is not provided.

Priorities and help requested:

Are you willing to submit a PR to fix? (Yes, No)

Requested priority: (Blocking, High, Normal, Low)

Products/sites affected: (if applicable)
Conversation Learner

[xugao]

This is will be something we will fully address with next major release since removing default values for these props are breaking changes.

Apart of that, our documentation is wrong for default max value. fix is #11380

[mattmazzola]

If I understand correctly the next version won't have this issue because there will be no defaults, but your answer doesn't explicitly state how the next version will deal with the more significant issue of different dismissal behaviors.

Does the next version also normalize the type of dismissal or at least provide a work around?

I would like there to be the same behavior for both scenarios

1. User enters 1234, and then presses tab, then clicks submit
Does not work.
Pressing tab or enter is a common learned behavior of users. This scenario incorrectly resets the value back to the default (100) as if 1234 was never entered.

2. User enters 1234, and then clicks submit
Does work.
When the spin button was blurred this time it didn't get reset and the value remained at 1234.

[micahgodbolt]

one solution to remove the max value is to use your own validation

<SpinButton onValidate={v => v} /> will always return the value passed in by the consumer.

[mattmazzola]

I tried it and it does work although I'll likely keep the min max solution. Over riding the onValidate makes it seem like it would also overwrite the functionality of stripping non number characters and don't want people to be worried about that. Setting min and max seems more clear that it can't do any harm, than setting onValidate to and identity function.

I think the larger concern is the different in behavior between scenarios 1 (tab to blur) and 2 above (click other control).
It would be helpful if someone can confirm if this difference is due to the same issue as mentioned here #10892 where the browser is emitting different events or based on different user paths.

[xugao]

@mattmazzola - sorry for the delayed response, i missed your replies!
on blur, the control would set the value based on min/max. And blur should happen in both scenarios you mentioned. Could you pls provide codepen to demo scenario 2 you had?

[mattmazzola]

I have code sandbox and two videos

https://codesandbox.io/s/spin-button-9ctz2

https://microsoft.sharepoint.com/:v:/t/convsysresearch/EaJGjyk0O1ZErpX7xcLcBxMBiJZEB-GTYPIQmXGUlghUZg?e=7Ny5rx

https://microsoft.sharepoint.com/:v:/t/convsysresearch/EQtJZAnonPNEiPOZqvKsS7QBEg4EffLJh7muxZj3ys6e0A?e=ThcObe

[xugao]

@mattmazzola - thank you for demo the issue! I will open a new issue to track the inconsistent behaviors you raised and fix accordingly.

just to be clear, i won't be able to fixing the original issue you mentioned until our next major release since it is a breaking change

[beyackle]

I'd like to be able to write a SpinButton with no inherent bounds to it. I thought the way to do it was to leave out the min/max properties, but those just set them to be 0 and 100 as default. The problems with tabbing away are secondary to the a11y behavior here. The aria-valuemin and aria-valuemax values are being read out by the screen-reader, which is misleading when the intent is not to have bounds in the first place. Setting the mas to Number.MAX_SAFE_INTEGER is even worse here, because now it's reading out a nonsensically large value as the maximum.

[dzearing]

Recommendation:

We should follow native HTML precedence:

// Renders an uncontrained spinbutton
<input type="number" />

// Renders a constrained spinbutton
<input type="number" min={0} max={100} />

This would be a behavior change (but not an api change) so we'd want to target for v8. I think a majority of scenarios would remain unaffected, and those which are affected are easily fixed.

[msft-github-bot]

🎉This issue was addressed in #16109, which has now been successfully released as @fluentui/[email protected].:tada:

Handy links:

• Release Notes
• Official NPM Release