support for Add Filter button (#3671)

* rework of footer to match new design and improve readability * adding filter button functionality, plus design tweaks to relevant menus * new icon * chip should not be removed when deselecting the only selected value * removing limit from filter list query * changed searchedValues to allValues, simplified display logic * updated tests to meet updated requirements * prettier unused vars build fix * remove keep-alive event * update let directive for named slot, add timeout to dashboard.spec * remove unused function * change to active logic when mounting chip component * resolve name collision * update page wait from timeout to selector * added specific method for adding a dimension name without a value * only unselected dimension names are shown in the add filter dropdown * add back limit * toggleDimensionNameSelection is now actually a toggling function * dispatch remove event rather than handle remove directly, plus bind to active property * use state manager action and prop cleanup * add back limit
rilldata · Dec 18, 2023 · ff09c7a · ff09c7a
1 parent fef0f7d
commit ff09c7a
Show file tree

Hide file tree

Showing 13 changed files with 309 additions and 153 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
@@ -1,5 +1,18 @@
-<div
-  class="flex flex-row mt-auto items-center justify-between gap-x-2 px-2 py-1 sticky bottom-0 border-t bg-gray-50 dark:bg-gray-600 border-gray-200 dark:border-gray-500"
->
+<footer>
   <slot />
-</div>
+</footer>
+
+<style lang="postcss">
+  footer {
+    @apply flex flex-row mt-auto items-center justify-end;
+    @apply bg-slate-100;
+    @apply border-t border-slate-300;
+    @apply bottom-0 sticky;
+    @apply gap-x-2 p-2 px-3.5;
+  }
+
+  footer:is(.dark) {
+    @apply bg-gray-800;
+    @apply border-gray-700;
+  }
+</style>
diff --git 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.
 -->
-<script lang="ts">
-  import { createEventDispatcher } from "svelte";
-  import { Writable, writable } from "svelte/store";
+
+<script context="module" lang="ts">
+  import { createEventDispatcher, onMount } from "svelte";
   import { fly } from "svelte/transition";
   import WithTogglableFloatingElement from "../../floating-element/WithTogglableFloatingElement.svelte";
   import Tooltip from "../../tooltip/Tooltip.svelte";
@@ -23,26 +23,39 @@ 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";
+</script>
 
+<script lang="ts">
   export let name: string;
   export let selectedValues: string[];
-  export let searchedValues: string[] | null;
+  export let allValues: string[] | null;
 
   /** an optional type label that will appear in the tooltip */
   export let typeLabel: string;
-  export let excludeMode;
+  export let excludeMode: boolean;
   export let colors: ChipColors = defaultChipColors;
   export let label: string | undefined = undefined;
 
+  let active = !selectedValues.length;
+
   const dispatch = createEventDispatcher();
 
-  const excludeStore: Writable<boolean> = writable(excludeMode);
-  $: excludeStore.set(excludeMode);
+  onMount(() => {
+    dispatch("mount");
+  });
+
+  function handleDismiss() {
+    if (!selectedValues.length) {
+      dispatch("remove");
+    } else {
+      active = false;
+    }
+  }
 </script>
 
 <WithTogglableFloatingElement
+  bind:active
   let:toggleFloatingElement
-  let:active
   distance={8}
   alignment="start"
 >
@@ -55,7 +68,10 @@ are details left to the consumer of the component; this component should remain
   >
     <Chip
       removable
-      on:click={toggleFloatingElement}
+      on:click={() => {
+        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
     </div>
   </Tooltip>
   <RemovableListMenu
-    {excludeStore}
     slot="floating-element"
-    let:toggleFloatingElement
-    on:escape={toggleFloatingElement}
-    on:click-outside={toggleFloatingElement}
+    {excludeMode}
+    {allValues}
+    {selectedValues}
+    on:escape={handleDismiss}
+    on:click-outside={handleDismiss}
     on:apply
     on:search
     on:toggle
-    {selectedValues}
-    {searchedValues}
   />
 </WithTogglableFloatingElement>
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
@@ -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<HTMLInputElement>("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<HTMLInputElement>("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
@@ -1,17 +1,16 @@
 <script lang="ts">
   import { createEventDispatcher } from "svelte";
-  import type { Writable } from "svelte/store";
-  import { Switch } from "../../button";
   import Cancel from "../../icons/Cancel.svelte";
   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";
+  import Button from "../../button/Button.svelte";
 
-  export let excludeStore: Writable<boolean>;
+  export let excludeMode: boolean;
   export let selectedValues: string[];
-  export let searchedValues: string[] | null = [];
+  export let allValues: string[] | null = [];
 
   let searchText = "";
 
@@ -21,33 +20,20 @@
     dispatch("search", searchText);
   }
 
-  function onToggleHandler() {
-    dispatch("toggle");
+  function toggleValue(value: string) {
+    dispatch("apply", value);
   }
 
-  /** On instantiation, only take the exact current selectedValues, so that
-   * when the user unchecks a menu item, it still persists in the FilterMenu
-   * until the user closes.
-   */
-  let candidateValues = [...selectedValues];
-  let valuesToDisplay = [...candidateValues];
-
-  // If searchedValues === null, search has not finished yet. So continue rendering the previous list
-  $: if (searchText && searchedValues !== null) {
-    valuesToDisplay = [...searchedValues];
-  } else if (!searchText) valuesToDisplay = [...candidateValues];
-
-  $: numSelectedNotInSearch = selectedValues.filter(
-    (v) => !valuesToDisplay.includes(v)
-  ).length;
-
-  function toggleValue(value) {
-    dispatch("apply", value);
+  function toggleSelectAll() {
+    allValues?.forEach((value) => {
+      if (!allSelected && selectedValues.includes(value)) return;
 
-    if (!candidateValues.includes(value)) {
-      candidateValues = [...candidateValues, value];
-    }
+      toggleValue(value);
+    });
   }
+
+  $: allSelected =
+    selectedValues?.length && allValues?.length === selectedValues.length;
 </script>
 
 <Menu
@@ -62,8 +48,7 @@
   on:click-outside
 >
   <!-- the min-height is set to have about 3 entries in it -->
-
-  <div class="px-1 pb-1">
+  <div class="px-3 py-2">
     <Search
       bind:value={searchText}
       on:input={onSearch}
@@ -74,8 +59,8 @@
 
   <!-- apply a wrapped flex element to ensure proper bottom spacing between body and footer -->
   <div class="flex flex-col flex-1 overflow-auto w-full pb-1">
-    {#if valuesToDisplay.length}
-      {#each valuesToDisplay as value}
+    {#if allValues?.length}
+      {#each allValues.sort() as value}
         <MenuItem
           icon
           animateSelect={false}
@@ -85,17 +70,17 @@
           }}
         >
           <svelte:fragment slot="icon">
-            {#if selectedValues.includes(value) && !$excludeStore}
+            {#if selectedValues.includes(value) && !excludeMode}
               <Check size="20px" color="#15141A" />
-            {:else if selectedValues.includes(value) && $excludeStore}
+            {:else if selectedValues.includes(value) && excludeMode}
               <Cancel size="20px" color="#15141A" />
             {:else}
               <Spacer size="20px" />
             {/if}
           </svelte:fragment>
           <span
             class:ui-copy-disabled={selectedValues.includes(value) &&
-              $excludeStore}
+              excludeMode}
           >
             {#if value?.length > 240}
               {value.slice(0, 240)}...
@@ -110,17 +95,20 @@
     {/if}
   </div>
   <Footer>
-    <span class="ui-copy">
-      <Switch on:click={() => onToggleHandler()} checked={$excludeStore}>
+    <Button type="text" on:click={toggleSelectAll}>
+      {#if allSelected}
+        Deselect all
+      {:else}
+        Select all
+      {/if}
+    </Button>
+
+    <Button type="secondary" on:click={() => dispatch("toggle")}>
+      {#if excludeMode}
+        Include
+      {:else}
         Exclude
-      </Switch>
-    </span>
-    {#if numSelectedNotInSearch}
-      <div class="ui-label">
-        {numSelectedNotInSearch} other value{numSelectedNotInSearch > 1
-          ? "s"
-          : ""} selected
-      </div>
-    {/if}
+      {/if}
+    </Button>
   </Footer>
 </Menu>
diff --git a/web-common/src/components/icons/ChevronRight.svelte b/web-common/src/components/icons/ChevronRight.svelte
@@ -0,0 +1,19 @@
+<script>
+  export let size = "1em";
+  export let color = "currentColor";
+</script>
+
+<svg
+  width={size}
+  viewBox="0 0 24 24"
+  fill="none"
+  xmlns="http://www.w3.org/2000/svg"
+>
+  <path
+    d="M9 18L15 12L9 6"
+    stroke={color}
+    stroke-width="2"
+    stroke-linecap="round"
+    stroke-linejoin="round"
+  />
+</svg>
diff --git a/web-common/src/components/search/Search.svelte b/web-common/src/components/search/Search.svelte
@@ -38,13 +38,18 @@
     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
     on:keydown={handleKeyDown}
     aria-label={label}
   />
 </form>
+
+<style lang="postcss">
+  .focus:focus {
+    @apply border-blue-400;
+  }
+</style>