Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(filter-bar): replace batch dispatch actions with loop of singular #3783

Merged
merged 3 commits into from
Jun 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/smart-ties-happen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@kaizen/components": patch
---

Refactor FilterBar to loop through singular dispatch actions instead of having a separate batch action for the same result.
56 changes: 56 additions & 0 deletions packages/components/src/FilterBar/FilterBar.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -349,5 +349,61 @@ describe("<FilterBar />", () => {
expect(flavourButton.textContent).toEqual("Flavour:Honey Milk Tea")
})
})

it("shows a removable filter when a value is set", async () => {
const Wrapper = (): JSX.Element => {
type ExternalEventValues = {
flavour: string
}

const [values, setValues] = useState<Partial<ExternalEventValues>>({})

const filters = [
{
id: "flavour",
name: "Flavour",
Component: (
<FilterBar.Select
items={[
{ value: "honey-milk-tea", label: "Honey Milk Tea" },
{ value: "lychee-green-tea", label: "Lychee Green Tea" },
]}
/>
),
isRemovable: true,
},
] satisfies Filters<ExternalEventValues>

return (
<div>
<FilterBar
filters={filters}
values={values}
onValuesChange={setValues}
/>
<button
type="button"
onClick={() => setValues({ flavour: "honey-milk-tea" })}
>
Update Flavour to honey-milk-tea
</button>
</div>
)
}

const { getByRole, queryByRole } = render(<Wrapper />)

expect(queryByRole("button", { name: "Flavour" })).not.toBeInTheDocument()

await user.click(
getByRole("button", { name: "Update Flavour to honey-milk-tea" })
)

await waitFor(() => {
expect(
getByRole("button", { name: "Flavour : Honey Milk Tea" })
).toBeVisible()
})
})
})
})
13 changes: 10 additions & 3 deletions packages/components/src/FilterBar/context/FilterBarContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,19 @@ export const FilterBarProvider = <ValuesMap extends FiltersValues>({
onValuesChange({ ...values, [id]: undefined })
},
getInactiveFilters: () => getInactiveFilters<ValuesMap>(state),
clearAllFilters: () =>
dispatch({ type: "clear_all_filters", onValuesChange }),
clearAllFilters: () => {
state.activeFilterIds.forEach(id => {
if (mappedFilters[id].isRemovable)
dispatch({ type: "deactivate_filter", id })
})
onValuesChange({})
},
} satisfies FilterBarContextValue<any, ValuesMap>

useEffect(() => {
dispatch({ type: "activate_filters_with_values", values })
Object.keys(values).forEach(id => {
if (values[id]) dispatch({ type: "activate_filter", id })
})
}, [values])

const activeFilters = Array.from(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,27 +38,6 @@ describe("filterBarStateReducer", () => {
})
})

describe("filterBarStateReducer: activate_filters_with_values", () => {
it("sets a filter to active and adds entry to active filters", () => {
const state = {
filters: stateFilters,
activeFilterIds: new Set<keyof Values>(),
} satisfies FilterBarState<Values>

const newState = filterBarStateReducer<Values>(state, {
type: "activate_filters_with_values",
values: {
flavour: "jasmine",
sugarLevel: 50,
},
})

expect(newState.activeFilterIds).toEqual(
new Set(["flavour", "sugarLevel"])
)
})
})

describe("filterBarStateReducer: deactivate_filter", () => {
it("sets a filter to inactive and removes entry from active filters", () => {
const state = {
Expand All @@ -74,37 +53,4 @@ describe("filterBarStateReducer", () => {
expect(newState.activeFilterIds).toEqual(new Set())
})
})

describe("filterBarStateReducer: clear_all_filters", () => {
it("sets all removable filters to inactive", () => {
const state = {
filters: {
flavour: {
id: "flavour",
name: "Flavour",
isOpen: false,
},
sugarLevel: {
id: "sugarLevel",
name: "Sugar Level",
isOpen: false,
isRemovable: true,
},
},
activeFilterIds: new Set<keyof Values>(["flavour", "sugarLevel"]),
} satisfies FilterBarState<Values>

const onValuesChange = jest.fn<void, [Partial<Values>]>()

const newState = filterBarStateReducer<Values>(state, {
type: "clear_all_filters",
onValuesChange,
})

expect(newState.activeFilterIds).toEqual(
new Set<keyof Values>(["flavour"])
)
expect(onValuesChange).toHaveBeenCalledWith({})
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,7 @@ type Actions<ValuesMap extends FiltersValues> =
data: Partial<InternalFilterState>
}
| { type: "activate_filter"; id: keyof ValuesMap }
| { type: "activate_filters_with_values"; values: Partial<ValuesMap> }
| { type: "deactivate_filter"; id: keyof ValuesMap }
| {
type: "clear_all_filters"
onValuesChange: (values: Partial<ValuesMap>) => void
}

export const filterBarStateReducer = <ValuesMap extends FiltersValues>(
state: FilterBarState<ValuesMap>,
Expand All @@ -30,21 +25,8 @@ export const filterBarStateReducer = <ValuesMap extends FiltersValues>(
state.activeFilterIds.add(action.id)
return { ...state }

case "activate_filters_with_values":
Object.keys(action.values).forEach(id => {
if (action.values[id]) state.activeFilterIds.add(id)
})
return { ...state }

case "deactivate_filter":
state.activeFilterIds.delete(action.id)
return { ...state }

case "clear_all_filters":
state.activeFilterIds.forEach(id => {
if (state.filters[id].isRemovable) state.activeFilterIds.delete(id)
})
action.onValuesChange({})
return { ...state }
}
}