From 37f5b42806e4d1452e9c545844b67b6f6af4fc4d Mon Sep 17 00:00:00 2001
From: Moshe Zemah <moshe@monday.com>
Date: Mon, 7 Jun 2021 13:48:00 +0300
Subject: [PATCH] Combobox fixes (#140)

---
 .../BreadcrumbContent/BreadcrumbContent.scss  |  1 +
 src/components/Combobox/Combobox.jsx          | 11 +++---
 src/components/Combobox/Combobox.scss         |  6 +++-
 .../Combobox/__stories__/combobox.stories.js  | 35 +++++++++++++------
 src/styles/states.scss                        |  1 -
 5 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/src/components/BreadcrumbsBar/BreadcrumbItem/BreadcrumbContent/BreadcrumbContent.scss b/src/components/BreadcrumbsBar/BreadcrumbItem/BreadcrumbContent/BreadcrumbContent.scss
index dad20db1f6..54d74942a3 100644
--- a/src/components/BreadcrumbsBar/BreadcrumbItem/BreadcrumbContent/BreadcrumbContent.scss
+++ b/src/components/BreadcrumbsBar/BreadcrumbItem/BreadcrumbContent/BreadcrumbContent.scss
@@ -45,5 +45,6 @@
 
   &.disabled {
     @include disabled;
+    pointer-events: none;
   }
 }
diff --git a/src/components/Combobox/Combobox.jsx b/src/components/Combobox/Combobox.jsx
index 1584a9de1a..dd5586b104 100644
--- a/src/components/Combobox/Combobox.jsx
+++ b/src/components/Combobox/Combobox.jsx
@@ -20,7 +20,7 @@ const renderOption = (index, option, isActive, isActiveByKeyboard, onOptionClick
       role="listitem"
       ariaLabel={ariaLabel}
       id={`combox-item-${id}`}
-      onMouseEnter={onOptionHover}
+      onMouseEnter={!disabled && onOptionHover}
       onClick={event => onOptionClick(event, index, option, true)}
       className={cx("combobox-option", {
         disabled,
@@ -73,6 +73,7 @@ const Combobox = forwardRef(
 
     const onOptionClick = useCallback(
       (_event, index, option, mouseClick) => {
+        if (option.disabled) return;
         onClick && onClick(option);
         setActiveItemIndex(index);
         if (mouseClick) {
@@ -99,7 +100,7 @@ const Combobox = forwardRef(
       return filterdOptions.map((option, index) => {
         return renderOption(index, option, activeItemIndex === index, isActiveByKeyboard, onOptionClick, onOptionHover);
       });
-    }, [options, filterValue, activeItemIndex, isActiveByKeyboard, onOptionClick, onOptionHover]);
+    }, [filterdOptions, activeItemIndex, isActiveByKeyboard, onOptionClick, onOptionHover]);
 
     const onChangeCallback = useCallback(
       value => {
@@ -114,7 +115,9 @@ const Combobox = forwardRef(
 
     const onAddNewCallback = useCallback(() => {
       onAddNew && onAddNew(filterValue);
-    }, [onAddNew, filterValue]);
+      // clear filter after adding
+      setFilterValue("");
+    }, [onAddNew, filterValue, setFilterValue]);
 
     useListKeyboardNavigation(
       inputRef,
@@ -156,7 +159,7 @@ const Combobox = forwardRef(
         ref={mergedRef}
         role="listbox"
         aria-activedescendant={`combobox-item-${id}`}
-        className={cx("combobox--wrapper", className)}
+        className={cx("combobox--wrapper", className, { empty: !hasResults })}
         id={id}
       >
         <Search
diff --git a/src/components/Combobox/Combobox.scss b/src/components/Combobox/Combobox.scss
index b85def86dd..1b24016ca5 100644
--- a/src/components/Combobox/Combobox.scss
+++ b/src/components/Combobox/Combobox.scss
@@ -11,6 +11,10 @@ $icon-margin: 4px;
   flex-direction: column;
   @include font-general-text();
 
+  &.empty {
+    max-height: 200px;
+  }
+
   &-search {
     flex: 0 0 auto;
   }
@@ -68,7 +72,7 @@ $icon-margin: 4px;
         @include theme-prop(background-color, primary-selected-color);
       }
 
-      &:hover:not(.disabled),
+      &:hover:not(.disabled):not(.selected),
       &.active {
         @include theme-prop(color, primary-text-color);
         @include theme-prop(background-color, primary-background-hover-color);
diff --git a/src/components/Combobox/__stories__/combobox.stories.js b/src/components/Combobox/__stories__/combobox.stories.js
index 22d5bed7b5..7c1e195874 100644
--- a/src/components/Combobox/__stories__/combobox.stories.js
+++ b/src/components/Combobox/__stories__/combobox.stories.js
@@ -1,4 +1,4 @@
-import React, { useState } from "react";
+import React, { useState, useCallback } from "react";
 import { action } from "@storybook/addon-actions";
 import { text, boolean, number, select } from "@storybook/addon-knobs";
 import { withPerformance } from "storybook-addon-performance";
@@ -20,29 +20,44 @@ export const Sandbox = () => (
   </div>
 );
 
-const getOptions = selectedId => {
-  return [
-    { id: "1", label: "first", leftIcon: "fa fa-star-o", selected: selectedId === "1" },
-    { id: "2", label: "second", rightIcon: "fa fa-star-o", selected: selectedId === "2" },
-    { id: "3", label: "disabled", disabled: true, rightIcon: "fa fa-star-o", selected: selectedId === "3" },
-    { id: "4", label: "fourth", selected: selectedId === "4" }
-  ];
+const getOptions = (selectedId, additionalOptions = []) => {
+  const options = [
+    { id: "1", label: "first", leftIcon: "fa fa-star-o" },
+    { id: "2", label: "second", rightIcon: "fa fa-star-o" },
+    { id: "3", label: "disabled", disabled: true, rightIcon: "fa fa-star-o" },
+    { id: "4", label: "fourth" }
+  ].concat(additionalOptions);
+
+  options.forEach(option => {
+    option.selected = option.id === selectedId;
+  });
+  return options;
 };
 
 const ComboboxWrapper = () => {
   const [selectedId, setSelectedId] = useState("2");
+  const [addedItems, setAddedItems] = useState([]);
+
+  const addNewItem = useCallback(
+    filterValue => {
+      const id = (5 + addedItems.length).toString();
+      setAddedItems(addedItems.concat([{ id, label: filterValue }]));
+      setSelectedId(id);
+    },
+    [addedItems, setAddedItems, setSelectedId]
+  );
 
   return (
     <div className="combobox-wrapper">
       <Combobox
         placeholder="Search for content"
-        onAddNew={value => console.log("Add new ", value)}
+        onAddNew={addNewItem}
         addNewLabel={value => `+ Add new ${value}`}
         onClick={option => {
           console.log("Clicked on ", option.label);
           setSelectedId(option.id);
         }}
-        options={getOptions(selectedId)}
+        options={getOptions(selectedId, addedItems)}
       />
     </div>
   );
diff --git a/src/styles/states.scss b/src/styles/states.scss
index 75de3df7fe..e67272f841 100644
--- a/src/styles/states.scss
+++ b/src/styles/states.scss
@@ -4,7 +4,6 @@
   @include theme-prop(border-color, disabled-background-color);
   @include theme-prop(color, disabled-text-color);
   cursor: not-allowed;
-  pointer-events: none;
   &:hover {
     background-color: transparent;
   }