fix: improve the MultiSelect component

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

@@ -15,12 +15,12 @@ describe('MultiSelect base', () => {
     it('should put the input element focused when clicked', () => {
         const input = new PageMultiSelect(MULTI_SELECT);
         input.click();
-        expect(input.hasFocus()).toBe(true);
+        expect(input.hasTriggerFocus()).toBe(true);
     });
 
     it('should put the input element focused when the label element is clicked', () => {
         const input = new PageMultiSelect(MULTI_SELECT);
         input.clickLabel();
-        expect(input.hasFocus()).toBe(true);
+        expect(input.hasTriggerFocus()).toBe(true);
     });
 });

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

@@ -0,0 +1,26 @@
+const PageMultiSelect = require('../../../src/components/MultiSelect/pageObject');
+
+const MULTI_SELECT = '#multiselect-component-11';
+
+describe('MultiSelect base', () => {
+    beforeAll(() => {
+        browser.url('/#!/MultiSelect/11');
+    });
+    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.hasInputFocus()).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.hasInputFocus()).toBe(false);
+    });
+});

src/components/InternalDropdown/index.js

@@ -164,7 +164,7 @@ const InternalDropdown = forwardRef((props, reference) => {
             if (multiple) {
                 if (Array.isArray(value)) {
                     if (value.some(v => v.name === name)) {
-                        return null;
+                        return onChange(value.filter(v => v.name !== name));
                     }
                     return onChange(value.concat([option]));
                 }
@@ -258,6 +258,7 @@ const InternalDropdown = forwardRef((props, reference) => {
             activeOptionName,
             currentValues,
             activeChildrenMap,
+            multiple,
         };
     }, [
         value,
@@ -267,6 +268,7 @@ const InternalDropdown = forwardRef((props, reference) => {
         activeOptionName,
         activeChildrenMap,
         handleChange,
+        multiple,
     ]);
 
     return (

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, StyledText, StyledButtonIcon } 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>,
@@ -70,14 +85,54 @@ describe('<MultiSelect />', () => {
     it('should fire focus event', () => {
         const mockOnFocus = jest.fn();
         const component = mount(<MultiSelect onFocus={mockOnFocus} />);
-        component.find(StyledInput).simulate('focus');
+        component.find(StyledButtonIcon).simulate('focus');
         expect(mockOnFocus).toHaveBeenCalledTimes(1);
     });
 
     it('should fire blur event', () => {
         const mockOnBlur = jest.fn();
         const component = mount(<MultiSelect onBlur={mockOnBlur} />);
-        component.find(StyledInput).simulate('blur');
+        component.find(StyledButtonIcon).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

@@ -2,23 +2,30 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import { StyledChip } from './styled';
 
-function Chips(props) {
-    const { value, variant, onDelete } = props;
+const Chips = props => {
+    const { value, variant, onDelete, disabled, readOnly } = props;
+    const getDeleteCallback = val => {
+        return disabled || readOnly ? null : () => onDelete(val);
+    };
+
     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 => {
+            return (
+                <StyledChip
+                    key={val.name}
+                    label={val.label}
+                    variant={variant}
+                    onDelete={getDeleteCallback(val)}
+                />
+            );
+        });
     }
-    return <StyledChip label={value.label} variant={variant} onDelete={() => onDelete(value)} />;
-}
+    return <StyledChip label={value.label} variant={variant} onDelete={getDeleteCallback(value)} />;
+};
 
 Chips.propTypes = {
     value: PropTypes.oneOfType([
@@ -34,12 +41,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/content.js

@@ -0,0 +1,50 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import { getContent } from './helpers';
+import { StyledText } from './styled';
+import Chips from './chips';
+
+const Content = props => {
+    const { variant, value, chipVariant, readOnly, disabled, onDelete } = props;
+
+    if (variant === 'chip') {
+        return (
+            <Chips
+                value={value}
+                variant={chipVariant}
+                readOnly={readOnly}
+                disabled={disabled}
+                onDelete={onDelete}
+            />
+        );
+    }
+
+    const content = getContent(value);
+    return <StyledText>{content}</StyledText>;
+};
+
+Content.propTypes = {
+    disabled: PropTypes.bool,
+    readOnly: PropTypes.bool,
+    variant: PropTypes.oneOf(['default', 'chip']),
+    chipVariant: PropTypes.oneOf(['base', 'neutral', 'outline-brand', 'brand']),
+    value: PropTypes.arrayOf(
+        PropTypes.shape({
+            name: PropTypes.string,
+            label: PropTypes.string,
+            value: PropTypes.any,
+        }),
+    ),
+    onDelete: PropTypes.func,
+};
+
+Content.defaultProps = {
+    disabled: false,
+    readOnly: false,
+    variant: 'default',
+    chipVariant: 'base',
+    value: undefined,
+    onDelete: () => {},
+};
+
+export default Content;

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/__test__/hasChips.spec.js → src/components/MultiSelect/helpers/__test__/hasContent.spec.js

@@ -1,17 +1,17 @@
-import hasChips from '../hasChips';
+import hasContent from '../hasContent';
 
-describe('hasChips', () => {
+describe('hasContent', () => {
     it('should return true', () => {
         const values = [{ label: 'Label', name: 'Name' }, [{ label: 'Label', name: 'Name' }]];
         values.forEach(value => {
-            expect(hasChips(value)).toBe(true);
+            expect(hasContent(value)).toBe(true);
         });
     });
 
     it('should return false', () => {
         const values = [undefined, null, 0, false, true, []];
         values.forEach(value => {
-            expect(hasChips(value)).toBe(false);
+            expect(hasContent(value)).toBe(false);
         });
     });
 });

src/components/MultiSelect/helpers/getContent.js

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

src/components/MultiSelect/helpers/hasChips.js → src/components/MultiSelect/helpers/hasContent.js

@@ -1,4 +1,4 @@
-export default function hasChips(value) {
+export default function hasContent(value) {
     if (!value) {
         return false;
     }

src/components/MultiSelect/helpers/index.js

@@ -1,4 +1,6 @@
-import hasChips from './hasChips';
+import hasContent from './hasContent';
 import positionResolver from './positionResolver';
+import getContent from './getContent';
+import normalizeValue from './normalizeValue';
 
-export { hasChips, positionResolver };
+export { hasContent, positionResolver, getContent, normalizeValue };

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;