From fe18d5c9107d4c3d2bc8fd256c4999a03ba48457 Mon Sep 17 00:00:00 2001 From: "Malik, Junaid" Date: Fri, 12 Jan 2024 16:10:42 +0800 Subject: [PATCH] #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. --- .../src/filter-clause/ExpandoCombobox.css | 73 ++++++++++--------- .../src/filter-clause/FilterClauseEditor.tsx | 8 +- .../filter-clause/useFilterClauseEditor.ts | 42 +++++------ .../src/combo-box/ComboBox.tsx | 12 ++- .../src/combo-box/useCombobox.ts | 3 + 5 files changed, 72 insertions(+), 66 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 index b9dc89624c..752d2b1dc6 100644 --- 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 index 2196955982..94eb7a5053 100644 --- 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 index 5e04913fa5..a89d8a4a2f 100644 --- 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 >( - (evt, column) => { + (_, column) => { setSelectedColumn(column ?? undefined); setOperator(undefined); setValue(undefined); @@ -242,8 +242,8 @@ export const useFilterClauseEditor = ({ [onCancel, setOperator] ); - const handleSelectionChangeOperator = useCallback( - (evt, selected) => { + const handleOperatorSelect = useCallback( + (_, selected) => { const op = selected; if (op === undefined || isValidFilterClauseOp(op)) { setOperator(op); @@ -293,25 +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); + onChange({ + column: selectedColumn?.name, + op: operator, + value: newValue, + }); } } }, @@ -371,8 +363,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 index dd328424dc..0e9031f352 100644 --- 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, "onSelect">, Pick, "ListItem" | "itemToString" | "source" | "width"> { InputProps?: InputProps; - ListProps?: Omit, "ListItem" | "itemToString" | "source">; + ListProps?: Omit< + ListProps, + "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["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 index 10bb9aa739..857bf01efc 100644 --- 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, "onSelect">, @@ -91,6 +92,7 @@ export const useCombobox = ({ itemsToString, itemToString = defaultItemToString as (item: Item) => string, listRef, + onListItemSelect, onOpenChange, onSelectionChange, onSetSelectedText, @@ -293,6 +295,7 @@ export const useCombobox = ({ label: "combobox", onKeyboardNavigation: handleKeyboardNavigation, onSelectionChange: handleSelectionChange, + onSelect: onListItemSelect, selected: collectionHook.itemToCollectionItemId(selectedProp as any), selectionKeys, selectionStrategy,