Combobox: Adding fixes for onChange and onPendingValueChanged callbacks

[khmakoto]

Current Behavior

Right now, selecting an option in a Combobox or writing down an option if freeform is allowed will send undefined as the value of the onChange callback. Furthermore, in the case that state is changed while typing on a freeform Combobox, onPendingValueChanged is being called twice, first with the correct value and then with undefined.

New Behavior

This PR fixes some of these issues by always sending the correct value to the onChange and onPendingValueChanged callbacks. This PR does not fix the issue of onPendingValueChanged being called twice.

Notes

See this codepen for how the previous behavior looks like.

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit cb5f420:

Sandbox	Source
@fluentui/react 8 starter	Configuration
@fluentui/react-components 9 starter	Configuration

[size-auditor]

Asset size changes

Project	Bundle	Baseline Size	New Size	Difference
office-ui-fabric-react	fluentui-react-ComboBox	233.564 kB	233.578 kB	14 bytes
office-ui-fabric-react	fluentui-react-TimePicker	223.017 kB	223.031 kB	14 bytes

 Over Tolerance (1024 B)  Over Baseline  Below Baseline  New   Removed 1 kB = 1000 B

Baseline commit: c7e8d9a5ea96e3e679fcee9f5d7a88f46777bd29 (build)

[fabricteam]

📊 Bundle size report

_{🤖 This report was generated against c7e8d9a5ea96e3e679fcee9f5d7a88f46777bd29}

[fabricteam]

Perf Analysis (@fluentui/react)

Scenario	Render type	Master Ticks	PR Ticks	Iterations	Status
buttonNative	mount	134	132	5000	Possible regression

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations	Status
BaseButton	mount	986	937	5000
Breadcrumb	mount	2761	2759	1000
Checkbox	mount	1561	1534	5000
CheckboxBase	mount	1314	1313	5000
ChoiceGroup	mount	4879	4849	5000
ComboBox	mount	1015	1017	1000
CommandBar	mount	10598	10631	1000
ContextualMenu	mount	12145	12198	1000
DefaultButton	mount	1159	1150	5000
DetailsRow	mount	3889	3974	5000
DetailsRowFast	mount	3917	3915	5000
DetailsRowNoStyles	mount	3704	3668	5000
Dialog	mount	2285	2264	1000
DocumentCardTitle	mount	184	171	1000
Dropdown	mount	3647	3356	5000
FocusTrapZone	mount	1974	1931	5000
FocusZone	mount	1867	1874	5000
IconButton	mount	1774	1824	5000
Label	mount	372	359	5000
Layer	mount	2986	3065	5000
Link	mount	486	486	5000
MenuButton	mount	1544	1505	5000
MessageBar	mount	2284	2217	5000
Nav	mount	3439	3368	1000
OverflowSet	mount	1116	1134	5000
Panel	mount	2222	2224	1000
Persona	mount	881	891	1000
Pivot	mount	1500	1484	1000
PrimaryButton	mount	1324	1339	5000
Rating	mount	7921	7821	5000
SearchBox	mount	1352	1349	5000
Shimmer	mount	2560	2554	5000
Slider	mount	1985	2038	5000
SpinButton	mount	5134	5395	5000
Spinner	mount	442	432	5000
SplitButton	mount	3278	3278	5000
Stack	mount	539	532	5000
StackWithIntrinsicChildren	mount	2377	2364	5000
StackWithTextChildren	mount	5385	5417	5000
SwatchColorPicker	mount	11957	11810	5000
TagPicker	mount	2775	2726	5000
TeachingBubble	mount	99794	99529	5000
Text	mount	454	431	5000
TextField	mount	1416	1449	5000
ThemeProvider	mount	1225	1218	5000
ThemeProvider	virtual-rerender	670	662	5000
ThemeProvider	virtual-rerender-with-unmount	1894	1883	5000
Toggle	mount	819	832	5000
buttonNative	mount	134	132	5000	Possible regression

[smhigley]

The value being the text of the option gets a little weird when you throw in multiSelect (e.g. un-selecting "Option B" still gets you the value "Option B"), or Select All.

I think part of the issue is that for freeform, the onChange value corresponds to the actual typed input value, and not the selected option, so it seems a bit inconsistent.

Not saying the current approach is wrong, but at a minimum we should probably document it well.

[smhigley]

the code changes LGTM though! So if the behavior itself is desired in the multiselect/select all cases, I'll approve 👍