fix: react-combobox never uses Option value as a display string

apps/vr-tests-react-components/src/stories/Combobox.stories.tsx

@@ -50,7 +50,7 @@ storiesOf('Combobox Converged', module)
     </Combobox>
   ))
   .addStory('Disabled with value', () => (
-    <Combobox disabled selectedOptions={['text']}>
+    <Combobox disabled value="text">
       <Option>text</Option>
     </Combobox>
   ))
@@ -88,13 +88,6 @@ storiesOf('Combobox Converged', module)
       <Option>text</Option>
     </Combobox>
   ))
-  .addStory('With multiselect value', () => (
-    <Combobox multiselect selectedOptions={['Green', 'Red']}>
-      <Option>Red</Option>
-      <Option>Green</Option>
-      <Option>Blue</Option>
-    </Combobox>
-  ))
   .addStory('Size: small', () => (
     <Combobox size="small">
       <Option>text</Option>
@@ -152,6 +145,24 @@ storiesOf('Combobox Converged', module)
       </Combobox>
     </div>
   ))
+  .addStory('With selection', () => (
+    <div style={{ paddingBottom: '120px' }}>
+      <Combobox selectedOptions={['Red']} open>
+        <Option>Red</Option>
+        <Option>Green</Option>
+        <Option>Blue</Option>
+      </Combobox>
+    </div>
+  ))
+  .addStory('With multiselect selection', () => (
+    <div style={{ paddingBottom: '120px' }}>
+      <Combobox multiselect selectedOptions={['Green', 'Red']} open>
+        <Option>Red</Option>
+        <Option>Green</Option>
+        <Option>Blue</Option>
+      </Combobox>
+    </div>
+  ))
   .addStory('Disabled option', () => (
     <div style={{ paddingBottom: '120px' }}>
       <Combobox open>

apps/vr-tests-react-components/src/stories/Dropdown.stories.tsx

@@ -50,7 +50,7 @@ storiesOf('Dropdown Converged', module)
     </Dropdown>
   ))
   .addStory('Disabled with value', () => (
-    <Dropdown disabled selectedOptions={['text']}>
+    <Dropdown disabled value="text">
       <Option>text</Option>
     </Dropdown>
   ))
@@ -91,7 +91,7 @@ storiesOf('Dropdown Converged', module)
     </Dropdown>
   ))
   .addStory('With multiselect value', () => (
-    <Dropdown multiselect selectedOptions={['Green', 'Red']}>
+    <Dropdown multiselect value="Green, Red" selectedOptions={['Green', 'Red']}>
       <Option>Red</Option>
       <Option>Green</Option>
       <Option>Blue</Option>
@@ -148,6 +148,20 @@ storiesOf('Dropdown Converged', module)
       <Option>Green</Option>
     </Dropdown>
   ))
+  .addStory('With selection', () => (
+    <Dropdown selectedOptions={['Red']} open>
+      <Option>Red</Option>
+      <Option>Green</Option>
+      <Option>Blue</Option>
+    </Dropdown>
+  ))
+  .addStory('With multiselect selection', () => (
+    <Dropdown multiselect selectedOptions={['Green', 'Red']} open>
+      <Option>Red</Option>
+      <Option>Green</Option>
+      <Option>Blue</Option>
+    </Dropdown>
+  ))
   .addStory('Disabled option', () => (
     <Dropdown open>
       <Option disabled>Red</Option>

change/@fluentui-react-combobox-6312fcfc-fc81-4a08-9c2e-b3a654ee49a0.json

@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "fix: update Combobox and Dropdown to never use the Option value string as a display value; input value must be set if selectedOptions or defaultSelectOptions is set\"",
+  "packageName": "@fluentui/react-combobox",
+  "email": "sarah.higley@microsoft.com",
+  "dependentChangeType": "patch"
+}

packages/react-components/react-combobox/src/components/Combobox/Combobox.test.tsx

@@ -407,6 +407,20 @@ describe('Combobox', () => {
     expect(getByTestId('green').getAttribute('aria-checked')).toEqual('false');
   });
 
+  it('should set display value to option text', () => {
+    const { getByRole, getByText } = render(
+      <Combobox defaultOpen>
+        <Option value="r">Red</Option>
+        <Option value="g">Green</Option>
+        <Option value="b">Blue</Option>
+      </Combobox>,
+    );
+
+    userEvent.click(getByText('Green'));
+
+    expect((getByRole('combobox') as HTMLInputElement).value).toEqual('Green');
+  });
+
   it('should change defaultSelectedOptions on click', () => {
     const { getByTestId } = render(
       <Combobox open defaultSelectedOptions={['Green']}>

packages/react-components/react-combobox/src/components/Combobox/useCombobox.tsx

@@ -107,7 +107,7 @@ export const useCombobox_unstable = (props: ComboboxProps, ref: React.Ref<HTMLIn
     // handle selection and updating value if freeform is false
     if (!baseState.open && !freeform) {
       // select matching option, if the value fully matches
-      if (value && activeOption && value.trim().toLowerCase() === activeOption?.value.toLowerCase()) {
+      if (value && activeOption && value.trim().toLowerCase() === activeOption?.text.toLowerCase()) {
         baseState.selectOption(ev, activeOption);
       }
 
@@ -141,11 +141,7 @@ export const useCombobox_unstable = (props: ComboboxProps, ref: React.Ref<HTMLIn
     setFocusVisible(true);
 
     // clear selection for single-select if the input value no longer matches the selection
-    if (
-      !multiselect &&
-      selectedOptions.length === 1 &&
-      (inputValue.length < 1 || selectedOptions[0].indexOf(inputValue) !== 0)
-    ) {
+    if (!multiselect && selectedOptions.length === 1 && (inputValue.length < 1 || !matchingOption)) {
       clearSelection(ev);
     }
   };

packages/react-components/react-combobox/src/components/Dropdown/Dropdown.test.tsx

@@ -417,6 +417,20 @@ describe('Dropdown', () => {
     });
   });
 
+  it('should set display value to option text', () => {
+    const { getByRole, getByText } = render(
+      <Dropdown defaultOpen>
+        <Option value="r">Red</Option>
+        <Option value="g">Green</Option>
+        <Option value="b">Blue</Option>
+      </Dropdown>,
+    );
+
+    fireEvent.click(getByText('Green'));
+
+    expect(getByRole('combobox').textContent).toEqual('Green');
+  });
+
   it('calls onOptionSelect with correct data for multi-select', () => {
     const onOptionSelect = jest.fn();
 

packages/react-components/react-combobox/src/components/Dropdown/useDropdown.tsx

@@ -52,7 +52,7 @@ export const useDropdown_unstable = (props: DropdownProps, ref: React.Ref<HTMLBu
 
   const getNextMatchingOption = (): OptionValue | undefined => {
     // first check for matches for the full searchString
-    let matcher = (optionValue: string) => optionValue.toLowerCase().indexOf(searchString.current) === 0;
+    let matcher = (optionText: string) => optionText.toLowerCase().indexOf(searchString.current) === 0;
     let matches = getOptionsMatchingText(matcher);
     let startIndex = activeOption ? getIndexOfId(activeOption.id) : 0;
 
@@ -71,7 +71,7 @@ export const useDropdown_unstable = (props: DropdownProps, ref: React.Ref<HTMLBu
       // if the search is all the same letter, cycle through options starting with that letter
       if (allSameLetter) {
         startIndex++;
-        matcher = (optionValue: string) => optionValue.toLowerCase().indexOf(letters[0]) === 0;
+        matcher = (optionText: string) => optionText.toLowerCase().indexOf(letters[0]) === 0;
         matches = getOptionsMatchingText(matcher);
       }
     }

packages/react-components/react-combobox/src/utils/OptionCollection.types.ts

@@ -26,7 +26,10 @@ export type OptionCollectionState = {
   getOptionById(id: string): OptionValue | undefined;
 
   /** Returns an array of options filtered by a value matching function against the option's text string. */
-  getOptionsMatchingText(matcher: (value: string) => boolean): OptionValue[];
+  getOptionsMatchingText(matcher: (text: string) => boolean): OptionValue[];
+
+  /** Returns an array of options filtered by a value matching function against the option's value string. */
+  getOptionsMatchingValue(matcher: (value: string) => boolean): OptionValue[];
 
   /** The unordered option data. */
   options: OptionValue[];

packages/react-components/react-combobox/src/utils/Selection.types.ts

@@ -2,7 +2,10 @@ import * as React from 'react';
 import { OptionValue } from './OptionCollection.types';
 
 export type SelectionProps = {
-  /* For an uncontrolled component, sets the initial selection */
+  /**
+   * For an uncontrolled component, sets the initial selection.
+   * If this is set, the `defaultValue` prop MUST also be set.
+   */
   defaultSelectedOptions?: string[];
 
   /**
@@ -19,6 +22,7 @@ export type SelectionProps = {
   /**
    * An array of selected option keys.
    * Use this with `onOptionSelect` to directly control the selected option(s)
+   * If this is set, the `value` prop MUST also be controlled.
    */
   selectedOptions?: string[];
 };

packages/react-components/react-combobox/src/utils/useComboboxBaseState.ts

@@ -22,7 +22,7 @@ export const useComboboxBaseState = (
   } = props;
 
   const optionCollection = useOptionCollection();
-  const { getOptionAtIndex, getOptionsMatchingText } = optionCollection;
+  const { getOptionAtIndex, getOptionsMatchingValue } = optionCollection;
 
   const [activeOption, setActiveOption] = React.useState<OptionValue | undefined>();
 
@@ -56,13 +56,22 @@ export const useComboboxBaseState = (
       return props.defaultValue;
     }
 
+    const selectedOptionsText = getOptionsMatchingValue(optionValue => {
+      return selectedOptions.includes(optionValue);
+    }).map(option => option.text);
+
     if (multiselect) {
       // editable inputs should not display multiple selected options in the input as text
-      return editable ? '' : selectedOptions.join(', ');
+      return editable ? '' : selectedOptionsText.join(', ');
     }
 
-    return selectedOptions[0];
-  }, [controllableValue, editable, isFirstMount, multiselect, props.defaultValue, selectedOptions]);
+    return selectedOptionsText[0];
+
+    // do not change value after isFirstMount changes,
+    // we do not want to accidentally override defaultValue on a second render
+    // unless another value is intentionally set
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [controllableValue, editable, getOptionsMatchingValue, multiselect, props.defaultValue, selectedOptions]);
 
   // Handle open state, which is shared with options in context
   const [open, setOpenState] = useControllableState({
@@ -84,7 +93,7 @@ export const useComboboxBaseState = (
     if (open && !activeOption) {
       // if it is single-select and there is a selected option, start at the selected option
       if (!multiselect && selectedOptions.length > 0) {
-        const selectedOption = getOptionsMatchingText(v => v === selectedOptions[0]).pop();
+        const selectedOption = getOptionsMatchingValue(v => v === selectedOptions[0]).pop();
         selectedOption && setActiveOption(selectedOption);
       }
       // default to starting at the first option

packages/react-components/react-combobox/src/utils/useOptionCollection.ts

@@ -15,16 +15,20 @@ export const useOptionCollection = (): OptionCollectionState => {
       const item = nodes.current.find(node => node.option.id === id);
       return item?.option;
     };
-    const getOptionsMatchingText = (matcher: (value: string) => boolean) => {
+    const getOptionsMatchingText = (matcher: (text: string) => boolean) => {
       return nodes.current.filter(node => matcher(node.option.text)).map(node => node.option);
     };
+    const getOptionsMatchingValue = (matcher: (value: string) => boolean) => {
+      return nodes.current.filter(node => matcher(node.option.value)).map(node => node.option);
+    };
 
     return {
       getCount,
       getOptionAtIndex,
       getIndexOfId,
       getOptionById,
       getOptionsMatchingText,
+      getOptionsMatchingValue,
     };
   }, []);