#479 #477 #395 - Picker - bug fixes (#491)

* fixes for Picker component * fixes for tests * small change * refactored stories for picker component
livechat · Feb 23, 2023 · baf46cb · baf46cb
1 parent 8d7aa78
commit baf46cb
Show file tree

Hide file tree

Showing 7 changed files with 107 additions and 131 deletions.
diff --git a/packages/react-components/src/components/Picker/Picker.stories.tsx b/packages/react-components/src/components/Picker/Picker.stories.tsx
@@ -1,104 +1,122 @@
 import * as React from 'react';
 import { ComponentMeta, Story } from '@storybook/react';
 
-import { IPickerProps, Picker as PickerComponent } from './Picker';
+import { IPickerProps, Picker } from './Picker';
 import { defaultExtendedOptions, defaultOptions } from './constants';
 import { IPickerListItem } from './PickerList';
 
 import './Picker.stories.css';
+import { StoryDescriptor } from '../../stories/components/StoryDescriptor';
 
 export default {
   title: 'Components/Picker',
-  component: PickerComponent,
+  component: Picker,
   parameters: {
     componentSubtitle: `TBD`,
   },
   argTypes: {
     onSelect: { action: 'changed' },
   },
-} as ComponentMeta<typeof PickerComponent>;
+} as ComponentMeta<typeof Picker>;
 
-const StoryTemplate: Story<IPickerProps> = (args: IPickerProps) => {
+const commonWidth: React.CSSProperties = { width: 300 };
+
+const PickerComponent = (args: IPickerProps) => {
   const [selectedItems, setSelectedItems] = React.useState<
     IPickerListItem[] | null
   >(args.selected || null);
 
   return (
-    <div style={{ height: 320 }}>
-      <PickerComponent
-        {...args}
-        selected={selectedItems}
-        onSelect={(items) => setSelectedItems(items)}
-      />
-    </div>
+    <Picker
+      {...args}
+      selected={selectedItems}
+      onSelect={(items) => setSelectedItems(items)}
+    />
   );
 };
 
-export const Picker = StoryTemplate.bind({});
-Picker.args = {
-  options: defaultOptions,
-};
-
-export const PickerWithLabel = StoryTemplate.bind({});
-PickerWithLabel.args = {
-  options: defaultOptions,
-  label: 'Picker',
+const StoryTemplate: Story<IPickerProps> = (args: IPickerProps) => {
+  return (
+    <div style={{ height: 320 }}>
+      <PickerComponent {...args} />
+    </div>
+  );
 };
 
-export const PickerWithError = StoryTemplate.bind({});
-PickerWithError.args = {
+export const Default = StoryTemplate.bind({});
+Default.args = {
   options: defaultOptions,
-  error: 'Error message',
 };
 
-export const DisabledPicker = StoryTemplate.bind({});
-DisabledPicker.args = {
+export const States = (args: IPickerProps): React.ReactElement => (
+  <div style={{ ...commonWidth, marginBottom: 100 }}>
+    <StoryDescriptor title="Basic">
+      <PickerComponent {...args} />
+    </StoryDescriptor>
+    <StoryDescriptor title="Multi select + Basic">
+      <PickerComponent {...args} type="multi" />
+    </StoryDescriptor>
+    <StoryDescriptor title="Disabled">
+      <PickerComponent {...args} disabled />
+    </StoryDescriptor>
+    <StoryDescriptor title="Error">
+      <PickerComponent {...args} error="Example text" />
+    </StoryDescriptor>
+    <StoryDescriptor title="Error + Disabled">
+      <PickerComponent {...args} disabled error="Example text" />
+    </StoryDescriptor>
+    <StoryDescriptor title="Disabled + Selected option">
+      <PickerComponent
+        {...args}
+        disabled
+        selected={[{ key: 'two', name: 'Option two' }]}
+      />
+    </StoryDescriptor>
+    <StoryDescriptor title="Multi select + Disabled + Selected option">
+      <PickerComponent
+        {...args}
+        type="multi"
+        disabled
+        selected={[
+          { key: 'two', name: 'Option two' },
+          { key: 'three', name: 'Option three' },
+        ]}
+      />
+    </StoryDescriptor>
+  </div>
+);
+States.args = {
   options: defaultOptions,
-  disabled: true,
+  label: 'Example label',
 };
 
-export const PickerWithGroupedOptions = StoryTemplate.bind({});
-PickerWithGroupedOptions.args = {
-  options: defaultExtendedOptions,
-};
-
-export const PickerWithDisabledSearch = StoryTemplate.bind({});
-PickerWithDisabledSearch.args = {
-  searchDisabled: true,
-  options: defaultExtendedOptions,
-};
-
-export const PickerWithSelectedOption = StoryTemplate.bind({});
-PickerWithSelectedOption.args = {
-  selected: [{ key: 'two', name: 'Option two' }],
-  options: defaultExtendedOptions,
-};
-
-export const PickerInMultiselectMode = StoryTemplate.bind({});
-PickerInMultiselectMode.args = {
-  type: 'multi',
-  options: defaultExtendedOptions,
-  selectAllOptionText: 'Select all',
-};
-
-export const PickerInMultiselectModeWithSelectedOptions = StoryTemplate.bind(
-  {}
+export const PickerWithGroupedOptions = (
+  args: IPickerProps
+): React.ReactElement => (
+  <div style={{ ...commonWidth, marginBottom: 320 }}>
+    <PickerComponent {...args} />
+  </div>
 );
-PickerInMultiselectModeWithSelectedOptions.args = {
-  selected: [
-    { key: 'two', name: 'Option two' },
-    { key: 'four', name: 'Option four' },
-  ],
-  type: 'multi',
+PickerWithGroupedOptions.args = {
   options: defaultExtendedOptions,
-  selectAllOptionText: 'Select all',
 };
 
 const CustomPickerOption: React.FC = ({ children }) => (
   <div className="custom-picker-option">{children}</div>
 );
 
-export const PickerWithOptionsAsCustomElements = StoryTemplate.bind({});
+export const PickerWithOptionsAsCustomElements = (
+  args: IPickerProps
+): React.ReactElement => (
+  <div style={{ ...commonWidth, marginBottom: 320 }}>
+    <StoryDescriptor title="Single select">
+      <PickerComponent {...args} />
+    </StoryDescriptor>
+    <StoryDescriptor title="Multi select">
+      <PickerComponent {...args} type="multi" />
+    </StoryDescriptor>
+  </div>
+);
 PickerWithOptionsAsCustomElements.args = {
   options: [
     {
@@ -157,65 +175,3 @@ PickerWithOptionsAsCustomElements.args = {
     },
   ],
 };
-
-export const PickerInMultiselectModeWithOptionsAsCustomElements =
-  StoryTemplate.bind({});
-PickerInMultiselectModeWithOptionsAsCustomElements.args = {
-  type: 'multi',
-  options: [
-    {
-      key: 'one',
-      name: 'Example custom element one',
-      customElement: {
-        listItemBody: (
-          <CustomPickerOption>
-            <img
-              className="image"
-              src="https://avatars2.githubusercontent.com/u/29309941?s=88&v=4"
-            />
-            <div>
-              <div className="title">Example custom element one</div>
-              <div className="description">Example custom element</div>
-            </div>
-          </CustomPickerOption>
-        ),
-        selectedItemBody: (
-          <CustomPickerOption>
-            <img
-              className="image selected"
-              src="https://avatars2.githubusercontent.com/u/29309941?s=88&v=4"
-            />
-            <div className="title selected">Example custom element one</div>
-          </CustomPickerOption>
-        ),
-      },
-    },
-    {
-      key: 'two',
-      name: 'Example custom element two',
-      customElement: {
-        listItemBody: (
-          <CustomPickerOption>
-            <img
-              className="image"
-              src="https://avatars2.githubusercontent.com/u/29309941?s=88&v=4"
-            />
-            <div>
-              <div className="title">Example custom element two</div>
-              <div className="description">Example custom element</div>
-            </div>
-          </CustomPickerOption>
-        ),
-        selectedItemBody: (
-          <CustomPickerOption>
-            <img
-              className="image selected"
-              src="https://avatars2.githubusercontent.com/u/29309941?s=88&v=4"
-            />
-            <div className="title selected">Example custom element two</div>
-          </CustomPickerOption>
-        ),
-      },
-    },
-  ],
-};
diff --git a/packages/react-components/src/components/Picker/Picker.tsx b/packages/react-components/src/components/Picker/Picker.tsx
@@ -120,7 +120,7 @@ export const Picker: React.FC<IPickerProps> = ({
       return onSelect(null);
     }
 
-    const newSelectedItems = items.filter((item) =>
+    const newSelectedItems = options.filter((item) =>
       newSelectedItemsKeys.includes(item.key)
     );
 
@@ -208,6 +208,7 @@ export const Picker: React.FC<IPickerProps> = ({
           <TriggerBody
             isOpen={isListOpen}
             isSearchDisabled={searchDisabled}
+            isDisabled={disabled}
             placeholder={placeholder}
             iconSize={tagIconSize}
             items={selected}

diff --git a/packages/react-components/src/components/Picker/PickerList.module.scss b/packages/react-components/src/components/Picker/PickerList.module.scss
@@ -35,7 +35,6 @@ $base-class: 'picker-list';
     cursor: pointer;
     padding: 6px 11px;
     height: 36px;
-    overflow: hidden;
 
     &:hover {
       background-color: var(--surface-basic-hover);
@@ -72,6 +71,13 @@ $base-class: 'picker-list';
       font-weight: 600;
     }
 
+    &__content {
+      flex: 1;
+      overflow: hidden;
+      text-overflow: ellipsis;
+      white-space: nowrap;
+    }
+
     &__custom {
       width: 100%;
       height: auto;

diff --git a/packages/react-components/src/components/Picker/PickerList.spec.tsx b/packages/react-components/src/components/Picker/PickerList.spec.tsx
@@ -53,23 +53,23 @@ describe('<PickerList> component', () => {
   });
 
   it('should mark selected list item as selected', () => {
-    const { getByText } = renderComponent({
+    const { getByTestId } = renderComponent({
       ...defaultProps,
       isOpen: true,
       selectedItemsKeys: ['three'],
     });
 
-    expect(getByText('Option three')).toHaveAttribute('aria-selected', 'true');
+    expect(getByTestId('three')).toHaveAttribute('aria-selected', 'true');
   });
 
   it('should mark selected list item as disabled', () => {
-    const { getByText } = renderComponent({
+    const { getByTestId } = renderComponent({
       ...defaultProps,
       isOpen: true,
       items: [{ key: 'three', name: 'Option three', disabled: true }],
     });
 
-    expect(getByText('Option three')).toHaveAttribute('aria-disabled', 'true');
+    expect(getByTestId('three')).toHaveAttribute('aria-disabled', 'true');
   });
 
   it('should display default empty state if no filter result', () => {

diff --git a/packages/react-components/src/components/Picker/PickerList.tsx b/packages/react-components/src/components/Picker/PickerList.tsx
@@ -224,6 +224,7 @@ export const PickerList: React.FC<IPickerListProps> = ({
 
         return (
           <li
+            data-testid={item.key}
             ref={(element) => {
               if (currentItemKey === item.key) {
                 element?.scrollIntoView({ block: 'nearest' });
@@ -240,7 +241,9 @@ export const PickerList: React.FC<IPickerListProps> = ({
             })}
             onClick={() => !item.disabled && handleOnClick(item)}
           >
-            {getOptionContent(item)}
+            <div className={styles[`${itemClassName}__content`]}>
+              {getOptionContent(item)}
+            </div>
             {isItemSelected(item.key) && <Icon kind="link" source={Check} />}
           </li>
         );

diff --git a/packages/react-components/src/components/Picker/TriggerBody.module.scss b/packages/react-components/src/components/Picker/TriggerBody.module.scss
@@ -6,6 +6,13 @@ $base-class: 'picker-trigger-body';
   gap: 4px;
   justify-content: flex-start;
 
+  &__item {
+    flex: 1;
+    overflow: hidden;
+    text-overflow: ellipsis;
+    white-space: nowrap;
+  }
+
   &__input {
     display: flex;
     flex-grow: 2;

diff --git a/packages/react-components/src/components/Picker/TriggerBody.tsx b/packages/react-components/src/components/Picker/TriggerBody.tsx
@@ -1,4 +1,5 @@
 import * as React from 'react';
+import cx from 'clsx';
 
 import { IPickerListItem } from './PickerList';
 import { Tag } from '../Tag';
@@ -12,6 +13,7 @@ const baseClass = 'picker-trigger-body';
 export interface ITriggerBodyProps {
   isOpen: boolean;
   isSearchDisabled?: boolean;
+  isDisabled?: boolean;
   placeholder: string;
   items?: IPickerListItem[] | null;
   type: PickerType;
@@ -23,6 +25,7 @@ export interface ITriggerBodyProps {
 export const TriggerBody: React.FC<ITriggerBodyProps> = ({
   isOpen,
   isSearchDisabled,
+  isDisabled,
   placeholder,
   items,
   type,
@@ -45,7 +48,7 @@ export const TriggerBody: React.FC<ITriggerBodyProps> = ({
       );
     }
 
-    return item.name;
+    return <div className={styles[`${baseClass}__item`]}>{item.name}</div>;
   };
 
   const handleOnChange = (e: React.ChangeEvent<HTMLInputElement>) => {
@@ -75,7 +78,7 @@ export const TriggerBody: React.FC<ITriggerBodyProps> = ({
                 key={item.name}
                 className={styles[`${baseClass}__tag`]}
                 iconSize={iconSize}
-                dismissible
+                dismissible={!isDisabled}
                 onRemove={() => onItemRemove(item)}
               >
                 {getSingleItem(item)}