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 @@ -
+
+ + + 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 c34207ef3d0..dfbbb9499a5 100644 --- a/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte +++ b/web-common/src/components/chip/removable-list-chip/RemovableListChip.svelte @@ -11,9 +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. --> - + @@ -55,7 +68,10 @@ are details left to the consumer of the component; this component should remain > { + toggleFloatingElement(); + dispatch("click"); + }} on:remove={() => dispatch("remove")} {active} {...colors} @@ -91,15 +107,14 @@ are details left to the consumer of the component; this component should remain 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(); 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..245e49f389c 100644 --- a/web-common/src/components/chip/removable-list-chip/RemovableListMenu.svelte +++ b/web-common/src/components/chip/removable-list-chip/RemovableListMenu.svelte @@ -1,17 +1,16 @@ - -
+
- {#if valuesToDisplay.length} - {#each valuesToDisplay as value} + {#if allValues?.length} + {#each allValues.sort() as value} - {#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 +80,7 @@ {#if value?.length > 240} {value.slice(0, 240)}... @@ -110,17 +95,20 @@ {/if}
- - onToggleHandler()} checked={$excludeStore}> + + +
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 @@ + + + + + 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..9b27a5cced4 100644 --- a/web-common/src/components/searchable-filter-menu/SearchableFilterDropdown.svelte +++ b/web-common/src/components/searchable-filter-menu/SearchableFilterDropdown.svelte @@ -1,7 +1,6 @@ + + + + + + + Add filter + + + { + toggleFloatingElement(); + toggleDimensionNameSelection(e.detail.name); + }} + /> + + + diff --git a/web-common/src/features/dashboards/filters/Filters.svelte b/web-common/src/features/dashboards/filters/Filters.svelte index a159fa90fdb..505edb3e07a 100644 --- a/web-common/src/features/dashboards/filters/Filters.svelte +++ b/web-common/src/features/dashboards/filters/Filters.svelte @@ -25,13 +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"; @@ -47,23 +52,25 @@ The main feature-set component for dashboard filters } let searchText = ""; - let searchedValues: string[] | null = null; - let activeDimensionName; + let allValues: string[] | null = null; + let activeDimensionName: string; + $: activeColumn = dimensions.find((d) => d.name === activeDimensionName)?.column ?? activeDimensionName; let topListQuery: ReturnType | undefined; - $: if (activeDimensionName) + $: if (activeDimensionName) { 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]) ?? []; + allValues = topListData.map((datum) => datum[activeColumn]) ?? []; } $: hasFilters = isFiltered($dashboardStore.filters); @@ -120,7 +127,7 @@ 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 = "") { activeDimensionName = name; searchText = value; } @@ -144,31 +151,42 @@ 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"}
toggleFilterMode(StateManagers, name)} - on:remove={() => - clearFilterForDimension( - StateManagers, - name, - isInclude ? true : false - )} - on:apply={(event) => - toggleDimensionValue(StateManagers, name, event.detail)} + on:remove={() => toggleDimensionNameSelection(name)} + on:apply={(event) => { + toggleDimensionValue(StateManagers, name, event.detail); + }} on:search={(event) => { setActiveDimension(name, event.detail); }} + on:click={() => { + setActiveDimension(name, ""); + }} + on:mount={() => { + setActiveDimension(name); + }} typeLabel="dimension" name={isInclude ? label : `Exclude ${label}`} excludeMode={isInclude ? false : true} colors={getColorForChip(isInclude)} label="View filter" {selectedValues} - {searchedValues} + {allValues} > Click to edit the the filters in this dimension @@ -176,35 +194,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/selectors/index.ts b/web-common/src/features/dashboards/selectors/index.ts index dd9f11f8bd5..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: "15", + limit: "100", offset: "0", sort: [], filter: { 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..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 @@ -38,6 +38,34 @@ export function toggleDimensionValueSelection( } } +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 filterIndex = filters.findIndex( + (filter) => filter.name === dimensionName + ); + + if (filterIndex === -1) { + filters.push({ + name: dimensionName, + in: [], + }); + } else { + filters.splice(filterIndex, 1); + } +} + export const dimensionFilterActions = { /** * Toggles whether the given dimension value is selected in the @@ -48,4 +76,5 @@ export const dimensionFilterActions = { * the include/exclude mode is a toggle for the entire dimension. */ toggleDimensionValueSelection, + toggleDimensionNameSelection, }; diff --git a/web-local/test/ui/dashboards.spec.ts b/web-local/test/ui/dashboards.spec.ts index 3d10daf997b..79cd8bee52c 100644 --- a/web-local/test/ui/dashboards.spec.ts +++ b/web-local/test/ui/dashboards.spec.ts @@ -245,6 +245,8 @@ test.describe("dashboard", () => { // Filter to Facebook via leaderboard await page.getByRole("button", { name: "Facebook 19.3k" }).click(); + await page.waitForSelector("text=Publisher Facebook"); + // Change filter to excluded await page.getByText("Publisher Facebook").click(); await page.getByRole("button", { name: "Exclude" }).click();