Improve UX of the "Recipients" element in the Scheduled Report form (#…

…3500) * Edit info tooltip * Create `dashed` button variant * Create new recipients element * Fix `Enter` submitting the whole form * Refactor into a new `InputArray` form element * Delete unused code * Prevent button's height from collapsing * Handle overflow * Prevent `IconButton` from submitting form * Fix spacing to allow for scrollbar and focus state * Improve the `input` identifier * Focus on the new input element
rilldata · Nov 16, 2023 · f606072 · f606072 · github-actions · Nov 16, 2023
1 parent 26c2b4f
commit f606072
Show file tree

Hide file tree

Showing 7 changed files with 179 additions and 82 deletions.
diff --git a/web-common/src/components/button/Button.svelte b/web-common/src/components/button/Button.svelte
@@ -1,7 +1,8 @@
 <script lang="ts">
   import { createEventDispatcher } from "svelte";
 
-  export let type: "primary" | "secondary" | "highlighted" | "text" = "primary";
+  export let type: "primary" | "secondary" | "highlighted" | "text" | "dashed" =
+    "primary";
   export let status: "info" | "error" = "info";
   export let disabled = false;
   export let compact = false;
@@ -26,6 +27,8 @@
       highlighted:
         "text-gray-500 border border-gray-200 hover:bg-gray-200 hover:text-gray-600 hover:border-gray-200 focus:ring-blue-300 shadow-lg rounded-sm h-8 ",
       text: "text-gray-900 hover:bg-gray-300 focus:ring-blue-300",
+      dashed:
+        "text-gray-800 border border-dashed rounded-sm border-gray-300 shadow-sm hover:bg-gray-100 hover:text-gray-700 hover:border-gray-300 focus:ring-blue-300 ",
     },
     error: {
       primary:
@@ -45,19 +48,23 @@
     customClasses = undefined,
   }) {
     return `
-  ${compact ? "px-2 py-0.5" : "px-3 py-0.5"} text-xs font-normal leading-snug
- flex flex-row gap-x-2 min-w-fit items-center transition-transform duration-100
+  ${compact ? "px-2" : "px-3"} py-0.5 text-xs font-normal leading-snug
+ flex flex-row gap-x-2 min-w-fit items-center justify-center transition-transform duration-100
   focus:outline-none focus:ring-2
   ${customClasses ? customClasses : levels[status][type]}
   ${disabledClasses}
+  ${
+    type === "highlighted"
+      ? "min-h-[32px]"
+      : compact
+      ? "h-auto"
+      : "min-h-[28px]"
+  }
   `;
   }
-
-  const height = type === "highlighted" ? "32px" : compact ? "auto" : "28px";
 </script>
 
 <button
-  style:height
   {disabled}
   class={buttonClasses({ type, compact, status })}
   on:click={handleClick}

diff --git a/web-common/src/components/button/IconButton.svelte b/web-common/src/components/button/IconButton.svelte
@@ -22,6 +22,7 @@
   alignment={tooltipAlignment}
 >
   <button
+    type="button"
     on:click
     aria-label={ariaLabel}
     class:cursor-auto={disabled}

diff --git a/web-common/src/components/forms/InputArray.svelte b/web-common/src/components/forms/InputArray.svelte
@@ -0,0 +1,87 @@
+<script lang="ts">
+  import { createEventDispatcher } from "svelte";
+  import { slide } from "svelte/transition";
+  import { Button, IconButton } from "../button";
+  import Add from "../icons/Add.svelte";
+  import InfoCircle from "../icons/InfoCircle.svelte";
+  import Trash from "../icons/Trash.svelte";
+  import Tooltip from "../tooltip/Tooltip.svelte";
+  import TooltipContent from "../tooltip/TooltipContent.svelte";
+
+  export let id = "";
+  export let label = "";
+  export let values: any[];
+  export let errors: any[];
+  // The accessorKey is necessary due to the way svelte-forms-lib works with arrays.
+  // See: https://svelte-forms-lib-sapper-docs.vercel.app/array
+  export let accessorKey: string;
+  export let placeholder = "";
+  export let hint = "";
+  export let addItemLabel = "Add item";
+
+  const dispatch = createEventDispatcher();
+
+  function handleKeyDown(event: KeyboardEvent) {
+    if (event.key === "Enter") {
+      event.preventDefault();
+    }
+  }
+</script>
+
+<div class="flex flex-col gap-y-2.5">
+  <div class="flex items-center gap-x-1">
+    <label for={id} class="text-gray-800 text-sm font-medium">{label}</label>
+    {#if hint}
+      <Tooltip location="right" alignment="middle" distance={8}>
+        <div class="text-gray-500" style="transform:translateY(-.5px)">
+          <InfoCircle size="13px" />
+        </div>
+        <TooltipContent maxWidth="400px" slot="tooltip-content">
+          {hint}
+        </TooltipContent>
+      </Tooltip>
+    {/if}
+  </div>
+  <div
+    class="flex flex-col gap-y-4 max-h-[200px] pl-1 pr-4 py-1 overflow-y-auto"
+  >
+    {#each values as value, i}
+      <div class="flex flex-col gap-y-2">
+        <div class="flex gap-x-2 items-center">
+          <input
+            bind:value={values[i][accessorKey]}
+            id="{id}.{i}.{accessorKey}"
+            autocomplete="off"
+            {placeholder}
+            class="bg-white rounded-sm border border-gray-300 px-3 py-[5px] h-8 cursor-pointer focus:outline-blue-500 w-full text-xs {errors[
+              i
+            ]?.accessorKey && 'border-red-500'}"
+            on:keydown={handleKeyDown}
+          />
+          <IconButton
+            on:click={() =>
+              dispatch("remove-item", {
+                index: i,
+              })}
+          >
+            <Trash size="16px" className="text-gray-500 cursor-pointer" />
+          </IconButton>
+        </div>
+        {#if errors[i]?.[accessorKey]}
+          <div
+            in:slide|local={{ duration: 200 }}
+            class="text-red-500 text-sm py-px"
+          >
+            {errors[i][accessorKey]}
+          </div>
+        {/if}
+      </div>
+    {/each}
+    <Button on:click={() => dispatch("add-item")} type="dashed">
+      <div class="flex gap-x-2">
+        <Add className="text-gray-700" />
+        {addItemLabel}
+      </div>
+    </Button>
+  </div>
+</div>
diff --git a/web-common/src/components/forms/InputV2.svelte b/web-common/src/components/forms/InputV2.svelte
@@ -21,6 +21,12 @@
       inputElement.focus();
     });
   }
+
+  function handleKeyDown(event: KeyboardEvent) {
+    if (event.key === "Enter") {
+      event.preventDefault();
+    }
+  }
 </script>
 
 <div class="flex flex-col gap-y-2">
@@ -45,6 +51,7 @@
     bind:value
     on:input
     on:change
+    on:keydown={handleKeyDown}
     {id}
     name={id}
     type="text"

diff --git a/web-common/src/components/icons/Trash.svelte b/web-common/src/components/icons/Trash.svelte
@@ -0,0 +1,20 @@
+<script lang="ts">
+  export let size = "1em";
+  export let color = "currentColor";
+  export let className = "";
+</script>
+
+<svg
+  height={size}
+  viewBox="0 0 24 24"
+  fill="none"
+  xmlns="http://www.w3.org/2000/svg"
+  class={className}
+>
+  <path
+    fill-rule="evenodd"
+    clip-rule="evenodd"
+    d="M9.33211 3.33211C9.09745 3.56676 9 3.82523 9 4V5H15V4C15 3.82523 14.9025 3.56676 14.6679 3.33211C14.4332 3.09745 14.1748 3 14 3H10C9.82523 3 9.56676 3.09745 9.33211 3.33211ZM17 5V4C17 3.17477 16.5975 2.43324 16.0821 1.91789C15.5668 1.40255 14.8252 1 14 1H10C9.17477 1 8.43324 1.40255 7.91789 1.91789C7.40255 2.43324 7 3.17477 7 4V5H3C2.44772 5 2 5.44772 2 6C2 6.55228 2.44772 7 3 7H4V20C4 20.8252 4.40255 21.5668 4.91789 22.0821C5.43324 22.5975 6.17477 23 7 23H17C17.8252 23 18.5668 22.5975 19.0821 22.0821C19.5975 21.5668 20 20.8252 20 20V7H21C21.5523 7 22 6.55228 22 6C22 5.44772 21.5523 5 21 5H17ZM6 7V20C6 20.1748 6.09745 20.4332 6.33211 20.6679C6.56676 20.9025 6.82523 21 7 21H17C17.1748 21 17.4332 20.9025 17.6679 20.6679C17.9025 20.4332 18 20.1748 18 20V7H6ZM10 10C10.5523 10 11 10.4477 11 11V17C11 17.5523 10.5523 18 10 18C9.44772 18 9 17.5523 9 17V11C9 10.4477 9.44772 10 10 10ZM14 10C14.5523 10 15 10.4477 15 11V17C15 17.5523 14.5523 18 14 18C13.4477 18 13 17.5523 13 17V11C13 10.4477 13.4477 10 14 10Z"
+    fill={color}
+  />
+</svg>
diff --git a/web-common/src/features/dashboards/scheduled-reports/CreateScheduledReportModal.svelte b/web-common/src/features/dashboards/scheduled-reports/CreateScheduledReportModal.svelte
@@ -1,6 +1,9 @@
 <script lang="ts">
   import { page } from "$app/stores";
-  import { createAdminServiceCreateReport } from "@rilldata/web-admin/client";
+  import {
+    createAdminServiceCreateReport,
+    createAdminServiceGetCurrentUser,
+  } from "@rilldata/web-admin/client";
   import Dialog from "@rilldata/web-common/components/dialog-v2/Dialog.svelte";
   import TimePicker from "@rilldata/web-common/components/forms/TimePicker.svelte";
   import { notifications } from "@rilldata/web-common/components/notifications";
@@ -14,14 +17,14 @@
   import { createForm } from "svelte-forms-lib";
   import * as yup from "yup";
   import { Button } from "../../../components/button";
+  import InputArray from "../../../components/forms/InputArray.svelte";
   import InputV2 from "../../../components/forms/InputV2.svelte";
   import Select from "../../../components/forms/Select.svelte";
   import {
     getAbbreviationForIANA,
     getLocalIANA,
     getUTCIANA,
   } from "../../../lib/time/timezone";
-  import RecipientsList from "./RecipientsList.svelte";
   import {
     convertToCron,
     getNextQuarterHour,
@@ -37,10 +40,11 @@
   $: organization = $page.params.organization;
   $: project = $page.params.project;
 
-  const createReport = createAdminServiceCreateReport();
   const dispatch = createEventDispatcher();
   const queryClient = useQueryClient();
+  const createReport = createAdminServiceCreateReport();
 
+  const user = createAdminServiceGetCurrentUser();
   const userLocalIANA = getLocalIANA();
   const UTCIana = getUTCIANA();
 
@@ -56,11 +60,18 @@
       timeZone: userLocalIANA,
       exportFormat: V1ExportFormat.EXPORT_FORMAT_CSV,
       exportLimit: "",
-      recipients: [] as string[],
+      recipients: [
+        { email: $user.data?.user?.email ? $user.data.user.email : "" },
+        { email: "" },
+      ],
     },
     validationSchema: yup.object({
       title: yup.string().required("Required"),
-      recipients: yup.array().min(1, "Required"),
+      recipients: yup.array().of(
+        yup.object().shape({
+          email: yup.string().email("Invalid email"),
+        })
+      ),
     }),
     onSubmit: async (values) => {
       const refreshCron = convertToCron(
@@ -82,7 +93,7 @@
               exportLimit: values.exportLimit || undefined,
               exportFormat: values.exportFormat,
               openProjectSubpath: `/${queryArgs.metricsViewName}?state=${dashState}`,
-              recipients: values.recipients,
+              recipients: values.recipients.map((r) => r.email).filter(Boolean),
             },
           },
         });
@@ -100,25 +111,9 @@
     },
   });
 
-  // This form-within-a-form is used to add recipients to the parent form
-  const {
-    form: newRecipientForm,
-    errors: newRecipientErrors,
-    handleSubmit: newRecipientHandleSubmit,
-  } = createForm({
-    initialValues: {
-      newRecipient: "",
-    },
-    validationSchema: yup.object({
-      newRecipient: yup.string().email("Invalid email"),
-    }),
-    onSubmit: (values) => {
-      if (values.newRecipient) {
-        $form["recipients"] = $form["recipients"].concat(values.newRecipient);
-      }
-      $newRecipientForm.newRecipient = "";
-    },
-  });
+  // There's a bug in how `svelte-forms-lib` types the `$errors` store for arrays.
+  // See: https://github.com/tjinauyeung/svelte-forms-lib/issues/154#issuecomment-1087331250
+  $: recipientErrors = $errors.recipients as unknown as { email: string }[];
 </script>
 
 <Dialog {open}>
@@ -206,27 +201,37 @@
       optional
       placeholder="1000"
     />
-    <div class="flex flex-col gap-y-2">
-      <form
-        autocomplete="off"
-        id="add-recipient-form"
-        on:submit|preventDefault={newRecipientHandleSubmit}
-      >
-        <InputV2
-          bind:value={$newRecipientForm["newRecipient"]}
-          error={$newRecipientErrors["newRecipient"]}
-          hint="Recipients may receive different views based on the project's security policies.
-           Recipients without access to the project will not be able to view the report."
-          id="newRecipient"
-          label="Recipients"
-          placeholder="Add an email address"
-        />
-      </form>
-      <RecipientsList bind:recipients={$form["recipients"]} />
-    </div>
+    <InputArray
+      id="recipients"
+      label="Recipients"
+      bind:values={$form["recipients"]}
+      bind:errors={recipientErrors}
+      accessorKey="email"
+      hint="Recipients will receive different views based on their security policy.
+        Recipients without project access can't view the report."
+      placeholder="Enter an email address"
+      addItemLabel="Add email"
+      on:add-item={() => {
+        $form["recipients"] = $form["recipients"].concat({ email: "" });
+        recipientErrors = recipientErrors.concat({ email: "" });
+
+        // Focus on the new input element
+        setTimeout(() => {
+          const input = document.getElementById(
+            `recipients.${$form["recipients"].length - 1}.email`
+          );
+          input?.focus();
+        }, 0);
+      }}
+      on:remove-item={(event) => {
+        const index = event.detail.index;
+        $form["recipients"] = $form["recipients"].filter((r, i) => i !== index);
+        recipientErrors = recipientErrors.filter((r, i) => i !== index);
+      }}
+    />
   </form>
   <svelte:fragment slot="footer">
-    <div class="flex items-center gap-x-2 mt-2">
+    <div class="flex items-center gap-x-2 mt-5">
       {#if $createReport.isError}
         <div class="text-red-500">{$createReport.error.message}</div>
       {/if}
@@ -235,7 +240,8 @@
         Cancel
       </Button>
       <Button
-        disabled={$isSubmitting || $form["recipients"].length === 0}
+        disabled={$isSubmitting ||
+          $form["recipients"].filter((r) => r.email).length === 0}
         form="create-scheduled-report-form"
         submitForm
         type="primary"

diff --git a/web-common/src/features/dashboards/scheduled-reports/RecipientsList.svelte b/web-common/src/features/dashboards/scheduled-reports/RecipientsList.svelte