ben.carrasco/prowler
1
0
Fork 0
mirror of https://github.com/prowler-cloud/prowler.git synced 2026-08-19 09:30:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(ui): remove "Clear all" button from filter pills strip (#10481)

Browse Source
This commit is contained in:
Alan Buscaglia
2026-03-30 12:26:01 +02:00
committed by GitHub
parent 6f6d62f51f
commit 6df74529d6
6 changed files with 11 additions and 506 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/CHANGELOG.md
+1
View File
@@ -7,6 +7,7 @@ All notable changes to the **Prowler UI** are documented in this file.
### 🔄 Changed
- Attack Paths custom openCypher queries now use a code editor with syntax highlighting and line numbers [(#10445)](https://github.com/prowler-cloud/prowler/pull/10445)
- Filter summary strip: removed redundant "Clear all" link next to pills (use top-bar Clear Filters instead) and switched chip variant from `outline` to `tag` for consistency [(#10481)](https://github.com/prowler-cloud/prowler/pull/10481)
### 🐞 Fixed
ui/components/filters/filter-summary-strip.test.tsx
+4 -83
View File
@@ -38,7 +38,6 @@ import {
// TODO (E2E): Full filter strip flow should be covered in Playwright tests:
// - Filter chips appear after staging selections in the findings page
// - Removing a chip via the X button un-stages that filter value
// - "Clear all" removes all staged filter chips at once
// - Chips disappear after applying filters (pending state resets to URL state)
// ──────────────────────────────────────────────────────────────────────────
@@ -55,15 +54,10 @@ describe("FilterSummaryStrip", () => {
it("should not render anything", () => {
// Given
const onRemove = vi.fn();
const onClearAll = vi.fn();
// When
const { container } = render(
<FilterSummaryStrip
chips={[]}
onRemove={onRemove}
onClearAll={onClearAll}
/>,
<FilterSummaryStrip chips={[]} onRemove={onRemove} />,
);
// Then
@@ -77,16 +71,9 @@ describe("FilterSummaryStrip", () => {
it("should render a chip for each filter value", () => {
// Given
const onRemove = vi.fn();
const onClearAll = vi.fn();
// When
render(
<FilterSummaryStrip
chips={mockChips}
onRemove={onRemove}
onClearAll={onClearAll}
/>,
);
render(<FilterSummaryStrip chips={mockChips} onRemove={onRemove} />);
// Then — 3 chips should be visible (2 severity + 1 status)
expect(screen.getAllByTestId("badge")).toHaveLength(3);
@@ -95,7 +82,6 @@ describe("FilterSummaryStrip", () => {
it("should display the label and value text for each chip", () => {
// Given
const onRemove = vi.fn();
const onClearAll = vi.fn();
// When
render(
@@ -108,7 +94,6 @@ describe("FilterSummaryStrip", () => {
},
]}
onRemove={onRemove}
onClearAll={onClearAll}
/>,
);
@@ -120,7 +105,6 @@ describe("FilterSummaryStrip", () => {
it("should display displayValue when provided instead of value", () => {
// Given
const onRemove = vi.fn();
const onClearAll = vi.fn();
// When
render(
@@ -134,7 +118,6 @@ describe("FilterSummaryStrip", () => {
},
]}
onRemove={onRemove}
onClearAll={onClearAll}
/>,
);
@@ -143,39 +126,12 @@ describe("FilterSummaryStrip", () => {
expect(screen.queryByText("FAIL")).not.toBeInTheDocument();
});
it("should render a 'Clear all' button", () => {
// Given
const onRemove = vi.fn();
const onClearAll = vi.fn();
// When
render(
<FilterSummaryStrip
chips={mockChips}
onRemove={onRemove}
onClearAll={onClearAll}
/>,
);
// Then
expect(
screen.getByRole("button", { name: "Clear all" }),
).toBeInTheDocument();
});
it("should render an aria-label region for accessibility", () => {
// Given
const onRemove = vi.fn();
const onClearAll = vi.fn();
// When
render(
<FilterSummaryStrip
chips={mockChips}
onRemove={onRemove}
onClearAll={onClearAll}
/>,
);
render(<FilterSummaryStrip chips={mockChips} onRemove={onRemove} />);
// Then
expect(
@@ -191,7 +147,6 @@ describe("FilterSummaryStrip", () => {
// Given
const user = userEvent.setup();
const onRemove = vi.fn();
const onClearAll = vi.fn();
render(
<FilterSummaryStrip
@@ -203,7 +158,6 @@ describe("FilterSummaryStrip", () => {
},
]}
onRemove={onRemove}
onClearAll={onClearAll}
/>,
);
@@ -222,15 +176,8 @@ describe("FilterSummaryStrip", () => {
// Given
const user = userEvent.setup();
const onRemove = vi.fn();
const onClearAll = vi.fn();
render(
<FilterSummaryStrip
chips={mockChips}
onRemove={onRemove}
onClearAll={onClearAll}
/>,
);
render(<FilterSummaryStrip chips={mockChips} onRemove={onRemove} />);
// When — click the X button for "high" severity
const removeHighButton = screen.getByRole("button", {
@@ -243,30 +190,4 @@ describe("FilterSummaryStrip", () => {
expect(onRemove).toHaveBeenCalledTimes(1);
});
});
// ── onClearAll interaction ───────────────────────────────────────────────
describe("onClearAll", () => {
it("should call onClearAll when 'Clear all' is clicked", async () => {
// Given
const user = userEvent.setup();
const onRemove = vi.fn();
const onClearAll = vi.fn();
render(
<FilterSummaryStrip
chips={mockChips}
onRemove={onRemove}
onClearAll={onClearAll}
/>,
);
// When
await user.click(screen.getByRole("button", { name: "Clear all" }));
// Then
expect(onClearAll).toHaveBeenCalledTimes(1);
expect(onRemove).not.toHaveBeenCalled();
});
});
});
ui/components/filters/filter-summary-strip.tsx
+1 -13
View File
@@ -21,8 +21,6 @@ export interface FilterSummaryStripProps {
chips: FilterChip[];
/** Called when the user clicks the X on a chip */
onRemove: (key: string, value: string) => void;
/** Called when the user clicks "Clear all" */
onClearAll: () => void;
/** Optional extra class names for the outer wrapper */
className?: string;
}
@@ -33,13 +31,11 @@ export interface FilterSummaryStripProps {
*
* - Hidden when `chips` is empty.
* - Each chip carries its own X button to remove that single value.
* - A "Clear all" link removes everything at once.
* - Reusable: no Findings-specific logic, driven entirely by props.
*/
export const FilterSummaryStrip = ({
chips,
onRemove,
onClearAll,
className,
}: FilterSummaryStripProps) => {
if (chips.length === 0) return null;
@@ -54,7 +50,7 @@ export const FilterSummaryStrip = ({
{chips.map((chip) => (
<Badge
key={`${chip.key}-${chip.value}`}
variant="outline"
variant="tag"
className="flex items-center gap-1 pr-1"
>
<span className="text-text-neutral-primary text-xs">
@@ -71,14 +67,6 @@ export const FilterSummaryStrip = ({
</button>
</Badge>
))}
<button
type="button"
onClick={onClearAll}
className="text-text-neutral-secondary hover:text-text-neutral-primary text-xs underline-offset-2 hover:underline focus-visible:ring-1 focus-visible:ring-offset-1 focus-visible:outline-none"
>
Clear all
</button>
</div>
);
};
ui/components/findings/findings-filters.tsx
+1 -20
View File
@@ -119,7 +119,6 @@ export const FindingsFilters = ({
applyAll,
discardAll,
clearAndApply,
clearKeys,
hasChanges,
changeCount,
getFilterValue,
@@ -199,20 +198,6 @@ export const FindingsFilters = ({
setPending(filterKey, nextValues);
};
// Handler for clearing all chips: clears only the filter keys visible as chips,
// without touching provider/account selectors.
const PROVIDER_KEYS = new Set([
"filter[provider_type__in]",
"filter[provider_id__in]",
]);
const handleClearAllChips = () => {
const chipKeys = Array.from(new Set(filterChips.map((c) => c.key))).filter(
(k) => !PROVIDER_KEYS.has(k),
);
clearKeys(chipKeys);
};
// Derive pending muted state for the checkbox.
// Note: "filter[muted]" participates in batch mode — applyAll includes it
// when present in pending state, and the defaultParams option ensures
@@ -291,11 +276,7 @@ export const FindingsFilters = ({
</div>
{/* Summary strip: shown below filter bar when there are pending changes */}
<FilterSummaryStrip
chips={filterChips}
onRemove={handleChipRemove}
onClearAll={handleClearAllChips}
/>
<FilterSummaryStrip chips={filterChips} onRemove={handleChipRemove} />
{/* Expandable filters section */}
{hasCustomFilters && (
ui/hooks/use-filter-batch.test.ts
-327
View File
@@ -336,167 +336,6 @@ describe("useFilterBatch", () => {
});
});
// ── clearAll ───────────────────────────────────────────────────────────────
describe("clearAll", () => {
it("should clear all pending filters including provider and account keys", () => {
// Given — user has pending provider, account, severity, and status filters
setSearchParams({});
const { result } = renderHook(() => useFilterBatch());
act(() => {
result.current.setPending("filter[provider_type__in]", [
"aws",
"azure",
]);
result.current.setPending("filter[provider_id__in]", [
"provider-uuid-1",
]);
result.current.setPending("filter[severity__in]", ["critical"]);
result.current.setPending("filter[status__in]", ["FAIL"]);
});
// Pre-condition — all filters are pending
expect(
result.current.pendingFilters["filter[provider_type__in]"],
).toEqual(["aws", "azure"]);
expect(result.current.pendingFilters["filter[provider_id__in]"]).toEqual([
"provider-uuid-1",
]);
expect(result.current.pendingFilters["filter[severity__in]"]).toEqual([
"critical",
]);
// When
act(() => {
result.current.clearAll();
});
// Then — pending state must be TRULY EMPTY (no keys at all, not even with empty arrays)
expect(result.current.pendingFilters).toEqual({});
// getFilterValue normalises missing keys to [] so all selectors show "all selected"
expect(
result.current.getFilterValue("filter[provider_type__in]"),
).toEqual([]);
expect(result.current.getFilterValue("filter[provider_id__in]")).toEqual(
[],
);
expect(result.current.getFilterValue("filter[severity__in]")).toEqual([]);
expect(result.current.getFilterValue("filter[status__in]")).toEqual([]);
});
it("should also clear provider/account keys that came from the URL (applied state)", () => {
// Given — URL has provider and account filters applied
setSearchParams({
"filter[provider_type__in]": "aws",
"filter[provider_id__in]": "provider-uuid-1",
"filter[severity__in]": "critical",
});
const { result } = renderHook(() => useFilterBatch());
// Pre-condition — filters are loaded from URL into pending
expect(
result.current.pendingFilters["filter[provider_type__in]"],
).toEqual(["aws"]);
expect(result.current.pendingFilters["filter[provider_id__in]"]).toEqual([
"provider-uuid-1",
]);
// When
act(() => {
result.current.clearAll();
});
// Then — pending state must be truly empty (no keys, not { key: [] })
expect(result.current.pendingFilters).toEqual({});
// provider and account must be cleared even though they came from the URL
expect(
result.current.getFilterValue("filter[provider_type__in]"),
).toEqual([]);
expect(result.current.getFilterValue("filter[provider_id__in]")).toEqual(
[],
);
expect(result.current.getFilterValue("filter[severity__in]")).toEqual([]);
});
it("should mark hasChanges as true after clear when URL still has applied filters", () => {
// Given — URL has filters applied
setSearchParams({
"filter[provider_type__in]": "aws",
"filter[severity__in]": "critical",
});
const { result } = renderHook(() => useFilterBatch());
// Pre-condition — no pending changes (matches URL)
expect(result.current.hasChanges).toBe(false);
// When — clear all
act(() => {
result.current.clearAll();
});
// Then — hasChanges must be true (pending is empty, URL still has filters)
expect(result.current.hasChanges).toBe(true);
});
it("should NOT clear excluded keys (filter[search]) but DOES clear filter[muted]", () => {
// Given — URL has search (excluded) plus muted and severity (both in batch)
setSearchParams({
"filter[search]": "my-search",
"filter[muted]": "false",
"filter[severity__in]": "critical",
});
const { result } = renderHook(() => useFilterBatch());
// Pre-condition — muted and severity are in pendingFilters; search is excluded
expect(result.current.pendingFilters["filter[search]"]).toBeUndefined();
expect(result.current.pendingFilters["filter[muted]"]).toEqual(["false"]);
// When
act(() => {
result.current.clearAll();
});
// Then — severity and muted are cleared; search remains excluded (undefined in pending)
expect(result.current.getFilterValue("filter[severity__in]")).toEqual([]);
expect(result.current.pendingFilters["filter[search]"]).toBeUndefined();
// muted is a batch key, so it gets cleared by clearAll
expect(result.current.pendingFilters["filter[muted]"]).toBeUndefined();
});
it("should clear applied URL filters even if they were explicitly removed from pendingFilters", () => {
// This covers the edge case where pendingFilters diverged from URL state
// (e.g., URL has provider filter but the key was removed from pending via removePending)
setSearchParams({
"filter[provider_type__in]": "gcp",
"filter[severity__in]": "high",
});
const { result } = renderHook(() => useFilterBatch());
// Remove the provider key from pending (diverge from URL state)
act(() => {
result.current.removePending("filter[provider_type__in]");
});
// Pre-condition — provider is gone from pending but still in URL
expect(
result.current.pendingFilters["filter[provider_type__in]"],
).toBeUndefined();
// When — clearAll should clear BOTH pending keys AND applied URL keys
act(() => {
result.current.clearAll();
});
// Then — severity is cleared
expect(result.current.getFilterValue("filter[severity__in]")).toEqual([]);
// provider_type__in was in the URL (applied state), so clearAll must handle it
expect(
result.current.getFilterValue("filter[provider_type__in]"),
).toEqual([]);
});
});
// ── discardAll ─────────────────────────────────────────────────────────────
describe("discardAll", () => {
@@ -679,170 +518,4 @@ describe("useFilterBatch", () => {
expect(calledUrl).toContain("page=1");
});
});
// ── clearKeys ─────────────────────────────────────────────────────────────
describe("clearKeys", () => {
it("should remove only specified keys and push URL", () => {
// Given
setSearchParams({});
const { result } = renderHook(() => useFilterBatch());
act(() => {
result.current.setPending("filter[severity__in]", ["critical"]);
result.current.setPending("filter[status__in]", ["FAIL"]);
result.current.setPending("filter[region__in]", ["us-east-1"]);
});
// When
act(() => {
result.current.clearKeys(["filter[severity__in]"]);
});
// Then — severity is gone; status and region remain
expect(
result.current.pendingFilters["filter[severity__in]"],
).toBeUndefined();
expect(result.current.pendingFilters["filter[status__in]"]).toEqual([
"FAIL",
]);
expect(result.current.pendingFilters["filter[region__in]"]).toEqual([
"us-east-1",
]);
// And the pushed URL contains the remaining keys but not severity
expect(mockPush).toHaveBeenCalledTimes(1);
const calledUrl: string = mockPush.mock.calls[0][0];
expect(calledUrl).toContain("status");
expect(calledUrl).toContain("region");
expect(calledUrl).not.toContain("severity");
});
it("should accept keys without 'filter[' prefix", () => {
// Given
setSearchParams({});
const { result } = renderHook(() => useFilterBatch());
act(() => {
result.current.setPending("filter[severity__in]", ["critical"]);
});
// When — pass key without filter[] wrapper
act(() => {
result.current.clearKeys(["severity__in"]);
});
// Then — severity is cleared
expect(
result.current.pendingFilters["filter[severity__in]"],
).toBeUndefined();
expect(mockPush).toHaveBeenCalledTimes(1);
const calledUrl: string = mockPush.mock.calls[0][0];
expect(calledUrl).not.toContain("severity");
});
it("should preserve provider/account keys not in the cleared list", () => {
// Given
setSearchParams({});
const { result } = renderHook(() => useFilterBatch());
act(() => {
result.current.setPending("filter[provider_type__in]", ["aws"]);
result.current.setPending("filter[severity__in]", ["critical"]);
result.current.setPending("filter[status__in]", ["FAIL"]);
});
// When — clear only severity and status; leave provider untouched
act(() => {
result.current.clearKeys([
"filter[severity__in]",
"filter[status__in]",
]);
});
// Then — provider_type__in is still in pending
expect(
result.current.pendingFilters["filter[provider_type__in]"],
).toEqual(["aws"]);
expect(
result.current.pendingFilters["filter[severity__in]"],
).toBeUndefined();
expect(
result.current.pendingFilters["filter[status__in]"],
).toBeUndefined();
// And the pushed URL retains provider but not severity/status
expect(mockPush).toHaveBeenCalledTimes(1);
const calledUrl: string = mockPush.mock.calls[0][0];
expect(calledUrl).toContain("provider_type__in");
expect(calledUrl).not.toContain("severity");
expect(calledUrl).not.toContain("status__in");
});
it("should apply defaultParams after clearing", () => {
// Given
setSearchParams({});
const { result } = renderHook(() =>
useFilterBatch({ defaultParams: { "filter[muted]": "false" } }),
);
act(() => {
result.current.setPending("filter[severity__in]", ["critical"]);
});
// When
act(() => {
result.current.clearKeys(["filter[severity__in]"]);
});
// Then — defaultParam is present in the pushed URL
expect(mockPush).toHaveBeenCalledTimes(1);
const calledUrl: string = mockPush.mock.calls[0][0];
expect(calledUrl).toContain("filter%5Bmuted%5D=false");
});
it("should reset pagination to page 1", () => {
// Given — URL already has a page param
mockSearchParamsValue = new URLSearchParams({
"filter[severity__in]": "critical",
page: "5",
});
const { result } = renderHook(() => useFilterBatch());
// When
act(() => {
result.current.clearKeys(["filter[severity__in]"]);
});
// Then — page is reset to 1
expect(mockPush).toHaveBeenCalledTimes(1);
const calledUrl: string = mockPush.mock.calls[0][0];
expect(calledUrl).toContain("page=1");
});
it("should handle empty keys array gracefully", () => {
// Given
setSearchParams({});
const { result } = renderHook(() => useFilterBatch());
act(() => {
result.current.setPending("filter[severity__in]", ["critical"]);
});
// When — clear no keys at all
act(() => {
result.current.clearKeys([]);
});
// Then — pending is unchanged
expect(result.current.pendingFilters["filter[severity__in]"]).toEqual([
"critical",
]);
// And router.push was still called (navigates with current state)
expect(mockPush).toHaveBeenCalledTimes(1);
const calledUrl: string = mockPush.mock.calls[0][0];
expect(calledUrl).toContain("severity");
});
});
});
ui/hooks/use-filter-batch.ts
+4 -63
View File
@@ -24,27 +24,12 @@ export interface UseFilterBatchReturn {
applyAll: () => void;
/** Discard all pending changes, reset pending to the current URL state */
discardAll: () => void;
/**
* Clear all pending filters to an empty state (no filters selected).
* Unlike `discardAll`, this does NOT reset to the URL state — it sets
* pending to `{}` (truly empty). The user must click Apply to push
* the empty state to the URL.
* Includes provider/account keys and all batch-managed filter keys.
*/
clearAll: () => void;
/**
* Clear all batch-managed filters and immediately navigate (router.push)
* with defaultParams applied. Equivalent to clearAll() + applyAll() but
* avoids the async state gap between the two calls.
* with defaultParams applied. Resets pending state to empty and pushes
* the resulting URL in one step.
*/
clearAndApply: () => void;
/**
* Clear only the specified filter keys from pending state and immediately
* navigate (router.push) with the remaining pending filters + defaultParams.
* Used by "Clear all" in the pills strip to remove only pill-visible filters
* without touching provider/account selectors.
*/
clearKeys: (keys: string[]) => void;
/** Remove a single filter key from pending state */
removePending: (key: string) => void;
/** Whether pending state differs from the current URL */
@@ -226,59 +211,17 @@ export const useFilterBatch = (
setPendingFilters(applied);
};
/**
* Clears ALL pending batch filters to an empty state (no filters selected).
*
* Unlike `discardAll`, this resets pending to `{}` — not to the current URL
* state. This covers both:
* - Keys that are already in `pendingFilters` (pending-only or URL-loaded)
* - Keys that are in the applied (URL) state but were removed from pending
* via `removePending` (edge case: diverged state)
*
* The user must click Apply to push the empty state to the URL.
* `applyAll()` removes all batch-managed URL params first, so even keys
* absent from `pendingFilters` will be removed from the URL on apply.
*/
const clearAll = () => {
// Return a truly empty object — no filters pending at all.
// `getFilterValue` normalises missing keys to [] so selectors will show
// their "all selected" / placeholder state immediately.
setPendingFilters({});
};
/**
* Clears ALL batch-managed filters and immediately navigates (router.push).
*
* Works around the async gap between clearAll() + applyAll(): instead of
* setting pending to `{}` and then calling applyAll() (which would still
* read the old pendingFilters from the closure), this function builds the
* target URL directly from an empty pending state and pushes it in one step.
* defaultParams (e.g. filter[muted]=false) are applied as usual.
* Builds the target URL directly from an empty pending state and pushes it
* in one step. defaultParams (e.g. filter[muted]=false) are applied as usual.
*/
const clearAndApply = () => {
setPendingFilters({});
buildAndPush({});
};
/**
* Removes only the specified filter keys from pending state and immediately
* navigates (router.push) with the remaining filters + defaultParams.
*
* Used by the pills strip "Clear all" to remove pill-visible filters (severity,
* status, delta, region, service, etc.) without touching provider/account selectors.
*/
const clearKeys = (keys: string[]) => {
const normalizedKeys = keys.map((k) =>
k.startsWith("filter[") ? k : `filter[${k}]`,
);
const nextPending: PendingFilters = { ...pendingFilters };
normalizedKeys.forEach((k) => {
delete nextPending[k];
});
setPendingFilters(nextPending);
buildAndPush(nextPending);
};
const getFilterValue = (key: string): string[] => {
const filterKey = key.startsWith("filter[") ? key : `filter[${key}]`;
return pendingFilters[filterKey] ?? [];
@@ -297,9 +240,7 @@ export const useFilterBatch = (
setPending,
applyAll,
discardAll,
clearAll,
clearAndApply,
clearKeys,
removePending,
hasChanges,
changeCount,