#1074 fix bug when editing filters

- onSelectionChange on ExpandoCombobox only gets called when a selection changes, which cause issues when someone tries to select the already selected item from the list dropdown. With this change I've provided a onListItemSelect handler which is same as onSelect on Lists. - Also, text selection used to `appear` blank in ExpandoCombobox because the background & text both used to change to white on selection. Changed the background color on selection to a shade of blue.
finos · Jan 12, 2024 · f49eb81 · f49eb81
1 parent 6ed4a5b
commit f49eb81
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 73 deletions.
diff --git a/vuu-ui/packages/vuu-filters/src/filter-clause/ExpandoCombobox.css b/vuu-ui/packages/vuu-filters/src/filter-clause/ExpandoCombobox.css
@@ -1,38 +1,37 @@
 .vuuExpandoCombobox {
-    --expando-combobox-height: var(--vuuExpandoCombobox-height, 24px);
-    --expando-combobox-fontSize: var(--vuuExpandoCombobox-fontSizew, 12px);
+  --expando-combobox-height: var(--vuuExpandoCombobox-height, 24px);
+  --expando-combobox-fontSize: var(--vuuExpandoCombobox-fontSizew, 12px);
 
-    --saltInput-outline: none;
-    --saltInput-fontSize: var(--expando-combobox-fontSize);
-    --saltInput-height: var(--expando-combobox-height);
-    --saltInput-minWidth: 4px;
-
-    display: inline-flex;
-    flex-direction: column;
-    height: var(--expando-combobox-height);
-    min-width: 4px;
-    padding: 0;
+  --saltInput-outline: none;
+  --saltInput-fontSize: var(--expando-combobox-fontSize);
+  --saltInput-height: var(--expando-combobox-height);
+  --saltInput-minWidth: 4px;
 
+  display: inline-flex;
+  flex-direction: column;
+  height: var(--expando-combobox-height);
+  min-width: 4px;
+  padding: 0;
 }
 
 .vuuExpandoCombobox .saltInput {
-    background-color: transparent;
-    position: absolute;
+  background-color: transparent;
+  position: absolute;
 }
 
 .vuuExpandoCombobox .vuuDropdown {
-    height: 100%;
+  height: 100%;
 }
 
 /** double up the selector just to increase specificity, won't need when we use cascade layers */
 .vuuExpandoCombobox-Input.saltInput {
-    border: none;
-    left:0px;
-    margin:0;
-    min-height: 100%;
-    padding:0;
-    right: 0px;
-    width: auto;
+  border: none;
+  left: 0px;
+  margin: 0;
+  min-height: 100%;
+  padding: 0;
+  right: 0px;
+  width: auto;
 }
 
 .vuuExpandoCombobox .saltInput-input {
@@ -41,20 +40,26 @@
   display: block;
   flex: 1;
   font: inherit;
-  margin:0;
-  min-width:0;
+  margin: 0;
+  min-width: 0;
   outline: none;
   padding: 0;
-
+}
+
+.vuuExpandoCombobox .saltInput-input::selection {
+  background-color: var(
+    --salt-text-background-selected,
+    var(--vuu-color-blue-40)
+  );
 }
 
 .vuuExpandoCombobox:before {
-    content: attr(data-text);
-    display: block;
-    font-size: var(--expando-combobox-fontSize);
-    height: 0px;
-    overflow: hidden;
-    /* visibility: hidden; */
-    white-space: pre-wrap;
-    /* position: absolute; */
-  }
+  content: attr(data-text);
+  display: block;
+  font-size: var(--expando-combobox-fontSize);
+  height: 0px;
+  overflow: hidden;
+  /* visibility: hidden; */
+  white-space: pre-wrap;
+  /* position: absolute; */
+}
diff --git a/vuu-ui/packages/vuu-filters/src/filter-clause/FilterClauseEditor.tsx b/vuu-ui/packages/vuu-filters/src/filter-clause/FilterClauseEditor.tsx
@@ -50,8 +50,8 @@ export const FilterClauseEditor = ({
     onClear,
     onClearKeyDown,
     onDeselectValue,
-    onSelectionChangeColumn,
-    onSelectionChangeOperator,
+    onColumnSelect,
+    onOperatorSelect,
     operator,
     operatorRef,
     selectedColumn,
@@ -129,7 +129,7 @@ export const FilterClauseEditor = ({
         data-field="column"
         initialHighlightedIndex={0}
         itemToString={(column) => column.name}
-        onSelectionChange={onSelectionChangeColumn}
+        onListItemSelect={onColumnSelect}
         ref={columnRef}
         source={columns}
         title="column"
@@ -144,7 +144,7 @@ export const FilterClauseEditor = ({
           })}
           data-field="operator"
           initialHighlightedIndex={0}
-          onSelectionChange={onSelectionChangeOperator}
+          onListItemSelect={onOperatorSelect}
           ref={operatorRef}
           source={getOperators(selectedColumn)}
           title="operator"

diff --git a/vuu-ui/packages/vuu-filters/src/filter-clause/useFilterClauseEditor.ts b/vuu-ui/packages/vuu-filters/src/filter-clause/useFilterClauseEditor.ts
@@ -197,10 +197,10 @@ export const useFilterClauseEditor = ({
     getFilterClauseValue(filterClause)
   );
 
-  const handleSelectionChangeColumn = useCallback<
+  const handleColumnSelect = useCallback<
     SingleSelectionHandler<ColumnDescriptor>
   >(
-    (evt, column) => {
+    (_, column) => {
       setSelectedColumn(column ?? undefined);
       setOperator(undefined);
       setValue(undefined);
@@ -242,8 +242,8 @@ export const useFilterClauseEditor = ({
     [onCancel, setOperator]
   );
 
-  const handleSelectionChangeOperator = useCallback<SingleSelectionHandler>(
-    (evt, selected) => {
+  const handleOperatorSelect = useCallback<SingleSelectionHandler>(
+    (_, selected) => {
       const op = selected;
       if (op === undefined || isValidFilterClauseOp(op)) {
         setOperator(op);
@@ -293,35 +293,17 @@ export const useFilterClauseEditor = ({
         // If value is valid, move on to next field
         const input = evt.target as HTMLInputElement;
         const field = input.closest("[data-field]") as HTMLElement;
-        if (field.dataset.field === "column") {
-          const column = findColumn(input.value);
-          if (column) {
-            setSelectedColumn(column);
-            focusNextElement();
-          }
-        } else if (field.dataset.field === "value") {
-          if (operator === "starts") {
-            // // don't let this bubble to the Toolbar, it would be
-            // // interpreted as selection
-            evt.stopPropagation();
-            const newValue = input.value;
-            setValue(newValue);
-            onChange({
-              column: selectedColumn?.name,
-              op: operator,
-              value: newValue,
-            });
-          }
+        if (field.dataset.field === "value" && operator === "starts") {
+          // // don't let this bubble to the Toolbar, it would be
+          // // interpreted as selection
+          evt.stopPropagation();
+          const newValue = input.value;
+          setValue(newValue);
+          handleChangeValue(newValue);
         }
       }
     },
-    [
-      findColumn,
-      onChange,
-      operator,
-      removeAndNavigateToNextInputIfAtBoundary,
-      selectedColumn?.name,
-    ]
+    [handleChangeValue, operator, removeAndNavigateToNextInputIfAtBoundary]
   );
 
   const handleClear = useCallback(
@@ -371,8 +353,8 @@ export const useFilterClauseEditor = ({
     onClear: handleClear,
     onClearKeyDown: handleClearKeyDown,
     onDeselectValue: handleDeselectValue,
-    onSelectionChangeColumn: handleSelectionChangeColumn,
-    onSelectionChangeOperator: handleSelectionChangeOperator,
+    onColumnSelect: handleColumnSelect,
+    onOperatorSelect: handleOperatorSelect,
     operator,
     operatorRef,
     selectedColumn,

diff --git a/vuu-ui/packages/vuu-ui-controls/src/combo-box/ComboBox.tsx b/vuu-ui/packages/vuu-ui-controls/src/combo-box/ComboBox.tsx
@@ -15,7 +15,7 @@ import {
   useCollectionItems,
 } from "../common-hooks";
 import { DropdownBase, DropdownBaseProps } from "../dropdown";
-import { List, ListProps } from "../list";
+import { List, ListControlProps, ListProps } from "../list";
 import { ChevronIcon } from "../list/ChevronIcon";
 import { useCombobox } from "./useCombobox";
 
@@ -31,7 +31,10 @@ export interface ComboBoxProps<
     Omit<ComponentSelectionProps<Item, S>, "onSelect">,
     Pick<ListProps<Item, S>, "ListItem" | "itemToString" | "source" | "width"> {
   InputProps?: InputProps;
-  ListProps?: Omit<ListProps<Item>, "ListItem" | "itemToString" | "source">;
+  ListProps?: Omit<
+    ListProps<Item>,
+    "ListItem" | "itemToString" | "source" | "onSelect" | "onSelectionChange"
+  >;
   allowBackspaceClearsSelection?: boolean;
   allowFreeText?: boolean;
   defaultValue?: string;
@@ -40,6 +43,7 @@ export interface ComboBoxProps<
   itemsToString?: (items: Item[]) => string;
   onDeselect?: () => void;
   onSetSelectedText?: (text: string) => void;
+  onListItemSelect?: ListProps<Item, S>["onSelect"];
   disableFilter?: boolean;
   value?: string;
 }
@@ -78,6 +82,7 @@ export const ComboBox = forwardRef(function Combobox<
     onDeselect,
     onOpenChange: onOpenChangeProp,
     onSelectionChange,
+    onListItemSelect,
     selected: selectedProp,
     selectionKeys,
     selectionStrategy,
@@ -154,6 +159,7 @@ export const ComboBox = forwardRef(function Combobox<
     isOpen: isOpenProp,
     itemToString,
     itemsToString,
+    onListItemSelect,
     onOpenChange: onOpenChangeProp,
     onSelectionChange,
     onSetSelectedText,
@@ -207,7 +213,7 @@ export const ComboBox = forwardRef(function Combobox<
           highlightedIndex={highlightedIndex}
           itemTextHighlightPattern={String(inputProps.value) || undefined}
           listHandlers={listHandlers}
-          onSelectionChange={onSelectionChange}
+          onSelectionChange={onSelectionChange} // not really needed, since onClick in listHandlers will be used instead.
           ref={listRef}
           selected={selected}
           selectionStrategy={selectionStrategy}

diff --git a/vuu-ui/packages/vuu-ui-controls/src/combo-box/useCombobox.ts b/vuu-ui/packages/vuu-ui-controls/src/combo-box/useCombobox.ts
@@ -45,6 +45,7 @@ export interface ComboboxHookProps<
       | "itemsToString"
       | "onDeselect"
       | "onSetSelectedText"
+      | "onListItemSelect"
       | "value"
     >,
     Omit<ComponentSelectionProps<Item, S>, "onSelect">,
@@ -91,6 +92,7 @@ export const useCombobox = <Item, S extends SelectionStrategy>({
   itemsToString,
   itemToString = defaultItemToString as (item: Item) => string,
   listRef,
+  onListItemSelect,
   onOpenChange,
   onSelectionChange,
   onSetSelectedText,
@@ -293,6 +295,7 @@ export const useCombobox = <Item, S extends SelectionStrategy>({
     label: "combobox",
     onKeyboardNavigation: handleKeyboardNavigation,
     onSelectionChange: handleSelectionChange,
+    onSelect: onListItemSelect,
     selected: collectionHook.itemToCollectionItemId(selectedProp as any),
     selectionKeys,
     selectionStrategy,