(7.0) ComboBox: fix multi-select checkbox role; improve docs and examples

change/office-ui-fabric-react-2020-12-30-03-22-23-combobox-checkbox-option-7.json

@@ -0,0 +1,8 @@
+{
+  "type": "patch",
+  "comment": "ComboBox: fix multi-select checkbox role; improve docs and examples",
+  "packageName": "office-ui-fabric-react",
+  "email": "elcraig@microsoft.com",
+  "dependentChangeType": "patch",
+  "date": "2020-12-30T09:22:16.015Z"
+}

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.test.tsx

@@ -4,12 +4,20 @@ import * as ReactTestUtils from 'react-dom/test-utils';
 import { mount, ReactWrapper } from 'enzyme';
 import * as renderer from 'react-test-renderer';
 import { KeyCodes, resetIds } from '../../Utilities';
+import { safeCreate } from '@uifabric/test-utilities';
 
 import { ComboBox, IComboBoxState } from './ComboBox';
 import { IComboBox, IComboBoxOption, IComboBoxProps } from './ComboBox.types';
 import { SelectableOptionMenuItemType } from '../../utilities/selectableOption/SelectableOption.types';
 import { expectOne, expectMissing, renderIntoDocument } from '../../common/testUtilities';
 
+const RENDER_OPTIONS: IComboBoxOption[] = [
+  { key: 'header', text: 'Header', itemType: SelectableOptionMenuItemType.Header },
+  { key: '1', text: 'Option 1' },
+  { key: 'divider', text: '', itemType: SelectableOptionMenuItemType.Divider },
+  { key: '2', text: 'Option 2' },
+];
+
 const DEFAULT_OPTIONS: IComboBoxOption[] = [
   { key: '1', text: '1' },
   { key: '2', text: '2' },
@@ -45,10 +53,16 @@ let domNode: HTMLElement | undefined;
 const createNodeMock = (el: React.ReactElement<{}>) => {
   return {
     __events__: {},
+    addEventListener: () => undefined,
+    removeEventListener: () => undefined,
   };
 };
 
 describe('ComboBox', () => {
+  beforeEach(() => {
+    resetIds();
+  });
+
   afterEach(() => {
     if (wrapper) {
       wrapper.unmount();
@@ -70,9 +84,41 @@ describe('ComboBox', () => {
   });
 
   it('Renders correctly', () => {
-    const component = renderer.create(<ComboBox options={DEFAULT_OPTIONS} text={'testValue'} />, { createNodeMock });
-    const tree = component.toJSON();
-    expect(tree).toMatchSnapshot();
+    safeCreate(
+      <ComboBox options={DEFAULT_OPTIONS} text={'testValue'} />,
+      component => {
+        const tree = component.toJSON();
+        expect(tree).toMatchSnapshot();
+      },
+      { createNodeMock },
+    );
+  });
+
+  it('Renders correctly when open', () => {
+    // Mock createPortal so that the options list ends up inside the wrapper for snapshotting.
+    // Since this is the first test to call mount(), we have to mount something with the real
+    // version of createPortal first to allow some global setup to run properly.
+    wrapper = mount(<div />);
+    wrapper.unmount();
+    spyOn(ReactDOM, 'createPortal').and.callFake(node => node);
+
+    const ref = React.createRef<IComboBox>();
+    wrapper = mount(<ComboBox options={RENDER_OPTIONS} defaultSelectedKey="2" componentRef={ref} />);
+    ref.current!.focus(true);
+    wrapper.update();
+    // Unlike react-test-renderer's toJSON, snapshots of DOM nodes don't include event handlers
+    // and have a few other differences, but it's a decent tradeoff since react-test-renderer
+    // doesn't support refs and makes it hard/impossible to open the ComboBox for a snapshot.
+    expect(wrapper.getDOMNode()).toMatchSnapshot();
+  });
+
+  it('Renders correctly when opened in multi-select mode', () => {
+    spyOn(ReactDOM, 'createPortal').and.callFake(node => node);
+    const ref = React.createRef<IComboBox>();
+    wrapper = mount(<ComboBox multiSelect options={RENDER_OPTIONS} defaultSelectedKey="2" componentRef={ref} />);
+    ref.current!.focus(true);
+    wrapper.update();
+    expect(wrapper.getDOMNode()).toMatchSnapshot();
   });
 
   it('renders with a Keytip correctly', () => {

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx

@@ -1460,7 +1460,7 @@ export class ComboBox extends React.Component<IComboBoxProps, IComboBoxState> {
           onMouseLeave={this._onOptionMouseLeave}
           role="option"
           // aria-selected should only be applied to checked items, not hovered items
-          aria-selected={isSelected ? 'true' : 'false'}
+          aria-selected={isChecked ? 'true' : 'false'}
           ariaLabel={item.ariaLabel}
           disabled={item.disabled}
           title={title}
@@ -1476,20 +1476,23 @@ export class ComboBox extends React.Component<IComboBoxProps, IComboBoxState> {
           id={id + '-list' + item.index}
           ariaLabel={item.ariaLabel}
           key={item.key}
-          data-index={item.index}
           styles={optionStyles}
           className={'ms-ComboBox-option'}
-          data-is-focusable={true}
           onChange={this._onItemClick(item)}
           label={item.text}
-          role="option"
           checked={isChecked}
           title={title}
           disabled={item.disabled}
           // eslint-disable-next-line react/jsx-no-bind
           onRenderLabel={onRenderCheckboxLabel}
           inputProps={{
-            'aria-selected': isSelected ? 'true' : 'false',
+            // aria-selected should only be applied to checked items, not hovered items
+            'aria-selected': isChecked ? 'true' : 'false',
+            role: 'option',
+            ...({
+              'data-index': item.index,
+              'data-is-focusable': true,
+            } as any),
           }}
         />
       );