Skip to content

Commit

Permalink
finos#1074 fix major bugs in FilterBar (finos#1117)
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
junaidzm13 authored Jan 11, 2024
1 parent 7623b26 commit 7743733
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 43 deletions.
42 changes: 21 additions & 21 deletions vuu-ui/packages/vuu-filters/src/filter-bar/useFilterBar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<MenuActionHandler>(
({ 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" })
Expand All @@ -299,7 +296,7 @@ export const useFilterBar = ({
return false;
}
},
[editFilter, editPillLabel, onAddFilter, onChangeFilter]
[editFilter, addIfNewElseUpdate]
);

useEffect(() => {
Expand Down Expand Up @@ -471,3 +468,6 @@ export const useFilterBar = ({
showMenu,
};
};

const appendIfNotPresent = (n: number) => (ns: number[]) =>
ns.includes(n) ? ns : ns.concat(n);
6 changes: 3 additions & 3 deletions vuu-ui/packages/vuu-filters/src/filter-bar/useFilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
15 changes: 2 additions & 13 deletions vuu-ui/packages/vuu-filters/src/filter-clause/TextInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,25 +58,15 @@ export const TextInput = forwardRef(function TextInput(
const getSuggestions = suggestionProvider();

const handleSingleValueSelectionChange = useCallback<SingleSelectionHandler>(
(evt, value) => onInputComplete(value),
(_, value) => onInputComplete(value),
[onInputComplete]
);

const handleMultiValueSelectionChange = useCallback<MultiSelectionHandler>(
(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
Expand Down Expand Up @@ -130,7 +120,6 @@ export const TextInput = forwardRef(function TextInput(
}
switch (operator) {
case "in":
//TODO multiselect
return (
<ExpandoCombobox
InputProps={InputProps}
Expand Down
7 changes: 1 addition & 6 deletions vuu-ui/packages/vuu-ui-controls/src/date-picker/types.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
import {
CalendarDate,
CalendarDateTime,
DateValue,
ZonedDateTime,
} from "@internationalized/date";
import { DateValue } from "@internationalized/date";
import { CalendarProps } from "../calendar/Calendar";
import { RangeSelectionValueType } from "../calendar";

Expand Down

0 comments on commit 7743733

Please sign in to comment.