From 77437337105e7e72b1109216dd9eba0d621486a6 Mon Sep 17 00:00:00 2001 From: Junaid <62522218+junaidzm13@users.noreply.github.com> Date: Thu, 11 Jan 2024 17:12:10 +0800 Subject: [PATCH] #1074 fix major bugs in FilterBar (#1117) - fixes a bug that crashes the app when a user tries editing an existing filter. Cause: we try finding the edited filter in `filters` state by reference but the reference has already been changed since editFilter is itself a state (reference changes on every set). - resets the editingFilter.current to undefined after apply save. Otherwise, crashes the app when the user has already successfully edited an existing filter and tries to add a new one. - also fixes an issue with TextInput multiselection handling, where it was wrongly sending a string instead of an array of length 1. Crashes the app when the user selects only a single element in a multi value filter clause. --- .../src/filter-bar/useFilterBar.ts | 42 +++++++++---------- .../vuu-filters/src/filter-bar/useFilters.ts | 6 +-- .../src/filter-clause/TextInput.tsx | 15 +------ .../vuu-ui-controls/src/date-picker/types.ts | 7 +--- 4 files changed, 27 insertions(+), 43 deletions(-) diff --git a/vuu-ui/packages/vuu-filters/src/filter-bar/useFilterBar.ts b/vuu-ui/packages/vuu-filters/src/filter-bar/useFilterBar.ts index 551193bd9..63ad4364b 100644 --- a/vuu-ui/packages/vuu-filters/src/filter-bar/useFilterBar.ts +++ b/vuu-ui/packages/vuu-filters/src/filter-bar/useFilterBar.ts @@ -252,43 +252,40 @@ export const useFilterBar = ({ [deleteFilter, editPillLabel, filters, focusFilterClause] ); + const addIfNewElseUpdate = useCallback( + (edited: Filter, existing: Filter | undefined) => { + if (existing === undefined) { + const idx = onAddFilter(edited); + editPillLabel(idx); + return idx; + } else { + return onChangeFilter(existing, edited); + } + }, + [editPillLabel, onAddFilter, onChangeFilter] + ); + const handleMenuAction = useCallback( ({ menuId }) => { switch (menuId) { case "apply-save": { - // TODO save these into state together - const isNewFilter = editingFilter.current === undefined; - const newFilter = editFilter as Filter; - const changeHandler = isNewFilter ? onAddFilter : onChangeFilter; - const indexOfNewFilter = changeHandler(newFilter); - - if (isNewFilter) { - editPillLabel(indexOfNewFilter); - } - + const editedFilter = editFilter as Filter; + const idx = addIfNewElseUpdate(editedFilter, editingFilter.current); + setActiveFilterIndex(appendIfNotPresent(idx)); setEditFilter(undefined); - - setActiveFilterIndex((indices) => - indices.includes(indexOfNewFilter) - ? indices - : indices.concat(indexOfNewFilter) - ); - + editingFilter.current = undefined; setShowMenu(false); return true; } - case "and-clause": { const newFilter = addClause( editFilter as Filter, EMPTY_FILTER_CLAUSE ); - console.log({ newFilter }); setEditFilter(newFilter); setShowMenu(false); return true; } - case "or-clause": setEditFilter((filter) => addClause(filter as Filter, {}, { combineWith: "or" }) @@ -299,7 +296,7 @@ export const useFilterBar = ({ return false; } }, - [editFilter, editPillLabel, onAddFilter, onChangeFilter] + [editFilter, addIfNewElseUpdate] ); useEffect(() => { @@ -471,3 +468,6 @@ export const useFilterBar = ({ showMenu, }; }; + +const appendIfNotPresent = (n: number) => (ns: number[]) => + ns.includes(n) ? ns : ns.concat(n); diff --git a/vuu-ui/packages/vuu-filters/src/filter-bar/useFilters.ts b/vuu-ui/packages/vuu-filters/src/filter-bar/useFilters.ts index 4a85266d7..877b40d60 100644 --- a/vuu-ui/packages/vuu-filters/src/filter-bar/useFilters.ts +++ b/vuu-ui/packages/vuu-filters/src/filter-bar/useFilters.ts @@ -167,12 +167,12 @@ export const useFilters = ({ ); const handleChangeFilter = useCallback( - (filter: Filter) => { + (oldFilter: Filter, newFilter: Filter) => { let index = -1; const newFilters = filters.map((f, i) => { - if (f === filter) { + if (f === oldFilter) { index = i; - return filter; + return newFilter; } else { return f; } diff --git a/vuu-ui/packages/vuu-filters/src/filter-clause/TextInput.tsx b/vuu-ui/packages/vuu-filters/src/filter-clause/TextInput.tsx index 44cd3fd7b..8bcec734b 100644 --- a/vuu-ui/packages/vuu-filters/src/filter-clause/TextInput.tsx +++ b/vuu-ui/packages/vuu-filters/src/filter-clause/TextInput.tsx @@ -58,25 +58,15 @@ export const TextInput = forwardRef(function TextInput( const getSuggestions = suggestionProvider(); const handleSingleValueSelectionChange = useCallback( - (evt, value) => onInputComplete(value), + (_, value) => onInputComplete(value), [onInputComplete] ); const handleMultiValueSelectionChange = useCallback( - (evt, value) => { - if (value.length === 1) { - onInputComplete(value[0]); - } else if (value.length > 1) { - onInputComplete(value); - } - }, + (_, values) => onInputComplete(values), [onInputComplete] ); - useEffect(() => { - // setValueInputValue(""); - }, [column]); - useEffect(() => { if (table) { const params: TypeaheadParams = valueInputValue @@ -130,7 +120,6 @@ export const TextInput = forwardRef(function TextInput( } switch (operator) { case "in": - //TODO multiselect return (