From c00f55256e650755c7dde18587caa863d8bb4665 Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Fri, 8 Dec 2023 13:27:59 -0500 Subject: [PATCH 01/21] rework of footer to match new design and improve readability --- .../chip/removable-list-chip/Footer.svelte | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/web-common/src/components/chip/removable-list-chip/Footer.svelte b/web-common/src/components/chip/removable-list-chip/Footer.svelte index 1ba6c126b93..0eebe05a980 100644 --- a/web-common/src/components/chip/removable-list-chip/Footer.svelte +++ b/web-common/src/components/chip/removable-list-chip/Footer.svelte @@ -1,5 +1,18 @@ -
+
-
+ + + From 847842515dc5c7b36a011333409c693bd2ed7820 Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Fri, 8 Dec 2023 13:32:55 -0500 Subject: [PATCH 02/21] adding filter button functionality, plus design tweaks to relevant menus --- .../RemovableListChip.svelte | 55 +++++++++++-- .../RemovableListMenu.svelte | 56 ++++++++----- .../src/components/search/Search.svelte | 11 ++- .../SearchableFilterDropdown.svelte | 4 +- .../dashboards/filters/FilterButton.svelte | 70 ++++++++++++++++ .../dashboards/filters/Filters.svelte | 82 +++++++++++-------- .../actions/dimension-filters.ts | 4 +- 7 files changed, 211 insertions(+), 71 deletions(-) create mode 100644 web-common/src/features/dashboards/filters/FilterButton.svelte diff --git a/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte b/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte index cbd5f749e7d..022f49c0c42 100644 --- a/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte +++ b/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte @@ -11,8 +11,9 @@ the name and then move the cursor to the right to cancel it. existing elements in the lib as well as changing the type (include, exclude) and enabling list search. The implementation of these parts are details left to the consumer of the component; this component should remain pure-ish (only internal state) if possible. --> - + { + toggleFloatingElement(); + dispatch("click"); + }} on:remove={() => dispatch("remove")} {active} {...colors} @@ -91,14 +113,29 @@ are details left to the consumer of the component; this component should remain { + if (!selectedValues.length) { + clearFilterForDimension(StateManagers, dimensionName, !excludeMode); + } else { + toggleFloatingElement(); + } + }} + on:click-outside={() => { + if (!selectedValues.length) { + clearFilterForDimension(StateManagers, dimensionName, !excludeMode); + } else { + toggleFloatingElement(); + } + }} + on:keep-alive={() => { + toggleDimensionValueSelection(dimensionName); + }} on:apply on:search on:toggle - {selectedValues} - {searchedValues} /> diff --git a/web-common/src/components/chip/removable-list-chip/RemovableListMenu.svelte b/web-common/src/components/chip/removable-list-chip/RemovableListMenu.svelte index 006e5a2a951..3f923d88fbb 100644 --- a/web-common/src/components/chip/removable-list-chip/RemovableListMenu.svelte +++ b/web-common/src/components/chip/removable-list-chip/RemovableListMenu.svelte @@ -8,8 +8,9 @@ import { Menu, MenuItem } from "../../menu"; import { Search } from "../../search"; import Footer from "./Footer.svelte"; + import Button from "../../button/Button.svelte"; - export let excludeStore: Writable; + export let excludeMode: boolean; export let selectedValues: string[]; export let searchedValues: string[] | null = []; @@ -33,7 +34,7 @@ let valuesToDisplay = [...candidateValues]; // If searchedValues === null, search has not finished yet. So continue rendering the previous list - $: if (searchText && searchedValues !== null) { + $: if (searchedValues !== null) { valuesToDisplay = [...searchedValues]; } else if (!searchText) valuesToDisplay = [...candidateValues]; @@ -41,13 +42,26 @@ (v) => !valuesToDisplay.includes(v) ).length; - function toggleValue(value) { + function toggleValue(value: string) { dispatch("apply", value); if (!candidateValues.includes(value)) { candidateValues = [...candidateValues, value]; } } + + function toggleSelectAll() { + valuesToDisplay.forEach((value) => { + if (!allSelected && selectedValues.includes(value)) return; + + toggleValue(value); + }); + + if (allSelected) dispatch("keep-alive"); + } + + $: allSelected = + selectedValues?.length && valuesToDisplay.length === selectedValues.length; -
+
@@ -85,9 +100,9 @@ }} > - {#if selectedValues.includes(value) && !$excludeStore} + {#if selectedValues.includes(value) && !excludeMode} - {:else if selectedValues.includes(value) && $excludeStore} + {:else if selectedValues.includes(value) && excludeMode} {:else} @@ -95,7 +110,7 @@ {#if value?.length > 240} {value.slice(0, 240)}... @@ -110,17 +125,20 @@ {/if}
- - onToggleHandler()} checked={$excludeStore}> + + +
diff --git a/web-common/src/components/search/Search.svelte b/web-common/src/components/search/Search.svelte index 1dc6de3b200..76637c22832 100644 --- a/web-common/src/components/search/Search.svelte +++ b/web-common/src/components/search/Search.svelte @@ -38,9 +38,8 @@ bind:this={ref} type="text" autocomplete="off" - class="bg-white border border-gray-200 {showBorderOnFocus - ? 'focus:border-blue-400' - : ''} outline-none rounded-sm block w-full pl-8 p-1" + class:focus={showBorderOnFocus} + class="bg-slate-50 border border-gray-200 outline-none rounded-sm block w-full pl-8 p-1" {placeholder} bind:value on:input @@ -48,3 +47,9 @@ aria-label={label} /> + + diff --git a/web-common/src/components/searchable-filter-menu/SearchableFilterDropdown.svelte b/web-common/src/components/searchable-filter-menu/SearchableFilterDropdown.svelte index fd1aabba432..6e58669582a 100644 --- a/web-common/src/components/searchable-filter-menu/SearchableFilterDropdown.svelte +++ b/web-common/src/components/searchable-filter-menu/SearchableFilterDropdown.svelte @@ -76,7 +76,7 @@ rounded={false} > -
+
@@ -99,8 +99,6 @@ ? "#9CA3AF" : "#15141A"} /> - {:else} - {/if} diff --git a/web-common/src/features/dashboards/filters/FilterButton.svelte b/web-common/src/features/dashboards/filters/FilterButton.svelte new file mode 100644 index 00000000000..6503841aa7c --- /dev/null +++ b/web-common/src/features/dashboards/filters/FilterButton.svelte @@ -0,0 +1,70 @@ + + + + + + + + Add filter + + + { + toggleFloatingElement(); + toggleDimensionValueSelection(e.detail.name); + }} + /> + + + diff --git a/web-common/src/features/dashboards/filters/Filters.svelte b/web-common/src/features/dashboards/filters/Filters.svelte index cc5561e1fc2..19c7fdf98f1 100644 --- a/web-common/src/features/dashboards/filters/Filters.svelte +++ b/web-common/src/features/dashboards/filters/Filters.svelte @@ -29,6 +29,7 @@ The main feature-set component for dashboard filters toggleDimensionValue, toggleFilterMode, } from "../actions"; + import FilterButton from "./FilterButton.svelte"; const StateManagers = getStateManagers(); const { dashboardStore } = StateManagers; @@ -54,14 +55,16 @@ The main feature-set component for dashboard filters activeDimensionName; let topListQuery: ReturnType | undefined; - $: if (activeDimensionName) + $: if (activeDimensionName) { + console.log({ activeDimensionName, searchText }); topListQuery = getFilterSearchList(StateManagers, { dimension: activeDimensionName, searchText, addNull: "null".includes(searchText), }); + } - $: if (!$topListQuery?.isFetching && searchText != "") { + $: if (!$topListQuery?.isFetching) { const topListData = $topListQuery?.data?.data ?? []; searchedValues = topListData.map((datum) => datum[activeColumn]) ?? []; } @@ -120,7 +123,8 @@ The main feature-set component for dashboard filters currentDimensionFilters.sort((a, b) => (a.name > b.name ? 1 : -1)); } - function setActiveDimension(name, value) { + function setActiveDimension(name, value = "") { + console.log({ name, value }); activeDimensionName = name; searchText = value; } @@ -144,8 +148,17 @@ The main feature-set component for dashboard filters >
- {#if currentDimensionFilters.length > 0} - + + + {#if !currentDimensionFilters.length} +
+ No filters selected +
+ {:else} {#each currentDimensionFilters as { name, label, selectedValues, filterType } (name)} {@const isInclude = filterType === "include"}
@@ -162,7 +175,15 @@ The main feature-set component for dashboard filters on:search={(event) => { setActiveDimension(name, event.detail); }} + on:click={(event) => { + console.log("CLICCKKKKCCCKK"); + setActiveDimension(name, ""); + }} + on:mount={() => { + setActiveDimension(name); + }} typeLabel="dimension" + dimensionName={name} name={isInclude ? label : `Exclude ${label}`} excludeMode={isInclude ? false : true} colors={getColorForChip(isInclude)} @@ -176,35 +197,26 @@ The main feature-set component for dashboard filters
{/each} - - {#if hasFilters} -
- clearAllFilters(StateManagers)} - > - - - - Clear filters - -
- {/if} -
- {:else if currentDimensionFilters.length === 0} -
- No filters selected -
- {:else} -   - {/if} + {#if hasFilters} +
+ clearAllFilters(StateManagers)} + > + + + + Clear filters + +
+ {/if} +
diff --git a/web-common/src/features/dashboards/state-managers/actions/dimension-filters.ts b/web-common/src/features/dashboards/state-managers/actions/dimension-filters.ts index f163d81720f..adad8f5e3a9 100644 --- a/web-common/src/features/dashboards/state-managers/actions/dimension-filters.ts +++ b/web-common/src/features/dashboards/state-managers/actions/dimension-filters.ts @@ -5,7 +5,7 @@ import { filtersForCurrentExcludeMode } from "../selectors/dimension-filters"; export function toggleDimensionValueSelection( { dashboard, cancelQueries }: DashboardMutables, dimensionName: string, - dimensionValue: string + dimensionValue?: string ) { const filters = filtersForCurrentExcludeMode({ dashboard })(dimensionName); // if there are no filters at this point we cannot update anything. @@ -33,7 +33,7 @@ export function toggleDimensionValueSelection( } else { filters.push({ name: dimensionName, - in: [dimensionValue], + in: dimensionValue ? [dimensionValue] : [], }); } } From 71acf1f7de3128bb442daeaef67f9eea9267ba4f Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Fri, 8 Dec 2023 13:33:07 -0500 Subject: [PATCH 03/21] new icon --- .../src/components/icons/ChevronRight.svelte | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 web-common/src/components/icons/ChevronRight.svelte diff --git a/web-common/src/components/icons/ChevronRight.svelte b/web-common/src/components/icons/ChevronRight.svelte new file mode 100644 index 00000000000..2ea1015102b --- /dev/null +++ b/web-common/src/components/icons/ChevronRight.svelte @@ -0,0 +1,19 @@ + + + + + From c68da3f8dc2ecad31b9d4805d45bceb68e7dfe41 Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Fri, 8 Dec 2023 19:39:03 -0500 Subject: [PATCH 04/21] chip should not be removed when deselecting the only selected value --- web-common/src/features/dashboards/actions/index.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/web-common/src/features/dashboards/actions/index.ts b/web-common/src/features/dashboards/actions/index.ts index 8957e315bff..4bc590add09 100644 --- a/web-common/src/features/dashboards/actions/index.ts +++ b/web-common/src/features/dashboards/actions/index.ts @@ -71,12 +71,6 @@ export function toggleDimensionValue( index, 1 ); - if ( - dashboard.filters[relevantFilterKey][dimensionEntryIndex].in - .length === 0 - ) { - dashboard.filters[relevantFilterKey].splice(dimensionEntryIndex, 1); - } // Only decrement pinIndex if the removed value was before the pinned value if (dashboard.pinIndex >= index) { From 0bb7d8532048cda1622a7f672b494374ba87adf5 Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Fri, 8 Dec 2023 19:39:56 -0500 Subject: [PATCH 05/21] removing limit from filter list query --- web-common/src/features/dashboards/selectors/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/web-common/src/features/dashboards/selectors/index.ts b/web-common/src/features/dashboards/selectors/index.ts index dd9f11f8bd5..2765d8149e4 100644 --- a/web-common/src/features/dashboards/selectors/index.ts +++ b/web-common/src/features/dashboards/selectors/index.ts @@ -73,7 +73,6 @@ export const getFilterSearchList = ( measureNames: [metricsExplorer.leaderboardMeasureName], timeStart: timeControls.timeStart, timeEnd: timeControls.timeEnd, - limit: "15", offset: "0", sort: [], filter: { From 8cc77d37a02d0dedc98026c239c1cdd207cbd033 Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Fri, 8 Dec 2023 19:42:49 -0500 Subject: [PATCH 06/21] changed searchedValues to allValues, simplified display logic --- .../RemovableListChip.svelte | 7 ++-- .../RemovableListMenu.svelte | 35 ++++--------------- .../dashboards/filters/Filters.svelte | 17 +++++---- 3 files changed, 16 insertions(+), 43 deletions(-) diff --git a/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte b/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte index 022f49c0c42..239749a49ec 100644 --- a/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte +++ b/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte @@ -31,7 +31,7 @@ are details left to the consumer of the component; this component should remain
- {#if valuesToDisplay.length} - {#each valuesToDisplay as value} + {#if allValues?.length} + {#each allValues.sort() as value} d.name === activeDimensionName)?.column ?? activeDimensionName; let topListQuery: ReturnType | undefined; $: if (activeDimensionName) { - console.log({ activeDimensionName, searchText }); topListQuery = getFilterSearchList(StateManagers, { dimension: activeDimensionName, searchText, @@ -66,7 +66,7 @@ The main feature-set component for dashboard filters $: if (!$topListQuery?.isFetching) { const topListData = $topListQuery?.data?.data ?? []; - searchedValues = topListData.map((datum) => datum[activeColumn]) ?? []; + allValues = topListData.map((datum) => datum[activeColumn]) ?? []; } $: hasFilters = isFiltered($dashboardStore.filters); @@ -124,7 +124,6 @@ The main feature-set component for dashboard filters } function setActiveDimension(name, value = "") { - console.log({ name, value }); activeDimensionName = name; searchText = value; } @@ -170,13 +169,13 @@ The main feature-set component for dashboard filters name, isInclude ? true : false )} - on:apply={(event) => - toggleDimensionValue(StateManagers, name, event.detail)} + on:apply={(event) => { + toggleDimensionValue(StateManagers, name, event.detail); + }} on:search={(event) => { setActiveDimension(name, event.detail); }} on:click={(event) => { - console.log("CLICCKKKKCCCKK"); setActiveDimension(name, ""); }} on:mount={() => { @@ -189,7 +188,7 @@ The main feature-set component for dashboard filters colors={getColorForChip(isInclude)} label="View filter" {selectedValues} - {searchedValues} + {allValues} > Click to edit the the filters in this dimension From f1399b0eec3492ca0f488f096aa7a2d133aecb61 Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Fri, 8 Dec 2023 19:43:10 -0500 Subject: [PATCH 07/21] updated tests to meet updated requirements --- .../RemovableListMenu.spec.ts | 53 ++++++++++--------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/web-common/src/components/chip/removable-list-chip/RemovableListMenu.spec.ts b/web-common/src/components/chip/removable-list-chip/RemovableListMenu.spec.ts index 8f3f50f1310..35ee88a02e4 100644 --- a/web-common/src/components/chip/removable-list-chip/RemovableListMenu.spec.ts +++ b/web-common/src/components/chip/removable-list-chip/RemovableListMenu.spec.ts @@ -1,28 +1,31 @@ import RemovableListMenu from "./RemovableListMenu.svelte"; import { describe, it, expect, vi } from "vitest"; import { render, waitFor, fireEvent, screen } from "@testing-library/svelte"; -import { writable } from "svelte/store"; describe("RemovableListMenu", () => { - it("renders selected values by default", async () => { + it("does not render selected values if not in all values", async () => { const { unmount } = render(RemovableListMenu, { - excludeStore: writable(false), - selectedValues: ["foo", "bar"], - searchedValues: null, + excludeMode: false, + selectedValues: ["x"], + allValues: ["foo", "bar"], }); const foo = screen.getByText("foo"); const bar = screen.getByText("bar"); expect(foo).toBeDefined(); expect(bar).toBeDefined(); + + const x = screen.queryByText("x"); + expect(x).toBeNull(); + unmount(); }); - it("renders selected values if search text is empty", async () => { + it("renders all values if search text is empty", async () => { const { unmount } = render(RemovableListMenu, { - excludeStore: writable(false), - selectedValues: ["foo", "bar"], - searchedValues: ["x"], + excludeMode: false, + selectedValues: [], + allValues: ["foo", "bar"], }); const foo = screen.getByText("foo"); @@ -34,9 +37,9 @@ describe("RemovableListMenu", () => { it("renders search values if search text is populated", async () => { const { unmount } = render(RemovableListMenu, { - excludeStore: writable(false), + excludeMode: false, selectedValues: ["foo", "bar"], - searchedValues: ["x"], + allValues: ["x"], }); const searchInput = screen.getByRole("textbox", { name: "Search list" }); @@ -51,35 +54,33 @@ describe("RemovableListMenu", () => { }); it("should render switch based on exclude store", async () => { - const excludeStore = writable(false); - const { unmount } = render(RemovableListMenu, { - excludeStore, + const { unmount, component } = render(RemovableListMenu, { + excludeMode: false, selectedValues: ["foo", "bar"], - searchedValues: ["x"], + allValues: ["x"], }); - const switchInput = screen.getByRole("switch"); - expect(switchInput.checked).toBe(false); + const switchInput = screen.getByText("Exclude"); + expect(switchInput).toBeDefined(); - excludeStore.set(true); - await waitFor(() => { - expect(switchInput.checked).toBe(true); - }); + await component.$set({ excludeMode: true }); + + const includeButton = screen.getByText("Include"); + expect(includeButton).toBeDefined(); unmount(); }); it("should dispatch toggle, apply, and search events", async () => { - const excludeStore = writable(false); const { unmount, component } = render(RemovableListMenu, { - excludeStore, - selectedValues: ["foo", "bar"], - searchedValues: ["x"], + excludeMode: false, + selectedValues: [], + allValues: ["foo", "bar"], }); const toggleSpy = vi.fn(); component.$on("toggle", toggleSpy); - const switchInput = screen.getByRole("switch"); + const switchInput = screen.getByText("Exclude"); await fireEvent.click(switchInput); expect(toggleSpy).toHaveBeenCalledOnce(); From d2b0de56c14e5b88deb7a97ad71572a420dc44e3 Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Fri, 8 Dec 2023 19:53:37 -0500 Subject: [PATCH 08/21] prettier unused vars build fix --- .../chip/removable-list-chip/RemovableListChip.svelte | 1 - .../chip/removable-list-chip/RemovableListMenu.svelte | 5 ----- .../searchable-filter-menu/SearchableFilterDropdown.svelte | 1 - web-common/src/features/dashboards/filters/Filters.svelte | 2 +- 4 files changed, 1 insertion(+), 8 deletions(-) diff --git a/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte b/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte index 239749a49ec..37f4b10a470 100644 --- a/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte +++ b/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte @@ -23,7 +23,6 @@ are details left to the consumer of the component; this component should remain import { Chip } from "../index"; import RemovableListBody from "./RemovableListBody.svelte"; import RemovableListMenu from "./RemovableListMenu.svelte"; - import { writable, Writable } from "svelte/store"; import { clearFilterForDimension } from "@rilldata/web-common/features/dashboards/actions"; import { getStateManagers } from "@rilldata/web-common/features/dashboards/state-managers/state-managers"; diff --git a/web-common/src/components/chip/removable-list-chip/RemovableListMenu.svelte b/web-common/src/components/chip/removable-list-chip/RemovableListMenu.svelte index eb88ad4c75b..6c45151be73 100644 --- a/web-common/src/components/chip/removable-list-chip/RemovableListMenu.svelte +++ b/web-common/src/components/chip/removable-list-chip/RemovableListMenu.svelte @@ -20,10 +20,6 @@ dispatch("search", searchText); } - function onToggleHandler() { - dispatch("toggle"); - } - function toggleValue(value: string) { dispatch("apply", value); } @@ -54,7 +50,6 @@ on:click-outside > -
import type { SearchableFilterSelectableItem } from "@rilldata/web-common/components/searchable-filter-menu/SearchableFilterSelectableItem"; import Check from "../icons/Check.svelte"; - import Spacer from "../icons/Spacer.svelte"; import { Menu, MenuItem } from "../menu"; import { Search } from "../search"; import Footer from "./Footer.svelte"; diff --git a/web-common/src/features/dashboards/filters/Filters.svelte b/web-common/src/features/dashboards/filters/Filters.svelte index 26a591fa5db..682485de082 100644 --- a/web-common/src/features/dashboards/filters/Filters.svelte +++ b/web-common/src/features/dashboards/filters/Filters.svelte @@ -175,7 +175,7 @@ The main feature-set component for dashboard filters on:search={(event) => { setActiveDimension(name, event.detail); }} - on:click={(event) => { + on:click={() => { setActiveDimension(name, ""); }} on:mount={() => { From 0f0290ebec88fd3f0de83525b69bd1227d466cbf Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Thu, 14 Dec 2023 10:56:19 -0500 Subject: [PATCH 09/21] remove keep-alive event --- .../chip/removable-list-chip/RemovableListChip.svelte | 3 --- .../chip/removable-list-chip/RemovableListMenu.svelte | 2 -- 2 files changed, 5 deletions(-) diff --git a/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte b/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte index ce1168db3ab..581cb5537cb 100644 --- a/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte +++ b/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte @@ -127,9 +127,6 @@ are details left to the consumer of the component; this component should remain toggleFloatingElement(); } }} - on:keep-alive={() => { - toggleDimensionValueSelection(dimensionName); - }} on:apply on:search on:toggle diff --git a/web-common/src/components/chip/removable-list-chip/RemovableListMenu.svelte b/web-common/src/components/chip/removable-list-chip/RemovableListMenu.svelte index 6c45151be73..245e49f389c 100644 --- a/web-common/src/components/chip/removable-list-chip/RemovableListMenu.svelte +++ b/web-common/src/components/chip/removable-list-chip/RemovableListMenu.svelte @@ -30,8 +30,6 @@ toggleValue(value); }); - - if (allSelected) dispatch("keep-alive"); } $: allSelected = From bdac8f8c816bca629fe3b71fcd7354afb619db91 Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Thu, 14 Dec 2023 11:46:12 -0500 Subject: [PATCH 10/21] update let directive for named slot, add timeout to dashboard.spec --- .../chip/removable-list-chip/RemovableListChip.svelte | 1 + web-common/src/features/dashboards/filters/FilterButton.svelte | 1 + web-local/test/ui/dashboards.spec.ts | 2 ++ 3 files changed, 4 insertions(+) diff --git a/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte b/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte index 581cb5537cb..9784aebcb1d 100644 --- a/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte +++ b/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte @@ -109,6 +109,7 @@ are details left to the consumer of the component; this component should remain
{ // Filter to Facebook via leaderboard await page.getByRole("button", { name: "Facebook 19.3k" }).click(); + await page.waitForTimeout(100); + // Change filter to excluded await page.getByText("Publisher Facebook").click(); await page.getByRole("button", { name: "Exclude" }).click(); From 90d5085909c0a4b6686ceba08ca30e02c0f40be0 Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Thu, 14 Dec 2023 12:00:11 -0500 Subject: [PATCH 11/21] remove unused function --- .../chip/removable-list-chip/RemovableListChip.svelte | 6 ------ 1 file changed, 6 deletions(-) diff --git a/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte b/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte index 9784aebcb1d..e15621a90b3 100644 --- a/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte +++ b/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte @@ -43,12 +43,6 @@ are details left to the consumer of the component; this component should remain const StateManagers = getStateManagers(); - const { - actions: { - dimensionsFilter: { toggleDimensionValueSelection }, - }, - } = StateManagers; - onMount(() => { dispatch("mount"); }); From c8af4eb27d27d110069ce4c1541da7bb2a21b3ad Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Thu, 14 Dec 2023 13:23:01 -0500 Subject: [PATCH 12/21] change to active logic when mounting chip component --- .../chip/removable-list-chip/RemovableListChip.svelte | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte b/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte index e15621a90b3..1f15c069701 100644 --- a/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte +++ b/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte @@ -39,6 +39,8 @@ are details left to the consumer of the component; this component should remain export let label: string | undefined = undefined; export let dimensionName: string; + const active = !selectedValues.length; + const dispatch = createEventDispatcher(); const StateManagers = getStateManagers(); @@ -53,7 +55,7 @@ are details left to the consumer of the component; this component should remain let:active distance={8} alignment="start" - active={true} + {active} > Date: Thu, 14 Dec 2023 13:56:37 -0500 Subject: [PATCH 13/21] resolve name collision --- .../chip/removable-list-chip/RemovableListChip.svelte | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte b/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte index 1f15c069701..ac690557019 100644 --- a/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte +++ b/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte @@ -39,7 +39,7 @@ are details left to the consumer of the component; this component should remain export let label: string | undefined = undefined; export let dimensionName: string; - const active = !selectedValues.length; + const initiallyActive = !selectedValues.length; const dispatch = createEventDispatcher(); @@ -55,7 +55,7 @@ are details left to the consumer of the component; this component should remain let:active distance={8} alignment="start" - {active} + active={initiallyActive} > Date: Fri, 15 Dec 2023 05:53:00 -0500 Subject: [PATCH 14/21] update page wait from timeout to selector --- web-local/test/ui/dashboards.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web-local/test/ui/dashboards.spec.ts b/web-local/test/ui/dashboards.spec.ts index 3e772a82328..79cd8bee52c 100644 --- a/web-local/test/ui/dashboards.spec.ts +++ b/web-local/test/ui/dashboards.spec.ts @@ -245,7 +245,7 @@ test.describe("dashboard", () => { // Filter to Facebook via leaderboard await page.getByRole("button", { name: "Facebook 19.3k" }).click(); - await page.waitForTimeout(100); + await page.waitForSelector("text=Publisher Facebook"); // Change filter to excluded await page.getByText("Publisher Facebook").click(); From 164a0928a4191c8afedae8068d27433c60a57094 Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Fri, 15 Dec 2023 05:53:32 -0500 Subject: [PATCH 15/21] added specific method for adding a dimension name without a value --- .../actions/dimension-filters.ts | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/web-common/src/features/dashboards/state-managers/actions/dimension-filters.ts b/web-common/src/features/dashboards/state-managers/actions/dimension-filters.ts index adad8f5e3a9..0505108d9fa 100644 --- a/web-common/src/features/dashboards/state-managers/actions/dimension-filters.ts +++ b/web-common/src/features/dashboards/state-managers/actions/dimension-filters.ts @@ -5,7 +5,7 @@ import { filtersForCurrentExcludeMode } from "../selectors/dimension-filters"; export function toggleDimensionValueSelection( { dashboard, cancelQueries }: DashboardMutables, dimensionName: string, - dimensionValue?: string + dimensionValue: string ) { const filters = filtersForCurrentExcludeMode({ dashboard })(dimensionName); // if there are no filters at this point we cannot update anything. @@ -33,7 +33,31 @@ export function toggleDimensionValueSelection( } else { filters.push({ name: dimensionName, - in: dimensionValue ? [dimensionValue] : [], + in: [dimensionValue], + }); + } +} + +export function toggleDimensionNameSelection( + { dashboard, cancelQueries }: DashboardMutables, + dimensionName: string +) { + const filters = filtersForCurrentExcludeMode({ dashboard })(dimensionName); + // if there are no filters at this point we cannot update anything. + if (filters === undefined) { + return; + } + + // if we are able to update the filters, we must cancel any queries + // that are currently running. + cancelQueries(); + + const hasFilter = filters.find((filter) => filter.name === dimensionName); + + if (!hasFilter) { + filters.push({ + name: dimensionName, + in: [], }); } } @@ -48,4 +72,5 @@ export const dimensionFilterActions = { * the include/exclude mode is a toggle for the entire dimension. */ toggleDimensionValueSelection, + toggleDimensionNameSelection, }; From 154da92d68feccdcb9abec05ca56868a9ff62deb Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Fri, 15 Dec 2023 05:54:26 -0500 Subject: [PATCH 16/21] only unselected dimension names are shown in the add filter dropdown --- .../dashboards/filters/FilterButton.svelte | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/web-common/src/features/dashboards/filters/FilterButton.svelte b/web-common/src/features/dashboards/filters/FilterButton.svelte index 0d7ee6af365..43ef4d67456 100644 --- a/web-common/src/features/dashboards/filters/FilterButton.svelte +++ b/web-common/src/features/dashboards/filters/FilterButton.svelte @@ -11,18 +11,31 @@ const { selectors: { dimensions: { allDimensions }, + dimensionFilters: { getAllFilters, isFilterExcludeMode }, }, actions: { - dimensionsFilter: { toggleDimensionValueSelection }, + dimensionsFilter: { toggleDimensionNameSelection }, }, } = getStateManagers(); - $: selectableItems = $allDimensions - ? $allDimensions.map((d) => ({ + function filterExists(name: string, exclude: boolean): boolean { + const selected = exclude + ? $getAllFilters?.exclude + : $getAllFilters?.include; + + return selected?.find((f) => f.name === name) !== undefined; + } + + $: selectableItems = + $allDimensions + ?.map((d) => ({ name: d.name as string, label: d.label as string, })) - : []; + .filter((d) => { + const exclude = $isFilterExcludeMode(d.name); + return !filterExists(d.name, exclude); + }) ?? []; { toggleFloatingElement(); - toggleDimensionValueSelection(e.detail.name); + toggleDimensionNameSelection(e.detail.name); }} /> From ed91037b78a5dc84e34d7fb12c4c36dd03043d0c Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Fri, 15 Dec 2023 10:18:59 -0500 Subject: [PATCH 17/21] add back limit --- web-common/src/features/dashboards/selectors/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/web-common/src/features/dashboards/selectors/index.ts b/web-common/src/features/dashboards/selectors/index.ts index 2765d8149e4..8c0025ea11b 100644 --- a/web-common/src/features/dashboards/selectors/index.ts +++ b/web-common/src/features/dashboards/selectors/index.ts @@ -73,6 +73,7 @@ export const getFilterSearchList = ( measureNames: [metricsExplorer.leaderboardMeasureName], timeStart: timeControls.timeStart, timeEnd: timeControls.timeEnd, + // limit: "100", offset: "0", sort: [], filter: { From 8f7d72029c3711224a59ef4ae95f7d1112c52f5f Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Fri, 15 Dec 2023 10:19:24 -0500 Subject: [PATCH 18/21] toggleDimensionNameSelection is now actually a toggling function --- .../state-managers/actions/dimension-filters.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/web-common/src/features/dashboards/state-managers/actions/dimension-filters.ts b/web-common/src/features/dashboards/state-managers/actions/dimension-filters.ts index 0505108d9fa..2c56935608b 100644 --- a/web-common/src/features/dashboards/state-managers/actions/dimension-filters.ts +++ b/web-common/src/features/dashboards/state-managers/actions/dimension-filters.ts @@ -52,13 +52,17 @@ export function toggleDimensionNameSelection( // that are currently running. cancelQueries(); - const hasFilter = filters.find((filter) => filter.name === dimensionName); + const filterIndex = filters.findIndex( + (filter) => filter.name === dimensionName + ); - if (!hasFilter) { + if (filterIndex === -1) { filters.push({ name: dimensionName, in: [], }); + } else { + filters.splice(filterIndex, 1); } } From 1de1107c3e1b1ab6e2860a9af421d187119d4d9f Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Fri, 15 Dec 2023 10:22:07 -0500 Subject: [PATCH 19/21] dispatch remove event rather than handle remove directly, plus bind to active property --- .../RemovableListChip.svelte | 35 +++++++------------ 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte b/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte index ac690557019..dfbbb9499a5 100644 --- a/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte +++ b/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte @@ -23,8 +23,6 @@ are details left to the consumer of the component; this component should remain import { Chip } from "../index"; import RemovableListBody from "./RemovableListBody.svelte"; import RemovableListMenu from "./RemovableListMenu.svelte"; - import { clearFilterForDimension } from "@rilldata/web-common/features/dashboards/actions"; - import { getStateManagers } from "@rilldata/web-common/features/dashboards/state-managers/state-managers"; { - if (!selectedValues.length) { - clearFilterForDimension(StateManagers, dimensionName, !excludeMode); - } else { - toggleFloatingElement(); - } - }} - on:click-outside={() => { - if (!selectedValues.length) { - clearFilterForDimension(StateManagers, dimensionName, !excludeMode); - } else { - toggleFloatingElement(); - } - }} + on:escape={handleDismiss} + on:click-outside={handleDismiss} on:apply on:search on:toggle From 2d52fe9fedc869c839f7434c36f70976094894cd Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Fri, 15 Dec 2023 10:22:47 -0500 Subject: [PATCH 20/21] use state manager action and prop cleanup --- .../features/dashboards/filters/Filters.svelte | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/web-common/src/features/dashboards/filters/Filters.svelte b/web-common/src/features/dashboards/filters/Filters.svelte index 682485de082..505edb3e07a 100644 --- a/web-common/src/features/dashboards/filters/Filters.svelte +++ b/web-common/src/features/dashboards/filters/Filters.svelte @@ -25,14 +25,18 @@ The main feature-set component for dashboard filters import { getStateManagers } from "../state-managers/state-managers"; import { clearAllFilters, - clearFilterForDimension, toggleDimensionValue, toggleFilterMode, } from "../actions"; import FilterButton from "./FilterButton.svelte"; const StateManagers = getStateManagers(); - const { dashboardStore } = StateManagers; + const { + dashboardStore, + actions: { + dimensionsFilter: { toggleDimensionNameSelection }, + }, + } = StateManagers; /** the height of a row of chips */ const ROW_HEIGHT = "26px"; @@ -163,12 +167,7 @@ The main feature-set component for dashboard filters
toggleFilterMode(StateManagers, name)} - on:remove={() => - clearFilterForDimension( - StateManagers, - name, - isInclude ? true : false - )} + on:remove={() => toggleDimensionNameSelection(name)} on:apply={(event) => { toggleDimensionValue(StateManagers, name, event.detail); }} @@ -182,7 +181,6 @@ The main feature-set component for dashboard filters setActiveDimension(name); }} typeLabel="dimension" - dimensionName={name} name={isInclude ? label : `Exclude ${label}`} excludeMode={isInclude ? false : true} colors={getColorForChip(isInclude)} From b5f794f36c512d2431666118c538ec2a179f4d40 Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Fri, 15 Dec 2023 10:23:19 -0500 Subject: [PATCH 21/21] add back limit --- web-common/src/features/dashboards/selectors/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web-common/src/features/dashboards/selectors/index.ts b/web-common/src/features/dashboards/selectors/index.ts index 8c0025ea11b..f448754a770 100644 --- a/web-common/src/features/dashboards/selectors/index.ts +++ b/web-common/src/features/dashboards/selectors/index.ts @@ -73,7 +73,7 @@ export const getFilterSearchList = ( measureNames: [metricsExplorer.leaderboardMeasureName], timeStart: timeControls.timeStart, timeEnd: timeControls.timeEnd, - // limit: "100", + limit: "100", offset: "0", sort: [], filter: {