Implement SpinButton onValidValueUpdated callback.

[lijunle]

Pull request checklist

• Addresses an existing issue: Related to SpinButton: Add onChange handler #5326
• Include a change request file using $ npm run change

Description of changes

Implement SpinButton's onValidValueUpdated callback to notify the parent to control the component..

Focus areas to test

SpinButton component.

Microsoft Reviewers: Open in CodeFlow

[size-auditor]

Component	Baseline Size	PR Size	Size Change	Bundle Size	Tolerance Level
SpinButton	232905	233033	128	🔺	✔️

[jspurlin]

onValidValueUpdated?: (value: string) => void; [](start = 2, length = 46)

Should there be some information on how this should be used taking into account the onIncrement, onDecrement, and onValidate props? Also, how do those props and this one work together? If they don't work together in a sensible way there should be a warn mutually exclusive telling creators not to combine them (potentially as well as something in the documentation here)

[lijunle]

They are not really related. They work together great as the way they work. The onIncrement/onDecrement/onValidate is to inject some customized logic, the onValidValueUpdated is to listen on something happens. I think the doc here is completed, mention something unrelated is not wise.

[JasonGore]

I think the callback naming onValidValueUpdated is pretty complex and speaks to a lot of the architectural issues that SpinButton has (as discussed in #5326.) However, we do have an onValidate, as well as callbacks that use past tense naming to imply an action occurred. Could this be more simply named onValidated?

[lijunle]

@JasonGore I am fine with any naming part. IMO, one difference of onValidValueUpdated and onValidated is do we want to trigger the callback after validated with the same value. Again, I am fine with both.

I totally agree that the name exposes the architectural issue of SpinButton. I could like to help if we have a clear direction on goal. Could you please like to provide your ideas/suggestions to my questions here?

[jspurlin]

[lijunle]

@jspurlin Thank you very much for helping reviewing the changes!

@ecraig12345 Could you please help on taking a look?

[dzearing]

I'm a bit concerned with the naming here. Are we making this component more complicated than it needs to be?

[dzearing]

What I mean:

I expect to make a controlled version of this component like an input element:

I render using value <SpinButton value={12} ... />

The user clicks up arrow

The onChange event fires

The user gets the new value and rerenders, or, the user ignores the update and nothing changes.

Why do we need any more complexity than this?

[dzearing]

Let's hold and discuss this internally first.

[lijunle]

@dzearing The behavior of onChange is unclear yet, see the discussion in #5326 (comment)

If we have a clear view about how onChange callback works, I could like to contribute on it.

[dzearing]

@lijunle when you're back, let's reconvene on this issue. Closing the PR for now.