Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ const FiltersRoot = <Definition extends FiltersDefinition>({
// Also clear nested child filter keys to avoid orphaned values
const filterDef = filters?.[key]
if (filterDef?.type === "in" && filterDef.options) {
const nestedKeys = collectNestedFilterKeys(filterDef.options)
const nestedKeys = collectNestedFilterKeys(filterDef)
nestedKeys.forEach((nestedKey) => {
delete newFilters[nestedKey as keyof Definition]
})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
import userEvent from "@testing-library/user-event"
import "@testing-library/jest-dom/vitest"
import { describe, expect, it, vi } from "vitest"

import { zeroRender as render, screen, waitFor } from "@/testing/test-utils"

import type { FiltersDefinition } from "../types"

import { OneFilterPicker } from "../index"

/**
* With async options the picker can't walk the tree to find the nested child
* keys, so it used to treat the parent as empty: no active dot, no count,
* nothing to clear or to drop with the chip. `nestedFilterKeys` declares them.
*/
const officeOptions = [
{
value: "101",
label: "Barcelona HQ",
children: {
filterKey: "space",
options: [
{ value: "1", label: "Floor 1" },
{ value: "2", label: "Floor 2" },
],
},
},
]

const asyncDefinition = {
office: {
type: "in",
label: "Office",
options: {
nestedFilterKeys: ["space"],
options: async () => officeOptions,
},
},
space: {
type: "in",
label: "Space",
hideSelector: true,
options: {
options: [
{ value: "1", label: "Floor 1" },
{ value: "2", label: "Floor 2" },
],
},
},
} as const satisfies FiltersDefinition

const openPicker = async (user: ReturnType<typeof userEvent.setup>) => {
await user.click(screen.getByRole("button", { name: /filters/i }))
await waitFor(() =>
expect(screen.getByRole("button", { name: "Office" })).toBeInTheDocument()
)
}

describe("OneFilterPicker - nested filters with async options", () => {
it("marks the parent filter as active when only a nested child is selected", async () => {
const user = userEvent.setup({ pointerEventsCheck: 0 })

render(
<OneFilterPicker
filters={asyncDefinition}
value={{ space: ["1"] }}
onChange={vi.fn()}
/>
)

await openPicker(user)

expect(
screen.getByRole("button", {
name: "Office",
description: "Active filters: Office",
})
).toBeInTheDocument()
})

it("counts nested child selections in the parent's selected label", async () => {
const user = userEvent.setup({ pointerEventsCheck: 0 })

render(
<OneFilterPicker
filters={asyncDefinition}
value={{ space: ["1", "2"] }}
onChange={vi.fn()}
/>
)

await openPicker(user)
await user.click(screen.getByRole("button", { name: "Office" }))

expect(await screen.findByText("2 selected")).toBeInTheDocument()
})

it("clears nested child selections from the parent's select-all toggle", async () => {
const user = userEvent.setup({ pointerEventsCheck: 0 })
const onChange = vi.fn()

render(
<OneFilterPicker
filters={asyncDefinition}
value={{ space: ["1"] }}
onChange={onChange}
/>
)

await openPicker(user)
await user.click(screen.getByRole("button", { name: "Office" }))

await user.click(
await screen.findByRole("checkbox", { name: /select all/i })
)
await user.click(screen.getByRole("button", { name: /apply filters/i }))

// Empty values are dropped on apply: an orphaned `space: ["1"]` would survive.
expect(onChange).toHaveBeenCalledWith({})
})

it("drops nested child selections together with the parent chip", async () => {
const user = userEvent.setup({ pointerEventsCheck: 0 })
const onChange = vi.fn()

render(
<OneFilterPicker
filters={asyncDefinition}
value={{ office: ["101"], space: ["1"] }}
onChange={onChange}
/>
)

const removeChip = await screen.findByRole("button", {
name: "Close",
description: /^Office:/,
})
await user.click(removeChip)

expect(onChange).toHaveBeenCalledWith({})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export function FilterList<Definition extends FiltersDefinition>({
for (const [key, filter] of Object.entries(definition)) {
if (filter.type === "in" && "options" in filter) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- accessing nested options generically
const nested = collectNestedFilterKeys((filter as any).options)
const nested = collectNestedFilterKeys(filter as any)
if (nested.length > 0) map.set(key, nested)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { InFilterFlatOption } from "./components/InFilterFlatOption"
import { InFilterOptionRow } from "./components/InFilterOptionRow"
import {
collectNestedFilterKeys,
collectNestedFilterKeysFromOptions,
optionMatchesSearch,
} from "./components/option-utils"
import { InFilterOptionItem, InFilterOptions } from "./types"
Expand Down Expand Up @@ -168,8 +169,13 @@ export function InFilter<T extends string, R extends RecordType = RecordType>({
)

const nestedFilterKeys = useMemo(
() => collectNestedFilterKeys(schema.options),
[schema.options]
() => [
...new Set([
...collectNestedFilterKeys(schema),
...collectNestedFilterKeysFromOptions(options),
]),
],
[schema, options]
)

const nestedSelectionsCount = useMemo(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import { describe, expect, it } from "vitest"

import type { FilterTypeSchema } from "../../../types"
import type { InFilterOptionItem, InFilterOptions } from "../../types"

import { getCacheKey, loadOptions } from "../../useLoadOptions"
import { collectNestedFilterKeys } from "../option-utils"

type Schema = FilterTypeSchema<InFilterOptions<string>>

const workplaceOptions: InFilterOptionItem<string>[] = [
{
value: "barcelona",
label: "Barcelona Office",
children: {
filterKey: "workArea",
options: [
{
value: "floor-1",
label: "Floor 1",
children: {
filterKey: "desk",
options: [{ value: "desk-1", label: "Desk 1" }],
},
},
],
},
},
{ value: "remote", label: "Remote" },
]

describe("collectNestedFilterKeys", () => {
it("walks literal option arrays at every depth", () => {
const schema: Schema = {
label: "Workplace",
options: { options: workplaceOptions },
}

expect(collectNestedFilterKeys(schema)).toEqual(["workArea", "desk"])
})

it("returns no keys for a flat filter", () => {
const schema: Schema = {
label: "Department",
options: { options: [{ value: "eng", label: "Engineering" }] },
}

expect(collectNestedFilterKeys(schema)).toEqual([])
})

it("reads the keys declared in the schema when options are async", () => {
const schema: Schema = {
label: "Workplace",
options: {
nestedFilterKeys: ["workArea"],
options: async () => workplaceOptions,
},
}

expect(collectNestedFilterKeys(schema)).toEqual(["workArea"])
})

it("reads the keys declared in the schema when options come from a source", () => {
const schema: Schema = {
label: "Workplace",
options: {
nestedFilterKeys: ["workArea"],
source: { dataAdapter: { fetchData: async () => ({ records: [] }) } },
mapOptions: (item: InFilterOptionItem<string>) => item,
},
}

expect(collectNestedFilterKeys(schema)).toEqual(["workArea"])
})

it("falls back to the options resolved by a previous load", async () => {
const schema: Schema = {
label: "Workplace (cached)",
options: { cache: true, options: async () => workplaceOptions },
}

expect(collectNestedFilterKeys(schema)).toEqual([])

await loadOptions(getCacheKey(schema), () => workplaceOptions, true)

expect(collectNestedFilterKeys(schema)).toEqual(["workArea", "desk"])
})

it("merges the declared keys with the ones found in the options", async () => {
const schema: Schema = {
label: "Workplace (merged)",
options: {
cache: true,
nestedFilterKeys: ["workArea"],
options: async () => workplaceOptions,
},
}

await loadOptions(getCacheKey(schema), () => workplaceOptions, true)

expect(collectNestedFilterKeys(schema)).toEqual(["workArea", "desk"])
})
})
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { FilterTypeSchema } from "../../types"
import { InFilterOptionItem, InFilterOptions } from "../types"
import { getCacheKey, getCachedOptions } from "../useLoadOptions"

/**
* Recursively checks whether an option or any of its nested children
Expand Down Expand Up @@ -35,13 +37,9 @@ export function hasSelectedDescendant<T>(
return false
}

/**
* Collects all nested child filter keys from an InFilter's options.
* Used to determine if a parent filter should show an active indicator
* when any of its nested children have selections.
*/
export function collectNestedFilterKeys<T>(
filterOptions: InFilterOptions<T>
/** Nested child filter keys reachable from an option tree. */
export function collectNestedFilterKeysFromOptions<T>(
options: InFilterOptionItem<T>[] | undefined
): string[] {
const keys = new Set<string>()

Expand All @@ -54,8 +52,30 @@ export function collectNestedFilterKeys<T>(
}
}

if ("options" in filterOptions && Array.isArray(filterOptions.options)) {
collect(filterOptions.options)
collect(options ?? [])

return [...keys]
}

/**
* Nested child filter keys of an InFilter. The filter list and the chips render
* before the filter is ever opened, so the keys have to be resolvable without
* the options: declared in the schema, listed literally, or left over in the
* cache from a previous load.
*/
export function collectNestedFilterKeys<T>(
schema: FilterTypeSchema<InFilterOptions<T>>
): string[] {
const filterOptions = schema.options
const keys = new Set<string>(filterOptions.nestedFilterKeys ?? [])

const resolvedOptions =
"options" in filterOptions && Array.isArray(filterOptions.options)
? filterOptions.options
: getCachedOptions<T>(getCacheKey(schema))
Comment on lines +72 to +75

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fallback doesn't fire where it's needed. With async options and no nestedFilterKeys, the dot stays off after the options load and only appears on a later mount: nestedKeysMap in FilterList.tsx is a useMemo keyed on definition, so the cache filling later never invalidates it. Same filter, two behaviors, depending on history the user can't see. It also makes render impure, since this reads a mutable module-level Map during render.

Suggest dropping the fallback and letting nestedFilterKeys be the single answer for non-literal options. It's deterministic and already carries the fix on its own: all four integration tests pass on the declared keys alone.

Heads up that option-utils.test.ts:76 can't catch this. It calls loadOptions(getCacheKey(schema), ...) with the same object and asserts the read back, so it covers the Map round-trip, not the component path.


for (const key of collectNestedFilterKeysFromOptions(resolvedOptions)) {
keys.add(key)
}

return [...keys]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ export type InFilterOptionItem<T = unknown> = {
*/
export type InFilterOptions<T, _R extends RecordType = RecordType> = {
cache?: boolean
/**
* Filter keys holding nested child selections. Only needed for async or
* `source` options, which the picker can't walk to discover them.
*/
nestedFilterKeys?: string[]
/**
* Optional function to resolve labels for specific values without fetching all options.
* This is useful when you have a dynamic source and want to avoid fetching all options
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useCallback, useEffect, useState } from "react"

import { RecordType, useData, useDataSource } from "@/hooks/datasource"

import { InFilterDefinition } from "."
import type { InFilterDefinition } from "."
import { FilterTypeSchema } from "../types"
import { InFilterOptionItem, InFilterOptions } from "./types"

Expand All @@ -22,6 +22,13 @@ export function getCacheKey<T, R extends RecordType = RecordType>(
return JSON.stringify(schema)
}

/** Options a previous load resolved for a schema, without triggering one. */
export function getCachedOptions<T>(
cacheKey: string
): InFilterOptionItem<T>[] | undefined {
return optionsCache.get(cacheKey) as InFilterOptionItem<T>[] | undefined
}

/**
* Cache a label for a specific value in a schema
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export function getClearedFiltersValue<Filters extends FiltersDefinition>(
continue
}

for (const nestedKey of collectNestedFilterKeys(filter.options)) {
for (const nestedKey of collectNestedFilterKeys(filter)) {
clearedFilters[nestedKey as keyof Filters] =
[] as unknown as FiltersState<Filters>[keyof Filters]
}
Expand Down
Loading