From 7cde902978bbd48197e89dc6866074beb65237e2 Mon Sep 17 00:00:00 2001 From: Adrian Bonpin Date: Sat, 5 Sep 2026 18:38:44 +0800 Subject: [PATCH] Rework column filters and toolbar toggles - Remove the drag-and-drop FilterBuilder in favor of the existing '+ Add filter' row editor in the Column Filters popover - Debounce filter value edits (400ms) so a query only fires after the user stops typing, fixing a race where the in-flight fetch guard silently dropped the final filter value - Make every toolbar toggle (filter, sort, auto-refresh, show/hide columns, export, bulk actions) a clean plain toggle by wrapping the button + dropdown in a shared ref so the outside-click handler no longer fights the button click --- AGENTS.md | 2 +- .../db-viewer/FilterBuilder.test.tsx | 27 ---- .../components/db-viewer/FilterBuilder.tsx | 132 ------------------ .../db-viewer/TableControls.test.tsx | 95 ++++++++++++- .../components/db-viewer/TableControls.tsx | 128 +++++++++++++---- 5 files changed, 189 insertions(+), 195 deletions(-) delete mode 100644 desktop/src/components/db-viewer/FilterBuilder.test.tsx delete mode 100644 desktop/src/components/db-viewer/FilterBuilder.tsx diff --git a/AGENTS.md b/AGENTS.md index d473e17..037b6de 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -290,7 +290,7 @@ Planned work is prioritized in the [Project Roadmap](./ROADMAP.md) (source of tr | Smart default sort | ✅ | 12-tier priority: updated_at → created_at → *_at → *_id → seq/rank/version | | Data grid pagination | ✅ | Page nav, page size selector persisted in settings | | Column filtering (server-side) | ✅ | eq, neq, contains, starts, ends, gt, lt, null, notnull pushed to SQL WHERE | -| Visual filter builder | ✅ | Drag-and-drop column palette (@dnd-kit) with type-aware operators (textish → contains, else eq), AND semantics, persists in tab `filterRules` | +| Visual filter builder | ✅ | Column Filters popover with a "+ Add filter" row editor (column / operator / value selects, per-rule remove, AND semantics, persists in tab `filterRules`). Free-text values are debounced (400ms) so a query only fires after the user stops typing; column/operator changes apply immediately. The filter button is a plain toggle (open when closed, close when open) | | Column sorting (server-side) | ✅ | Multi-column asc/desc pushed to SQL ORDER BY | | Column show/hide | ✅ | Toggle visibility per column | | Column resize (drag handle) | ✅ | Double-click to auto-fit | diff --git a/desktop/src/components/db-viewer/FilterBuilder.test.tsx b/desktop/src/components/db-viewer/FilterBuilder.test.tsx deleted file mode 100644 index 0016a84..0000000 --- a/desktop/src/components/db-viewer/FilterBuilder.test.tsx +++ /dev/null @@ -1,27 +0,0 @@ -import { describe, it, expect, vi } from "vitest"; -import { render, screen, fireEvent } from "@testing-library/react"; -import { FilterBuilder } from "./FilterBuilder"; -import type { ColumnInfo } from "../../lib/types"; - -const cols: ColumnInfo[] = [ - { name: "id", data_type: "integer", is_nullable: false, is_pk: true, is_fk: false, fk_ref: null, default_value: null, editable: false, is_generated: false }, - { name: "name", data_type: "text", is_nullable: true, is_pk: false, is_fk: false, fk_ref: null, default_value: null, editable: true, is_generated: false }, -]; - -describe("FilterBuilder", () => { - it("drops a column chip into the drop zone to create a rule with a type-aware operator", () => { - const onChange = vi.fn(); - render(); - fireEvent.click(screen.getByText("name")); - expect(onChange).toHaveBeenCalledWith([ - expect.objectContaining({ column: "name", operator: "contains" }), - ]); - }); - it("removing a rule calls onChange without it", () => { - const onChange = vi.fn(); - const rules = [{ id: "r1", column: "name", operator: "contains" as const, value: "Al" }]; - render(); - fireEvent.click(screen.getByLabelText("Remove filter name")); - expect(onChange).toHaveBeenCalledWith([]); - }); -}); \ No newline at end of file diff --git a/desktop/src/components/db-viewer/FilterBuilder.tsx b/desktop/src/components/db-viewer/FilterBuilder.tsx deleted file mode 100644 index 68a320d..0000000 --- a/desktop/src/components/db-viewer/FilterBuilder.tsx +++ /dev/null @@ -1,132 +0,0 @@ -import { useState } from "react"; -import { DndContext, useDraggable, useDroppable } from "@dnd-kit/core"; -import { X } from "lucide-react"; -import { defaultFilterOperator } from "../grid/gridEditability"; -import type { ColumnInfo } from "../../lib/types"; -import type { FilterRule, FilterOperator } from "../../stores/dbViewerStore"; - -interface Props { - columns: ColumnInfo[]; - rules: FilterRule[]; - onChange: (rules: FilterRule[]) => void; -} - -function Chip({ col, onAdd }: { col: ColumnInfo; onAdd: () => void }) { - const { setNodeRef, attributes, listeners, isDragging } = useDraggable({ - id: `col-${col.name}`, - data: { column: col }, - }); - return ( - - ); -} - -const OPERATORS: FilterOperator[] = [ - "eq", - "neq", - "contains", - "starts", - "ends", - "gt", - "lt", - "null", - "notnull", -]; - -export function FilterBuilder({ columns, rules, onChange }: Props) { - const [val, setVal] = useState>({}); - - const addRule = (col: ColumnInfo) => { - const op = defaultFilterOperator(col.data_type); - onChange([ - ...rules, - { - id: `f-${Date.now()}-${col.name}`, - column: col.name, - operator: op, - value: "", - }, - ]); - }; - - const { setNodeRef, isOver } = useDroppable({ id: "filter-dropzone" }); - - const remove = (id: string) => onChange(rules.filter((r) => r.id !== id)); - const update = (id: string, patch: Partial) => - onChange(rules.map((r) => (r.id === id ? { ...r, ...patch } : r))); - - return ( - { - const id = e.active.id as string; - const colName = id.replace(/^col-/, ""); - const col = columns.find((c) => c.name === colName); - if (col && e.over?.id === "filter-dropzone") addRule(col); - }} - > -
- {columns.map((c) => ( - addRule(c)} /> - ))} -
-
- {rules.length === 0 && ( - - Drop columns here to add filters - - )} - {rules.map((r) => ( -
- {r.column} - - {!["null", "notnull"].includes(r.operator) && ( - { - setVal({ ...val, [r.id]: e.target.value }); - update(r.id, { value: e.target.value }); - }} - className="bg-surface border border-border rounded px-1 flex-1" - placeholder="value" - /> - )} - -
- ))} -
- - ); -} \ No newline at end of file diff --git a/desktop/src/components/db-viewer/TableControls.test.tsx b/desktop/src/components/db-viewer/TableControls.test.tsx index 05ea9b2..744e00e 100644 --- a/desktop/src/components/db-viewer/TableControls.test.tsx +++ b/desktop/src/components/db-viewer/TableControls.test.tsx @@ -325,13 +325,100 @@ describe("TableControls", () => { expect(screen.queryByLabelText(/insert row/i)).toBeNull(); }); - it("renders the FilterBuilder inside the filter popover", () => { + it("renders the column filter popover with the + Add filter action", () => { seed([makeTab()], "tab-1"); renderControls(); fireEvent.click(screen.getByLabelText(/column filters/i)); - expect( - screen.getByText("Drop columns here to add filters"), - ).toBeInTheDocument(); + expect(screen.getByText("Column Filters")).toBeInTheDocument(); + expect(screen.getByText("+ Add filter")).toBeInTheDocument(); + }); + + it("debounces filter value edits so a query only fires after the user stops typing", () => { + vi.useFakeTimers(); + seed([makeTab()], "tab-1"); + const onFilterChange = vi.fn(); + renderControls({ + onFilterChange, + filterRules: [ + { id: "r1", column: "id", operator: "contains", value: "" }, + ], + }); + + fireEvent.click(screen.getByLabelText(/column filters/i)); + + const input = screen.getByPlaceholderText("value"); + fireEvent.change(input, { target: { value: "a" } }); + fireEvent.change(input, { target: { value: "ab" } }); + fireEvent.change(input, { target: { value: "abc" } }); + + // No commit yet — the debounce hasn't elapsed. + expect(onFilterChange).not.toHaveBeenCalled(); + + act(() => { + vi.advanceTimersByTime(400); + }); + + // Only the final value is committed, once. + expect(onFilterChange).toHaveBeenCalledTimes(1); + expect(onFilterChange).toHaveBeenCalledWith([ + expect.objectContaining({ value: "abc" }), + ]); + }); + + it("closes the filter popover when the button is clicked while it is open", () => { + seed([makeTab()], "tab-1"); + renderControls(); + + fireEvent.click(screen.getByLabelText(/column filters/i)); + expect(screen.getByText("Column Filters")).toBeInTheDocument(); + + // Clicking the button while open closes it (plain toggle). + fireEvent.click(screen.getByLabelText(/column filters/i)); + expect(screen.queryByText("Column Filters")).not.toBeInTheDocument(); + }); + + it("toggles the sort rules popover open and closed", () => { + seed([makeTab()], "tab-1"); + renderControls(); + + fireEvent.click(screen.getByLabelText(/sort rules/i)); + expect(screen.getByText("Sort Rules")).toBeInTheDocument(); + + fireEvent.click(screen.getByLabelText(/sort rules/i)); + expect(screen.queryByText("Sort Rules")).not.toBeInTheDocument(); + }); + + it("toggles the auto-refresh dropdown open and closed", () => { + seed([makeTab()], "tab-1"); + renderControls(); + + fireEvent.click(screen.getByLabelText(/auto-refresh/i)); + expect(screen.getByText("Off")).toBeInTheDocument(); + + fireEvent.click(screen.getByLabelText(/auto-refresh/i)); + expect(screen.queryByText("Off")).not.toBeInTheDocument(); + }); + + it("toggles the show/hide columns dropdown open and closed", () => { + seed([makeTab()], "tab-1"); + renderControls(); + + fireEvent.click(screen.getByLabelText(/toggle columns/i)); + expect(screen.getByText("Visible columns")).toBeInTheDocument(); + + fireEvent.click(screen.getByLabelText(/toggle columns/i)); + expect(screen.queryByText("Visible columns")).not.toBeInTheDocument(); + }); + + it("toggles the export dropdown open and closed", () => { + seed([makeTab()], "tab-1"); + renderControls(); + + fireEvent.click(screen.getByLabelText(/export/i)); + expect(screen.getByText("JSON")).toBeInTheDocument(); + + fireEvent.click(screen.getByLabelText(/export/i)); + expect(screen.queryByText("JSON")).not.toBeInTheDocument(); }); it("export dropdown includes the Excel option", () => { diff --git a/desktop/src/components/db-viewer/TableControls.tsx b/desktop/src/components/db-viewer/TableControls.tsx index fad18aa..759317d 100644 --- a/desktop/src/components/db-viewer/TableControls.tsx +++ b/desktop/src/components/db-viewer/TableControls.tsx @@ -6,7 +6,6 @@ import { } from "lucide-react"; import { useDbViewerStore, type FilterRule, type SortRule } from "../../stores/dbViewerStore"; import { useNotificationStore } from "../../stores/notificationStore"; -import { FilterBuilder } from "./FilterBuilder"; import { Tooltip } from "../ui/Tooltip"; import { exportData } from "../../lib/exportData"; import type { ColumnInfo } from "../../lib/types"; @@ -49,22 +48,28 @@ function DropdownMenu({ open, setOpen, align, + wrapRef, children, }: { open: boolean; setOpen: (v: boolean) => void; align?: "left" | "right"; + /** When provided, outside-click detection uses this wrapper (which should + * contain both the toggle button and the menu) so the button never fights + * the outside-click handler. */ + wrapRef?: React.RefObject; children: React.ReactNode; }) { const ref = useRef(null); useEffect(() => { if (!open) return; const close = (e: MouseEvent) => { - if (ref.current && !ref.current.contains(e.target as Node)) setOpen(false); + const container = wrapRef?.current ?? ref.current; + if (container && !container.contains(e.target as Node)) setOpen(false); }; document.addEventListener("mousedown", close); return () => document.removeEventListener("mousedown", close); - }, [open, setOpen]); + }, [open, setOpen, wrapRef]); if (!open) return null; return ( @@ -79,28 +84,84 @@ function DropdownMenu({ ); } +/** + * Debounced free-text input for filter values. The value is committed to the + * rules (and thus triggers a server-side refetch) only after the user stops + * typing for `DEBOUNCE_MS`. Without this, every keystroke fires a query and + * the in-flight guard in `fetchData` silently drops intermediate fetches, so + * the final filter value can end up not being applied. + */ +const FILTER_DEBOUNCE_MS = 400; + +function DebouncedValueInput({ + value, + onCommit, + placeholder, +}: { + value: string; + onCommit: (v: string) => void; + placeholder?: string; +}) { + const [draft, setDraft] = useState(value); + const timerRef = useRef | null>(null); + + // Keep the draft in sync when the committed value changes externally + // (e.g. a rule is replaced or removed while this input is mounted). + useEffect(() => { + setDraft(value); + }, [value]); + + useEffect(() => { + return () => { + if (timerRef.current) clearTimeout(timerRef.current); + }; + }, []); + + const handleChange = (e: React.ChangeEvent) => { + const v = e.target.value; + setDraft(v); + if (timerRef.current) clearTimeout(timerRef.current); + timerRef.current = setTimeout(() => onCommit(v), FILTER_DEBOUNCE_MS); + }; + + return ( + + ); +} + function FilterModal({ columns, rules, onChange, open, setOpen, + wrapRef, }: { columns: ColumnInfo[]; rules: FilterRule[]; onChange: (rules: FilterRule[]) => void; open: boolean; setOpen: (v: boolean) => void; + wrapRef: React.RefObject; }) { - const ref = useRef(null); + // Close on outside click, but treat the toggle button (which lives inside + // the same wrapper) as part of the modal so it never fights the toggle. useEffect(() => { if (!open) return; const close = (e: MouseEvent) => { - if (ref.current && !ref.current.contains(e.target as Node)) setOpen(false); + if (wrapRef.current && !wrapRef.current.contains(e.target as Node)) { + setOpen(false); + } }; document.addEventListener("mousedown", close); return () => document.removeEventListener("mousedown", close); - }, [open, setOpen]); + }, [open, setOpen, wrapRef]); if (!open) return null; @@ -116,17 +177,13 @@ function FilterModal({ onChange(rules.map((r) => (r.id === id ? { ...r, ...patch } : r))); return ( -
+
Column Filters
- {rules.map((rule) => (
{rule.operator !== "null" && rule.operator !== "notnull" && ( - updateRule(rule.id, { value: e.target.value })} + onCommit={(v) => updateRule(rule.id, { value: v })} placeholder="value" - className="flex-1 rounded border border-border bg-surface text-xs px-1.5 py-1 text-text min-w-0" /> )} - + - + {EXPORT_FORMATS.map((fmt) => ( - + {AUTO_REFRESH_OPTIONS.map((opt) => (
{/* Sort */} -
+