[base-ui][NumberInput] Implement numberInputReducer

[mj12albert]

Part of #38514

This PR adds a numberInputReducer function that would be used to determine the internal value and inputValue of the useNumberInput hook. (I plan to integrate this with useNumberInput and useControllableReducer in a separate PR)

Action types:

• clamp: apply the clamp function to inputValue
• inputChange: update inputValue
• increment, decrement: step the value up or down based on step, multiplier
• incrementToMax, decrementToMin: directly set the value to max or min if they are set

The logic is mostly extracted from useNumberInput with one notable change – the value can be set to an empty string when the input is completely cleared. Previously it was just undefined but it shouldn't be used as it would change the controlled/uncontrolled state (#38513)

• I have followed (at least) the PR section of the contributing guide.

[mui-bot]

Netlify deploy preview

https://deploy-preview-38723--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 83d7ec4

[mj12albert]

@michaldudak Regarding managing the controlled/uncontrolled state, if a component is using useControllableReducer it wouldn't need to use the state provided by useControlled right? It just needs to show the error

[mj12albert]

I'm leaning towards calling this inputValue again, it's easier to associate with onInputChange

Even though this is just internal for now, I think we also need to provide the option to have inputValue as a controlled state

[michaldudak]

Regarding managing the controlled/uncontrolled state, if a component is using useControllableReducer it wouldn't need to use the state provided by useControlled right?

Right. useControllableReducer is an equivalent of useControlled in a sense that it handles both the controlled and uncontrolled values. You don't need both useControllableReducer and useControlled in the same component.

[mj12albert]

@michaldudak I've updated this ~ in the end I decided to not pass any events into this at all to keep it simpler (I was only using very few properties on the events anyway)

It's also much clearer what each action is going to do:

clamp: 'numberInput:clamp',
inputChange: 'numberInput:inputChange',
increment: 'numberInput:increment',
decrement: 'numberInput:decrement',
decrementToMin: 'numberInput:decrementToMin',
incrementToMax: 'numberInput:incrementToMax',

[michaldudak]

Looking good in general. I've got a couple of remarks but I like the direction!

[michaldudak]

Nit: technically, a "parsed" value would be a number, while here it's just a string representing the value.
BTW, do you intend developers to provide their own implementation of getInputValueAsString? What would be the use case?

[mj12albert]

Ah I haven't fully thought out the terminology yet (including getInputValueAsString), so let me rename parsedValue to numberValueAsString for now to be 100% literal 😄

The use case would be if a user wanted to accept something like $50 in the input, we would expose getInputValueAsString as a prop (with a better name) to accept a custom function to remove the $.

It would also require something like formatNewValueToDisplayValue() to add back the $ when the value is updated.

[michaldudak]

Do you think we need such a two-step process? Wouldn't it be enough if developers could override just the parsing function (that gets whatever string there is in the input and returns a number)?

[mj12albert]

@michaldudak I will benchmark possible alternatives, but with a currency/money input component in mind as a use-case (has currency symbol, decimals, thousands separator), I know this "parse + format" approach will work from redux-form

antd has a very similar api for their InputNumber component, though their example feels a bit weird when you use it: https://codesandbox.io/s/antd-number-input-formatter-and-parser-n74kgs?file=/src/index.js

[michaldudak]

If we used 1 | -1 as direction, we could get rid of some code without sacrificing readability IMO (using an enum would also work).

[mj12albert]

How about '+' | '-' 😬

[michaldudak]

This still requires some logic other than simple multiplication, but if you feel 1 | -1 is not expressive enough, let's leave it.

[mj12albert]

OK I've reverted this bit – it's just that having a number as direction just made me stop and think all the time 😅

[michaldudak]

I like it! Feels like the domain-specific action names look better than event-specific ones. I'm considering refactoring the existing ones in Select and Menu.

[michaldudak]

I like it! Feels like the domain-specific action names look better than event-specific ones. I'm considering refactoring the existing ones in Select and Menu.