fix: improve the MultiSelect component

fix: nexxtway#1645

integration/specs/MultiSelect/multiSelect-9.spec.js

@@ -0,0 +1,26 @@
+const PageMultiSelect = require('../../../src/components/MultiSelect/pageObject');
+
+const MULTI_SELECT = '#multiselect-component-9';
+
+describe('MultiSelect base', () => {
+    beforeAll(() => {
+        browser.url('/#!/MultiSelect/9');
+    });
+    beforeEach(() => {
+        browser.refresh();
+        const component = $(MULTI_SELECT);
+        component.waitForExist();
+    });
+
+    it('should not put the input element focused when clicked', () => {
+        const input = new PageMultiSelect(MULTI_SELECT);
+        input.click();
+        expect(input.hasFocus()).toBe(false);
+    });
+
+    it('should not put the input element focused when the label element is clicked', () => {
+        const input = new PageMultiSelect(MULTI_SELECT);
+        input.clickLabel();
+        expect(input.hasFocus()).toBe(false);
+    });
+});

src/components/MultiSelect/__test__/multiSelect.spec.js

@@ -5,7 +5,7 @@ import Option from '../../Option';
 import HelpText from '../../Input/styled/helpText';
 import ErrorText from '../../Input/styled/errorText';
 import Label from '../../Input/label/labelText';
-import { StyledChip, StyledPlaceholder, StyledInput } from '../styled';
+import { StyledChip, StyledPlaceholder, StyledInput, StyledText } from '../styled';
 
 describe('<MultiSelect />', () => {
     it('should render Label when label prop is passed', () => {
@@ -28,6 +28,21 @@ describe('<MultiSelect />', () => {
         expect(component.find(StyledPlaceholder).exists()).toBe(true);
     });
 
+    it('should render the default variant', () => {
+        const value = [
+            {
+                label: 'First',
+                name: 'first',
+            },
+            {
+                label: 'Second',
+                name: 'second',
+            },
+        ];
+        const component = mount(<MultiSelect value={value} />);
+        expect(component.find(StyledText).exists()).toBe(true);
+    });
+
     it('should render the correct amount of chips', () => {
         const value = [
             {
@@ -40,7 +55,7 @@ describe('<MultiSelect />', () => {
             },
         ];
         const component = mount(
-            <MultiSelect value={value}>
+            <MultiSelect value={value} variant="chip">
                 <Option name="first" label="First" />
                 <Option name="second" label="Second" />
             </MultiSelect>,
@@ -57,7 +72,7 @@ describe('<MultiSelect />', () => {
         ];
         const mockOnChange = jest.fn();
         const component = mount(
-            <MultiSelect value={value} onChange={mockOnChange}>
+            <MultiSelect value={value} variant="chip" onChange={mockOnChange}>
                 <Option name="first" label="First" />
                 <Option name="second" label="Second" />
             </MultiSelect>,
@@ -80,4 +95,44 @@ describe('<MultiSelect />', () => {
         component.find(StyledInput).simulate('blur');
         expect(mockOnBlur).toHaveBeenCalledTimes(1);
     });
+
+    it('should not render the buttons when readOnly', () => {
+        const value = [
+            {
+                label: 'First',
+                name: 'first',
+            },
+            {
+                label: 'Second',
+                name: 'second',
+            },
+        ];
+        const component = mount(
+            <MultiSelect value={value} variant="chip" readOnly>
+                <Option name="first" label="First" />
+                <Option name="second" label="Second" />
+            </MultiSelect>,
+        );
+        expect(component.find('button').exists()).toBe(false);
+    });
+
+    it('should not render the buttons when disabled', () => {
+        const value = [
+            {
+                label: 'First',
+                name: 'first',
+            },
+            {
+                label: 'Second',
+                name: 'second',
+            },
+        ];
+        const component = mount(
+            <MultiSelect value={value} variant="chip" disabled>
+                <Option name="first" label="First" />
+                <Option name="second" label="Second" />
+            </MultiSelect>,
+        );
+        expect(component.find('button').exists()).toBe(false);
+    });
 });

src/components/MultiSelect/chips.js

@@ -3,19 +3,24 @@ import PropTypes from 'prop-types';
 import { StyledChip } from './styled';
 
 function Chips(props) {
-    const { value, variant, onDelete } = props;
+    const { value, variant, onDelete, disabled, readOnly } = props;
     if (!value) {
         return null;
     }
+
     if (Array.isArray(value)) {
-        return value.map(val => (
-            <StyledChip
-                key={val.name}
-                label={val.label}
-                variant={variant}
-                onDelete={() => onDelete(val)}
-            />
-        ));
+        return value.map(val => {
+            const onDeleteCallback = disabled || readOnly ? null : () => onDelete(val);
+
+            return (
+                <StyledChip
+                    key={val.name}
+                    label={val.label}
+                    variant={variant}
+                    onDelete={onDeleteCallback}
+                />
+            );
+        });
     }
     return <StyledChip label={value.label} variant={variant} onDelete={() => onDelete(value)} />;
 }
@@ -34,12 +39,16 @@ Chips.propTypes = {
         ),
     ]),
     variant: PropTypes.oneOf(['base', 'neutral', 'outline-brand', 'brand']),
+    disabled: PropTypes.bool,
+    readOnly: PropTypes.bool,
     onDelete: PropTypes.func,
 };
 
 Chips.defaultProps = {
     value: undefined,
     variant: 'base',
+    disabled: undefined,
+    readOnly: undefined,
     onDelete: () => {},
 };
 

src/components/MultiSelect/helpers/__test__/getContent.spec.js

@@ -0,0 +1,18 @@
+import getContent from '../getContent';
+
+describe('getContent', () => {
+    it('should return null', () => {
+        const values = [false, true, undefined, null];
+        values.forEach(value => {
+            expect(getContent(value)).toBe(null);
+        });
+    });
+
+    it('should return the right string', () => {
+        const values = [{ label: 'Label' }, [{ label: 'Label 1' }, { label: 'Label 2' }]];
+        const expected = ['Label', 'Label 1, Label 2'];
+        values.forEach((value, index) => {
+            expect(getContent(value)).toBe(expected[index]);
+        });
+    });
+});

src/components/MultiSelect/helpers/getContent.js

@@ -0,0 +1,9 @@
+export default function getContent(value) {
+    if (!value || typeof value !== 'object') {
+        return null;
+    }
+    if (Array.isArray(value)) {
+        return value.map(item => item.label).join(', ');
+    }
+    return value.label;
+}

src/components/MultiSelect/index.d.ts

@@ -16,8 +16,9 @@ export interface MultiSelectProps extends BaseProps {
     required?: boolean;
     disabled?: boolean;
     readOnly?: boolean;
-    variant?: 'default' | 'bare';
+    variant?: 'default' | 'chip';
     chipVariant?: 'base' | 'neutral' | 'outline-brand' | 'brand';
+    isBare?: boolean;
     hideLabel?: boolean;
     value?: MultiSelectOption[];
     onChange?: (value: MultiSelectOption[]) => void;

src/components/MultiSelect/index.js

@@ -7,6 +7,7 @@ import {
     useErrorMessageId,
     useReduxForm,
     useLabelId,
+    useWindowResize,
 } from '../../libs/hooks';
 import {
     StyledInput,
@@ -15,6 +16,7 @@ import {
     StyledButtonIcon,
     StyledPlaceholder,
     StyledCombobox,
+    StyledText,
 } from './styled';
 import InternalDropdown from '../InternalDropdown';
 import InternalOverlay from '../InternalOverlay';
@@ -26,6 +28,7 @@ import { ENTER_KEY, SPACE_KEY, ESCAPE_KEY, TAB_KEY } from '../../libs/constants'
 import { hasChips, positionResolver } from './helpers';
 import Chips from './chips';
 import normalizeValue from './helpers/normalizeValue';
+import getContent from './helpers/getContent';
 
 const MultiSelect = React.forwardRef((props, ref) => {
     const {
@@ -40,9 +43,10 @@ const MultiSelect = React.forwardRef((props, ref) => {
         required,
         disabled,
         readOnly,
-        tabIndex,
+        tabIndex: tabIndexInProps,
         variant,
         chipVariant,
+        isBare,
         value,
         onChange,
         onFocus,
@@ -55,13 +59,13 @@ const MultiSelect = React.forwardRef((props, ref) => {
     const comboboxRef = useRef();
     useImperativeHandle(ref, () => ({
         focus: () => {
-            comboboxRef.current.focus();
+            triggerRef.current.focus();
         },
         click: () => {
-            comboboxRef.current.click();
+            triggerRef.current.click();
         },
         blur: () => {
-            comboboxRef.current.blur();
+            triggerRef.current.blur();
         },
     }));
 
@@ -77,7 +81,7 @@ const MultiSelect = React.forwardRef((props, ref) => {
         setIsOpen(false);
         // eslint-disable-next-line no-use-before-define
         stopListeningOutsideClick();
-        comboboxRef.current.focus();
+        setTimeout(() => triggerRef.current.focus(), 0);
     };
 
     const handleChange = val => {
@@ -141,8 +145,22 @@ const MultiSelect = React.forwardRef((props, ref) => {
         dropdownRef,
         handleOutsideClick,
     );
+    useWindowResize(() => setIsOpen(false), isOpen);
 
     const shouldRenderChips = hasChips(value);
+    const tabIndex = disabled || readOnly ? '-1' : tabIndexInProps;
+    const content =
+        variant === 'chip' ? (
+            <Chips
+                value={value}
+                variant={chipVariant}
+                readOnly={readOnly}
+                disabled={disabled}
+                onDelete={handleDelete}
+            />
+        ) : (
+            <StyledText>{getContent(value)}</StyledText>
+        );
 
     return (
         <StyledContainer id={id} className={className} style={style}>
@@ -155,7 +173,7 @@ const MultiSelect = React.forwardRef((props, ref) => {
             />
             <StyledCombobox
                 id={comboboxId}
-                variant={variant}
+                isBare={isBare}
                 error={error}
                 disabled={disabled}
                 role="combobox"
@@ -164,7 +182,7 @@ const MultiSelect = React.forwardRef((props, ref) => {
                 onFocus={onFocus}
                 onBlur={onBlur}
                 onKeyDown={handleKeyDown}
-                tabIndex={tabIndex}
+                tabIndex="-1"
                 ref={comboboxRef}
                 aria-labelledby={labelId}
             >
@@ -181,20 +199,20 @@ const MultiSelect = React.forwardRef((props, ref) => {
                     <RenderIf isTrue={!shouldRenderChips}>
                         <StyledPlaceholder>{placeholder}</StyledPlaceholder>
                     </RenderIf>
-                    <RenderIf isTrue={shouldRenderChips}>
-                        <Chips value={value} variant={chipVariant} onDelete={handleDelete} />
-                    </RenderIf>
+                    <RenderIf isTrue={shouldRenderChips}>{content}</RenderIf>
                 </StyledChipContainer>
-                <StyledButtonIcon
-                    title="Add"
-                    variant="neutral"
-                    size="small"
-                    icon={<PlusIcon />}
-                    onClick={handleTriggerClick}
-                    disabled={disabled}
-                    ref={triggerRef}
-                    tabIndex="-1"
-                />
+                <RenderIf isTrue={!readOnly && !disabled}>
+                    <StyledButtonIcon
+                        title="Add"
+                        variant="neutral"
+                        size="small"
+                        icon={<PlusIcon />}
+                        onClick={handleTriggerClick}
+                        disabled={disabled}
+                        ref={triggerRef}
+                        tabIndex={tabIndex}
+                    />
+                </RenderIf>
                 <InternalOverlay
                     isVisible={isOpen}
                     positionResolver={positionResolver}
@@ -250,9 +268,11 @@ MultiSelect.propTypes = {
     /** Specifies the tab order of an element (when the tab button is used for navigating). */
     tabIndex: PropTypes.oneOfType([PropTypes.number, PropTypes.string]),
     /** Specifies the variant for the input. */
-    variant: PropTypes.oneOf(['default', 'bare']),
+    variant: PropTypes.oneOf(['default', 'chip']),
     /** Specifies the variant for the chips. */
     chipVariant: PropTypes.oneOf(['base', 'neutral', 'outline-brand', 'brand']),
+    /** Specifies that an input will not have border. This value defaults to false. */
+    isBare: PropTypes.bool,
     /** Specifies the value of an input element. */
     value: PropTypes.arrayOf(
         PropTypes.shape({
@@ -288,6 +308,7 @@ MultiSelect.defaultProps = {
     tabIndex: 0,
     variant: 'default',
     chipVariant: 'base',
+    isBare: false,
     value: undefined,
     onChange: () => {},
     onFocus: () => {},