Skip to content

feat(app): remember collapsed pinned filters (#3227) #3605

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.

Sign up for GitHub

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

Merged
kodiakhq merged 3 commits into from
Nov 10, 2026
+263 −15
Merged

feat(app): remember collapsed pinned filters (#3227) #3605

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9480b46
feat(app): remember collapsed pinned filters (#3227)
bferreirq Nov 10, 2026
f1a4b17
fix(app): forget collapsed state on unpin, test the expand rule
bferreirq Nov 10, 2026
8576157
Merge branch 'main' into feat/remember-collapsed-pinned-filters
kodiakhq[bot] Nov 10, 2026
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
6 changes: 6 additions & 0 deletions .changeset/remember-collapsed-pinned-filters.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@hyperdx/app': patch
---

feat: A pinned filter collapsed in the search sidebar stays collapsed the next
time the source loads, instead of always opening
78 changes: 77 additions & 1 deletion packages/app/src/__tests__/pinnedFilters.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
* - localStorage migration logic
*/

import { mergePinnedData } from '@/searchFilters';
import { act, renderHook } from '@testing-library/react';

import { mergePinnedData, useCollapsedPinnedFields } from '@/searchFilters';

describe('mergePinnedData', () => {
it('returns empty fields and filters when both are null', () => {
Expand Down Expand Up @@ -167,3 +169,77 @@ describe('localStorage migration', () => {
expect(result[sourceB]).toEqual({ level: ['error'] });
});
});

@github-actions github-actions Bot Nov 10, 2026

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.

🔵 minor — Tests only cover the localStorage hook, not the new expand rules in the sidebar

The new tests only check useCollapsedPinnedFields. The real behaviour lives in the isDefaultExpanded / onExpandedChange changes in DBSearchPageFilters.tsx, and none of it is tested: a collapsed pinned field renders collapsed, it still opens when it has a selection, the collapsed state now takes priority over forceExpanded in the Shared Filters section, and unpinned fields are never stored. Any of these could break and the tests would still pass. Add a component test that renders DBSearchPageFilters with a pinned field, collapses it, remounts, and checks the filter-group-control expanded state, both with and without an active filter.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

@bferreirq bferreirq Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in f1a4b17: the expand-on-load rule now lives in getFilterGroupDefaultExpanded, with tests for a collapsed pinned field staying closed, an active filter still opening it, precedence over the Shared Filters forceExpanded, and stale entries ignored once unpinned. I didn't add a full DBSearchPageFilters render test since no existing test mounts the whole sidebar (it needs metadata, facets and pinned-filters API mocks); happy to add one if a maintainer wants it.

describe('useCollapsedPinnedFields', () => {
beforeEach(() => {
window.localStorage.clear();
});

it('remembers a collapsed pinned field across remounts', () => {
const first = renderHook(() => useCollapsedPinnedFields('source1'));
expect(first.result.current.isPinnedFieldCollapsed('ServiceName')).toBe(
false,
);

act(() =>
first.result.current.setPinnedFieldCollapsed('ServiceName', true),
);
expect(first.result.current.isPinnedFieldCollapsed('ServiceName')).toBe(
true,
);
first.unmount();

const second = renderHook(() => useCollapsedPinnedFields('source1'));
expect(second.result.current.isPinnedFieldCollapsed('ServiceName')).toBe(
true,
);
});

it('forgets the field once it is expanded again', () => {
const { result } = renderHook(() => useCollapsedPinnedFields('source1'));
act(() => result.current.setPinnedFieldCollapsed('ServiceName', true));
act(() => result.current.setPinnedFieldCollapsed('ServiceName', false));

expect(result.current.isPinnedFieldCollapsed('ServiceName')).toBe(false);
expect(
JSON.parse(window.localStorage.getItem('hdx-collapsed-pinned-fields')!),
).toEqual({ source1: [] });
});

it('keeps the collapsed state per source', () => {
const a = renderHook(() => useCollapsedPinnedFields('sourceA'));
act(() => a.result.current.setPinnedFieldCollapsed('ServiceName', true));

const b = renderHook(() => useCollapsedPinnedFields('sourceB'));
expect(b.result.current.isPinnedFieldCollapsed('ServiceName')).toBe(false);
});

it('does nothing without a source', () => {
const { result } = renderHook(() => useCollapsedPinnedFields(null));
act(() => result.current.setPinnedFieldCollapsed('ServiceName', true));

expect(result.current.isPinnedFieldCollapsed('ServiceName')).toBe(false);
expect(window.localStorage.getItem('hdx-collapsed-pinned-fields')).toBe(
null,
);
});
});

describe('useCollapsedPinnedFields pruning', () => {
beforeEach(() => {
window.localStorage.clear();
});

it('keeps only the fields still pinned after a reset', () => {
const { result } = renderHook(() => useCollapsedPinnedFields('source1'));
act(() => result.current.setPinnedFieldCollapsed('ServiceName', true));
act(() => result.current.setPinnedFieldCollapsed('SeverityText', true));

act(() =>
result.current.pruneCollapsedPinnedFields(f => f === 'SeverityText'),
);

expect(result.current.isPinnedFieldCollapsed('ServiceName')).toBe(false);
expect(result.current.isPinnedFieldCollapsed('SeverityText')).toBe(true);
});
});
87 changes: 73 additions & 14 deletions packages/app/src/components/DBSearchPageFilters.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import { usePinnedFiltersApi } from '@/pinnedFilters';
import {
FilterStateHook,
IS_ROOT_SPAN_COLUMN_NAME,
useCollapsedPinnedFields,
usePinnedFilters,
} from '@/searchFilters';
import { useSource } from '@/source';
Expand Down Expand Up @@ -918,6 +919,31 @@ function FilterGroupActions({

const voidFunc = () => {};

/**
* Whether a filter group opens on load. A pinned field the user collapsed
* stays collapsed unless it has an active filter. That persisted collapse
* intentionally takes precedence over a section-level `forceExpanded`
* (the Shared Filters section).
*/
export function getFilterGroupDefaultExpanded({
hasSelection,
isPinned,
isCollapsed,
isPrimary,
forceExpanded,
}: {
hasSelection: boolean;
isPinned: boolean;
isCollapsed: boolean;
isPrimary: boolean;
forceExpanded?: boolean;
}): boolean {
if (isPinned && isCollapsed && !hasSelection) {
return false;
}
return forceExpanded ?? (isPrimary || isPinned || hasSelection);
}

export const FilterGroup = ({
name,
options,
Expand Down Expand Up @@ -1172,6 +1198,11 @@ const DBSearchPageFiltersComponent = ({
hasPersonalPins,
hasSharedPins,
} = usePinnedFilters(sourceId ?? null);
const {
isPinnedFieldCollapsed,
setPinnedFieldCollapsed,
pruneCollapsedPinnedFields,
} = useCollapsedPinnedFields(sourceId ?? null);
const { data: pinnedFiltersApiData } = usePinnedFiltersApi(sourceId ?? null);
const [isSharedFiltersVisible, setSharedFiltersVisible] = useLocalStorage(
'hdx-shared-filters-visible',
Expand Down Expand Up @@ -1504,6 +1535,8 @@ const DBSearchPageFiltersComponent = ({
const { keyPrefix = '', isDefaultExpanded: forceExpanded } =
options ?? {};
const { grouped, nonGrouped } = groupFacetsByBaseName(facets);
const isFieldPinnedAny = (key: string) =>
isFieldPinned(key) || isSharedFieldPinned(key);

const makeValuePins = (key: string): ValuePinHandlers => ({
onPinClick: (value: string | boolean) => toggleFilterPin(key, value),
Expand All @@ -1515,9 +1548,21 @@ const DBSearchPageFiltersComponent = ({
});

const makeFieldPins = (key: string): FieldPinHandlers => ({
onFieldPinClick: () => toggleFieldPin(key),
onFieldPinClick: () => {

@github-actions github-actions Bot Nov 10, 2026

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.

🔵 minor — Collapsed state is only cleared on unpins made in this tab, so a re-pinned field can come back collapsed

The cleanup runs inside four hand-wired handlers: the two unpin handlers here and the two reset handlers at lines 1794–1802. Shared pins are stored on the team (usePinnedFiltersApi, searchFilters.tsx:503), so the cleanup never runs in these cases: a teammate unpins a shared field, another tab unpins it, or someone unpins through the NestedFilterGroup handlers at lines 1608–1610. The stale entry stays in hdx-collapsed-pinned-fields. When the field is pinned again, getFilterGroupDefaultExpanded sees isPinned && isCollapsed and opens it collapsed, which goes against the comment's promise that re-pinning "opens it like any newly pinned field". Fix: put the rule in one place instead of in the callers. Either clear the entry when a field becomes pinned, or have useCollapsedPinnedFields drop entries that are not in the merged pinned set (getPinnedFields). Then remove the four call-site checks.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

// Unpinned for good: forget its collapsed state, so pinning it again
// opens it like any newly pinned field.
if (isFieldPinned(key) && !isSharedFieldPinned(key)) {
setPinnedFieldCollapsed(key, false);
}
toggleFieldPin(key);
},
isFieldPinned: isFieldPinned(key),
onToggleSharedFieldPin: () => toggleSharedFieldPin(key),
onToggleSharedFieldPin: () => {
if (isSharedFieldPinned(key) && !isFieldPinned(key)) {
setPinnedFieldCollapsed(key, false);
}
toggleSharedFieldPin(key);
},
isSharedFieldPinned: isSharedFieldPinned(key),
});

Expand Down Expand Up @@ -1641,17 +1686,23 @@ const DBSearchPageFiltersComponent = ({
hasLoadedMore={extraFacetKeys.has(facet.key)}
isDefaultExpanded={(() => {
const entry = getFilterStateEntry(filterState, facet.key);
return (
forceExpanded ??
(isFieldPrimary(tableMetadata, facet.key) ||
isFieldPinned(facet.key) ||
isSharedFieldPinned(facet.key) ||
(entry != null &&
(entry.included.size > 0 ||
entry.excluded.size > 0 ||
entry.range != null)))
);
return getFilterGroupDefaultExpanded({
hasSelection:
entry != null &&
(entry.included.size > 0 ||
entry.excluded.size > 0 ||
entry.range != null),
isPinned: isFieldPinnedAny(facet.key),
isCollapsed: isPinnedFieldCollapsed(facet.key),
isPrimary: !!isFieldPrimary(tableMetadata, facet.key),
forceExpanded,
});
})()}
onExpandedChange={expanded => {

@github-actions github-actions Bot Nov 10, 2026

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.

🔵 minor — Collapsed state stays stored after the field is unpinned, so it comes back when the field is pinned again

onExpandedChange only writes while the field is pinned, and nothing removes the entry when the pin goes away. That includes toggleFieldPin, toggleSharedFieldPin and resetPersonalPins in searchFilters.tsx. So if you pin X, collapse it, unpin it, and later pin X again, it reloads collapsed even though you never collapsed it as a pinned field that time. Old entries also pile up in hdx-collapsed-pinned-fields. Fix: call setPinnedFieldCollapsed(key, false) when a field is unpinned and on reset, or ignore stored entries for fields that are no longer pinned.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

@bferreirq bferreirq Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in f1a4b17: unpinning a field (once no personal or shared pin is left) or resetting pins now clears its collapsed state, so pinning it again opens it.

if (isFieldPinnedAny(facet.key)) {
setPinnedFieldCollapsed(facet.key, !expanded);
}
}}
chartConfig={chartConfig}
isLive={isLive}
onRangeChange={range => setFilterRange(facet.key, range)}
Expand All @@ -1673,6 +1724,8 @@ const DBSearchPageFiltersComponent = ({
isFieldPinned,
toggleSharedFieldPin,
isSharedFieldPinned,
isPinnedFieldCollapsed,
setPinnedFieldCollapsed,
onColumnToggle,
displayedColumns,
loadMoreFacetsForKey,
Expand Down Expand Up @@ -1738,9 +1791,15 @@ const DBSearchPageFiltersComponent = ({
showFilterCounts={showFilterCounts}
onShowFilterCountsChange={setShowFilterCounts}
hasPersonalPins={hasPersonalPins}
onResetPersonalPins={resetPersonalPins}
onResetPersonalPins={() => {
pruneCollapsedPinnedFields(isSharedFieldPinned);
resetPersonalPins();
}}
hasSharedPins={hasSharedPins}
onResetSharedFilters={resetSharedFilters}
onResetSharedFilters={() => {
pruneCollapsedPinnedFields(isFieldPinned);
resetSharedFilters();
}}
showAllValues={showAllValues}
onShowAllValuesChange={setShowAllValues}
/>
Expand Down
58 changes: 58 additions & 0 deletions packages/app/src/components/__tests__/DBSearchPageFilters.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
cleanedFacetName,
FilterGroup,
type FilterGroupProps,
getFilterGroupDefaultExpanded,
} from '@/components/DBSearchPageFilters';
import { NestedFilterGroup } from '@/components/DBSearchPageFilters/NestedFilterGroup';
import {
Expand Down Expand Up @@ -1149,3 +1150,60 @@ describe('NestedFilterGroup', () => {
expect(screen.getByPlaceholderText('Search keys...')).toBeInTheDocument();
});
});

describe('getFilterGroupDefaultExpanded', () => {
const base = {
hasSelection: false,
isPinned: false,
isCollapsed: false,
isPrimary: false,
};

it('opens a pinned field by default', () => {
expect(getFilterGroupDefaultExpanded({ ...base, isPinned: true })).toBe(
true,
);
});

it('keeps a pinned field the user collapsed closed', () => {
expect(
getFilterGroupDefaultExpanded({
...base,
isPinned: true,
isCollapsed: true,
}),
).toBe(false);
});

it('still opens a collapsed pinned field that has an active filter', () => {
expect(
getFilterGroupDefaultExpanded({
...base,
isPinned: true,
isCollapsed: true,
hasSelection: true,
}),
).toBe(true);
});

it('keeps it closed in the Shared Filters section, which forces groups open', () => {
expect(
getFilterGroupDefaultExpanded({
...base,
isPinned: true,
isCollapsed: true,
forceExpanded: true,
}),
).toBe(false);
});

it('ignores a stale collapsed entry once the field is no longer pinned', () => {
expect(
getFilterGroupDefaultExpanded({
...base,
isCollapsed: true,
isPrimary: true,
}),
).toBe(true);
});
});
49 changes: 49 additions & 0 deletions packages/app/src/searchFilters.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,55 @@ function usePersonalPinnedFilters(sourceId: string | null) {
return { filters, fields, setFilters, setFields };
}

/**
* Pinned fields the user collapsed, per source, so they stay collapsed on the
* next load instead of always opening. Personal (localStorage), like the
* other expanded/collapsed states of the filters sidebar.
*/
export function useCollapsedPinnedFields(sourceId: string | null) {
const [collapsed, setCollapsed] = useLocalStorage<{
[sourceId: string]: string[];
}>('hdx-collapsed-pinned-fields', {});

const isPinnedFieldCollapsed = useCallback(
(field: string) =>
sourceId != null && (collapsed[sourceId] ?? []).includes(field),
[collapsed, sourceId],
);

const setPinnedFieldCollapsed = useCallback(
(field: string, isCollapsed: boolean) => {
if (!sourceId) return;
setCollapsed(prev => {
const fields = (prev[sourceId] ?? []).filter(f => f !== field);
return {
...prev,
[sourceId]: isCollapsed ? [...fields, field] : fields,
};
});
},
[sourceId, setCollapsed],
);

/** Keep only the collapsed fields for which `keep` is true (e.g. after a pin reset). */
const pruneCollapsedPinnedFields = useCallback(
(keep: (field: string) => boolean) => {
if (!sourceId) return;
setCollapsed(prev => ({
...prev,
[sourceId]: (prev[sourceId] ?? []).filter(keep),
}));
},
[sourceId, setCollapsed],
);

return {
isPinnedFieldCollapsed,
setPinnedFieldCollapsed,
pruneCollapsedPinnedFields,
};
}

export function usePinnedFilters(sourceId: string | null) {
// Personal pins: localStorage (per-user, per-browser)
const personal = usePersonalPinnedFilters(sourceId);
Expand Down
Loading