Skip to content

fix: Introduce zustand state to FacetsPanel to improve maintainability #4769

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

Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
4a35b59
Refactor facets
skynetigor May 11, 2025
3847487
add zustand store for facets panel
skynetigor May 12, 2025
91ef44a
refactor: improve facet handling and state management in FacetsPanel …
skynetigor May 12, 2025
1459c52
Merge branch 'main' into 4560-Incidents-page-load-with-resolved-ones-…
skynetigor May 12, 2025
c929232
refactor: streamline facet selection logic and improve button visibil…
skynetigor May 12, 2025
df88bc7
test: enhance incident tests with additional status handling and init…
skynetigor May 12, 2025
6272cca
fix: update default incident status options to include acknowledged s…
skynetigor May 13, 2025
d977096
refactor
skynetigor May 13, 2025
2ed4419
Fixes
skynetigor May 13, 2025
14ac953
test: update filtering logic to include acknowledged status in incide…
skynetigor May 13, 2025
c2574bc
Merge branch 'main' into 4560-Incidents-page-load-with-resolved-ones-…
skynetigor May 13, 2025
aed9611
Merge branch 'main' into 4560-Incidents-page-load-with-resolved-ones-…
skynetigor May 13, 2025
4c028c3
Merge branch '4560-Incidents-page-load-with-resolved-ones-ignoring-fi…
skynetigor May 13, 2025
5e455f2
feat: add useFacetsConfig hook for improved facets configuration hand…
skynetigor May 13, 2025
5560976
refactor
skynetigor May 13, 2025
9dff8d8
Merge branch 'main' into 4560-Incidents-page-load-with-resolved-ones-…
skynetigor May 14, 2025
7270d5e
Merge branch 'main' into 4560-Incidents-page-load-with-resolved-ones-…
skynetigor May 14, 2025
8d199f7
Merge branch 'main' into 4560-Incidents-page-load-with-resolved-ones-…
Matvey-Kuk May 15, 2025
b0ec408
refactor: remove unused buildCel function and related tests
skynetigor May 19, 2025
e055304
refactor: improve type safety in facets panel and store
skynetigor May 19, 2025
a148ae4
Apply suggestions from code review
skynetigor May 19, 2025
315f887
Merge branch 'main' into 4560-Incidents-page-load-with-resolved-ones-…
skynetigor May 19, 2025
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
Prev Previous commit
Next Next commit
refactor: improve facet handling and state management in FacetsPanel …
…and Facet components
  • Loading branch information
skynetigor committed May 12, 2025
commit 91ef44a315b84ce4dd2f14480ba2e5aa3f771f7b
56 changes: 10 additions & 46 deletions keep-ui/features/filter/facet-panel-server-side.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import React, { useEffect, useState } from "react";
import { FacetOptionsQueries, FacetOptionsQuery, FacetsConfig } from "./models";
import {
FacetDto,
FacetOptionsQueries,
FacetOptionsQuery,
FacetsConfig,
} from "./models";
import { useFacetActions, useFacetOptions, useFacets } from "./hooks";
import { InitialFacetsData } from "./api";
import { FacetsPanel } from "./facets-panel";
Expand Down Expand Up @@ -94,62 +99,21 @@ export const FacetsPanelServerSide: React.FC<FacetsPanelProps> = ({
revalidationToken
);

useEffect(
function reloadOptions() {
if (
facetsData === initialFacetsData?.facets &&
initialFacetsData?.facetOptions
) {
return;
}

const newFacetQueriesState = buildFacetsQueriesState();

if (newFacetQueriesState) {
// setFacetQueriesState(newFacetQueriesState);
}
},
// disabled because this effect uses currentFacetQueriesState that's also change in that effect
// eslint-disable-next-line react-hooks/exhaustive-deps
[facetsData]
);

function buildFacetsQueriesState() {
let newFacetQueriesState: { [key: string]: string } | undefined = undefined;

facetsData?.forEach((facet) => {
if (!newFacetQueriesState) {
newFacetQueriesState = {};
}
if (facetQueriesState && facet.id in facetQueriesState) {
newFacetQueriesState[facet.id] = facetQueriesState[facet.id];
} else {
newFacetQueriesState[facet.id] = "";
}
});

if (newFacetQueriesState) {
return newFacetQueriesState;
}

return null;
}

return (
<>
<FacetsPanel
panelId={entityName}
className={className || ""}
facets={facetsData?.slice(0, 1) as any}
facets={facetsData as FacetDto[]}
facetOptions={facetOptions as any}
areFacetOptionsLoading={!isSilentReloading && facetOptionsLoading}
clearFiltersToken={clearFiltersToken}
facetsConfig={facetsConfig}
onCelChange={onCelChange}
onAddFacet={() => setIsModalOpen(true)}
onLoadFacetOptions={(facetId) => {
setFacetQueriesState({ ...facetQueriesState, [facetId]: "" });
}}
onLoadFacetOptions={(facetId) =>
setFacetQueriesState({ ...facetQueriesState, [facetId]: "" })
}
onDeleteFacet={(facetId) => facetActions.deleteFacet(facetId)}
onReloadFacetOptions={(facetQueries) =>
setFacetQueriesState({ ...facetQueries })
Expand Down
47 changes: 38 additions & 9 deletions keep-ui/features/filter/facet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Skeleton from "react-loading-skeleton";
import { FacetValue } from "./facet-value";
import { FacetConfig, FacetDto, FacetOptionDto, FacetState } from "./models";
import { TrashIcon } from "@heroicons/react/24/outline";
import { useExistingFacetStore } from "./store";

export interface FacetProps {
facet: FacetDto;
Expand All @@ -16,7 +17,6 @@ export interface FacetProps {
optionsReloading: boolean;
showIcon?: boolean;
facetConfig?: FacetConfig;
onCelChange?: (cel: string) => void;
onLoadOptions?: () => void;
onDelete?: () => void;
}
Expand All @@ -27,10 +27,8 @@ export const Facet: React.FC<FacetProps> = ({
showIcon = true,
optionsLoading,
optionsReloading,
onCelChange,
onLoadOptions,
onDelete,
isOpenByDefault,
facetConfig,
}) => {
function getInitialFacetState(): Set<string> {
Expand Down Expand Up @@ -58,9 +56,13 @@ export const Facet: React.FC<FacetProps> = ({
);
const facetStateRef = useRef(facetState);
facetStateRef.current = facetState;
const onCelChangeRef = useRef(onCelChange);
onCelChangeRef.current = onCelChange;
const optionsRef = useRef(options);
optionsRef.current = options;
const facetRef = useRef(facet);
facetRef.current = facet;
const [isInitialized, setIsInitialized] = useState(false);
const clearFiltersToken = useExistingFacetStore((s) => s.clearFiltersToken);
const setFacetCelState = useExistingFacetStore((s) => s.setFacetCelState);

function valueToString(value: any): string {
if (typeof value === "string") {
Expand All @@ -70,6 +72,7 @@ export const Facet: React.FC<FacetProps> = ({
} else if (value == null) {
return "null";
}

return `${value}`;
}

Expand All @@ -82,21 +85,46 @@ export const Facet: React.FC<FacetProps> = ({
return;
}

setFacetState(new Set(options.map((opt) => opt.value)));
setFacetState(new Set(options.map((opt) => valueToString(opt.value))));
setIsInitialized(true);
}, [isInitialized, setIsInitialized, options, facetConfig]);

const currentCel = useMemo(() => {
const values = Array.from(facetState);

if (values.length === optionsRef.current?.length) {
return "";
}

if (!values.length) {
return "";
}

return `${facet.property_path} in [${values.join(", ")}]`;
}, [facet.property_path, facetState]);

useEffect(() => onCelChangeRef.current?.(currentCel), [currentCel]);
useEffect(
() => setFacetCelState(facet.id, currentCel),
[setFacetCelState, facet.id, currentCel]
);

useEffect(() => {
if (clearFiltersToken && facetRef.current && optionsRef.current) {
const facetState = new Set<string>();

if (facetConfig?.checkedByDefaultOptionValues) {
facetConfig.checkedByDefaultOptionValues.forEach((optionValue) => {
facetState.add(valueToString(optionValue));
});
} else {
optionsRef.current.forEach((option) => {
facetState.add(valueToString(option.value));
});
}

setFacetState(facetState);
}
}, [clearFiltersToken, setFacetState]);

useEffect(() => {
setIsLoaded(!!options); // Sync prop change with state
Expand All @@ -115,12 +143,13 @@ export const Facet: React.FC<FacetProps> = ({
);

const isOptionSelected = (optionValue: string) => {
return facetState.has(valueToString(optionValue));
const strValue = valueToString(optionValue);
return facetState.has(strValue);
};

function toggleFacetOption(value: any) {
const strValue = valueToString(value);
if (isOptionSelected(strValue)) {
if (isOptionSelected(value)) {
facetState.delete(strValue);
} else {
facetState.add(strValue);
Expand Down
127 changes: 41 additions & 86 deletions keep-ui/features/filter/facets-panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ import {
FacetOptionDto,
FacetOptionsQueries,
FacetsConfig,
FacetState,
} from "./models";
import { PlusIcon, XMarkIcon } from "@heroicons/react/24/outline";
import "react-loading-skeleton/dist/skeleton.css";
import clsx from "clsx";
import { useDebouncedValue } from "@/utils/hooks/useDebouncedValue";
import { FacetStoreProvider, useNewFacetStore } from "./store";
import { useStore } from "zustand";

export interface FacetsPanelProps {
panelId: string;
Expand Down Expand Up @@ -54,20 +55,15 @@ export const FacetsPanel: React.FC<FacetsPanelProps> = ({
onLoadFacetOptions = undefined,
onReloadFacetOptions = undefined,
}) => {
const defaultStateHandledForFacetIds = useMemo(() => new Set<string>(), []);
const [facetsState, setFacetsState] = useState<FacetState>({});
const [facetCelState, setFacetCelState] = useState<Record<
string,
string
> | null>(null);
const [debouncedFacetCelState] = useDebouncedValue(facetCelState, 100);

const [facetOptionQueries, setFacetOptionQueries] =
useState<FacetOptionsQueries | null>(null);
const facetOptionsRef = useRef<any>(facetOptions);
facetOptionsRef.current = facetOptions;
Copy link
Contributor

Choose a reason for hiding this comment

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

style: facetOptionsRef type should be explicitly defined as Record<string, FacetOptionDto[]> instead of any

Suggested change
const facetOptionsRef = useRef<any>(facetOptions);
facetOptionsRef.current = facetOptions;
const facetOptionsRef = useRef<{ [key: string]: FacetOptionDto[] }>(facetOptions);
facetOptionsRef.current = facetOptions;

const onCelChangeRef = useRef(onCelChange);
onCelChangeRef.current = onCelChange;
const store = useNewFacetStore();
const state = useStore(store);
const [debouncedFacetCelState] = useDebouncedValue(state.facetCelState, 100);

const facetsConfigIdBased = useMemo(() => {
const result: FacetsConfig = {};
Expand Down Expand Up @@ -99,29 +95,6 @@ export const FacetsPanel: React.FC<FacetsPanelProps> = ({

return result;
}, [facetsConfig, facets]);
const facetsConfigIdBasedRef = useRef(facetsConfigIdBased);
facetsConfigIdBasedRef.current = facetsConfigIdBased;

function getFacetState(facetId: string): Set<string> {
if (
!defaultStateHandledForFacetIds.has(facetId) &&
facetsConfigIdBased[facetId]?.checkedByDefaultOptionValues
) {
const facetState = new Set<string>(...(facetsState[facetId] || []));
const facet = facets.find((f) => f.id === facetId);

if (facet) {
facetsConfigIdBased[facetId]?.checkedByDefaultOptionValues.forEach(
(optionValue) => facetState.add(optionValue)
);
defaultStateHandledForFacetIds.add(facetId);
}

facetsState[facetId] = facetState;
}

return facetsState[facetId] || new Set<string>();
}

useEffect(() => {
if (facetOptionQueries) {
Expand Down Expand Up @@ -152,41 +125,24 @@ export const FacetsPanel: React.FC<FacetsPanelProps> = ({
});

setFacetOptionQueries(facetOptionQueries);

const filterCel = Object.values(debouncedFacetCelState || {})
.filter(Boolean)
.map((cel) => `(${cel})`)
.join(" && ");
onCelChangeRef.current && onCelChangeRef.current(filterCel);
}, [debouncedFacetCelState, setFacetOptionQueries]);

function clearFilters(): void {
Object.keys(facetsState).forEach((facetId) => facetsState[facetId].clear());
defaultStateHandledForFacetIds.clear();

const newFacetsState: FacetState = {};

facets.forEach((facet) => {
newFacetsState[facet.id] = getFacetState(facet.id);
});
setFacetsState(newFacetsState);
}

useEffect(
function clearFiltersWhenTokenChange(): void {
if (clearFiltersToken) {
clearFilters();
state.clearFilters();
}
// eslint-disable-next-line react-hooks/exhaustive-deps
},
[clearFiltersToken]
);

const updateFacetsCelState = (facetId: string, facetCel: string) =>
setFacetCelState({
...(facetCelState || {}),
[facetId]: facetCel,
});

return (
<section
id={`${panelId}-facets`}
Expand All @@ -204,47 +160,46 @@ export const FacetsPanel: React.FC<FacetsPanelProps> = ({
Add Facet
</button>
<button
onClick={() => clearFilters()}
onClick={() => state.clearFilters()}
className="p-1 pr-2 text-sm text-gray-600 hover:bg-gray-100 rounded flex items-center gap-1"
>
<XMarkIcon className="h-4 w-4" />
Reset
</button>
</div>

{!facets &&
[undefined, undefined, undefined].map((_, index) => (
<Facet
facet={
{
id: "",
name: "",
is_static: true,
} as FacetDto
}
key={index}
isOpenByDefault={true}
optionsLoading={true}
optionsReloading={false}
/>
))}

{facets &&
facets.map((facet, index) => (
<Facet
key={facet.id + index}
facet={facet}
onCelChange={(cel) => updateFacetsCelState(facet.id, cel)}
options={facetOptions?.[facet.id]}
optionsLoading={!facetOptions?.[facet.id]}
optionsReloading={areFacetOptionsLoading && !!facet.id}
facetConfig={facetsConfigIdBased[facet.id]}
onLoadOptions={() =>
onLoadFacetOptions && onLoadFacetOptions(facet.id)
}
onDelete={() => onDeleteFacet && onDeleteFacet(facet.id)}
/>
))}
<FacetStoreProvider store={store}>
{!facets &&
[undefined, undefined, undefined].map((_, index) => (
<Facet
facet={
{
id: "",
name: "",
is_static: true,
} as FacetDto
}
key={index}
isOpenByDefault={true}
optionsLoading={true}
optionsReloading={false}
/>
))}
{facets &&
facets.map((facet, index) => (
<Facet
key={facet.id + index}
facet={facet}
options={facetOptions?.[facet.id]}
optionsLoading={!facetOptions?.[facet.id]}
optionsReloading={areFacetOptionsLoading && !!facet.id}
facetConfig={facetsConfigIdBased[facet.id]}
onLoadOptions={() =>
onLoadFacetOptions && onLoadFacetOptions(facet.id)
}
onDelete={() => onDeleteFacet && onDeleteFacet(facet.id)}
/>
))}
</FacetStoreProvider>
</div>
</section>
);
Expand Down
Loading