refactor: clean up UI (#3288)

Signed-off-by: Roman Dmytrenko <[email protected]>
flipt-io · Jul 21, 2024 · 7cabab9 · 7cabab9
1 parent 2a0e3f4
commit 7cabab9
Show file tree

Hide file tree

Showing 7 changed files with 96 additions and 125 deletions.
diff --git a/ui/src/app/flags/flagsApi.ts b/ui/src/app/flags/flagsApi.ts
@@ -73,7 +73,7 @@ export const flagsApi = createApi({
           defaultVariantId: values.defaultVariant?.id || null,
           ...values
         };
-
+        delete update.defaultVariant;
         return {
           url: `/namespaces/${namespaceKey}/flags/${flagKey}`,
           method: 'PUT',

diff --git a/ui/src/app/flags/rules/Rules.tsx b/ui/src/app/flags/rules/Rules.tsx
@@ -28,7 +28,7 @@ import { selectCurrentNamespace } from '~/app/namespaces/namespacesSlice';
 import { useListSegmentsQuery } from '~/app/segments/segmentsApi';
 import EmptyState from '~/components/EmptyState';
 import Button from '~/components/forms/buttons/Button';
-import Combobox from '~/components/forms/Combobox';
+
 import Loading from '~/components/Loading';
 import Modal from '~/components/Modal';
 import DeletePanel from '~/components/panels/DeletePanel';
@@ -44,10 +44,11 @@ import { IFlag } from '~/types/Flag';
 import { IRule } from '~/types/Rule';
 import { ISegment, SegmentOperatorType } from '~/types/Segment';
 import { FilterableVariant, IVariant } from '~/types/Variant';
-import { truncateKey } from '~/utils/helpers';
 import { useUpdateFlagMutation } from '~/app/flags/flagsApi';
 import { INamespace } from '~/types/Namespace';
 import TextButton from '~/components/forms/buttons/TextButton';
+import SingleDistributionFormInput from '~/components/rules/forms/SingleDistributionForm';
+import { toFilterableVariant } from '~/utils/helpers';
 
 type RulesProps = {
   flag: IFlag;
@@ -61,19 +62,9 @@ export function DefaultVariant(props: RulesProps) {
 
   const namespace = useSelector(selectCurrentNamespace) as INamespace;
 
-  const readOnly = useSelector(selectReadonly);
-
   const [selectedVariant, setSelectedVariant] =
     useState<FilterableVariant | null>(() => {
-      const variant = flag.defaultVariant;
-      if (variant) {
-        return {
-          ...variant,
-          filterValue: truncateKey(variant.key),
-          displayValue: variant.key
-        };
-      }
-      return null;
+      return toFilterableVariant(flag.defaultVariant);
     });
 
   const [updateFlag] = useUpdateFlagMutation();
@@ -103,7 +94,6 @@ export function DefaultVariant(props: RulesProps) {
       values: {
         ...flag,
         defaultVariant: {
-          ...selectedVariant,
           id: selectedVariant?.id ?? ''
         } as IVariant
       }
@@ -131,86 +121,78 @@ export function DefaultVariant(props: RulesProps) {
     >
       {(formik) => {
         return (
-          <Form className="bg-white flex w-full flex-col overflow-y-scroll">
-            <div className="bg-white border-violet-300 w-full items-center space-y-2 rounded-md border shadow-md shadow-violet-100 hover:shadow-violet-200 sm:flex sm:flex-col lg:px-6 lg:py-2">
-              <div className="w-full flex-1">
-                <div className="bg-white border-gray-200 w-full border-b p-2">
-                  <div className="flex w-full flex-wrap items-center justify-between sm:flex-nowrap">
-                    <StarIcon className="text-gray-400 hidden h-4 w-4 justify-start hover:text-violet-300 sm:flex" />
-                    <h3 className="text-gray-700 text-sm font-normal leading-6">
-                      Default Variant
-                    </h3>
-                    <span className="hidden h-4 w-4 justify-end sm:flex" />
-                  </div>
-                </div>
-                <div className="flex w-full flex-1 items-center p-2 text-xs lg:p-0">
-                  <div className="flex grow flex-col items-center justify-center sm:ml-2">
-                    <div className="flex flex-col pb-4 pt-2">
-                      <p className="text-gray-600 text-center text-sm font-light">
-                        This is the default value that will be returned if no
-                        other rules match.
-                      </p>
-                    </div>
-                    <div className="flex items-baseline space-x-4 py-6 sm:space-y-0 sm:py-0">
+          <div className="bg-white border-violet-300 w-full items-center space-y-2 rounded-md border shadow-md shadow-violet-100 hover:shadow-violet-200 sm:flex sm:flex-col lg:px-4 lg:py-2">
+            <div className="bg-white border-gray-200 w-full border-b p-2">
+              <div className="flex w-full flex-wrap items-center justify-between sm:flex-nowrap">
+                <StarIcon className="text-gray-400 hidden h-4 w-4 justify-start hover:text-violet-300 sm:flex" />
+                <h3 className="text-gray-700 text-sm font-normal leading-6">
+                  Default Rule
+                </h3>
+                <span className="hidden h-4 w-4 justify-end sm:flex" />
+              </div>
+            </div>
+
+            <div className="flex grow flex-col items-center justify-center sm:ml-2">
+              <p className="text-gray-600 text-center text-sm font-light">
+                This is the default value that will be returned if no other
+                rules match.
+              </p>
+            </div>
+            <div className="flex w-full flex-1 items-center p-2 text-xs lg:p-0">
+              <div className="flex grow flex-col items-center justify-center sm:ml-2 md:flex-row md:justify-between">
+                <Form className="bg-white flex w-full flex-col overflow-y-scroll">
+                  <div className="w-full flex-1">
+                    <div className="space-y-6 py-6 sm:space-y-0 sm:py-0">
                       {flag.variants && flag.variants.length > 0 && (
-                        <div className="space-y-1 px-4 sm:grid sm:grid-cols-3 sm:gap-4 sm:space-y-0 sm:px-6 sm:py-2">
-                          <div>
-                            <label
-                              htmlFor="defaultVariant"
-                              className="text-gray-900 block text-sm font-medium sm:mt-px sm:pt-2"
-                            >
-                              Variant
-                            </label>
-                          </div>
-                          <div className="sm:col-span-2">
-                            <Combobox<FilterableVariant>
-                              id="defaultVariant"
-                              name="defaultVariant"
-                              values={flag.variants.map((v) => ({
-                                ...v,
-                                filterValue: truncateKey(v.key),
-                                displayValue: v.key
-                              }))}
-                              placeholder="Select a variant"
-                              selected={selectedVariant}
-                              setSelected={setSelectedVariant}
-                              disabled={readOnly}
-                            />
-                          </div>
-                        </div>
+                        <SingleDistributionFormInput
+                          id="variant-default"
+                          variants={flag.variants}
+                          selectedVariant={selectedVariant}
+                          setSelectedVariant={setSelectedVariant}
+                        />
                       )}
                     </div>
                   </div>
-                </div>
-                <div className="flex-shrink-0 py-1">
-                  <div className="flex justify-end space-x-3">
-                    <TextButton
-                      disabled={formik.isSubmitting}
-                      onClick={() => handleRemove()}
-                    >
-                      Remove
-                    </TextButton>
-                    <TextButton
-                      className="min-w-[60px]"
-                      disabled={formik.isSubmitting || !formik.dirty}
-                      onClick={() => formik.resetForm()}
-                    >
-                      Reset
-                    </TextButton>
-                    <TextButton
-                      type="submit"
-                      className="min-w-[60px]"
-                      disabled={
-                        !formik.isValid || formik.isSubmitting || !formik.dirty
-                      }
-                    >
-                      {formik.isSubmitting ? <Loading isPrimary /> : 'Update'}
-                    </TextButton>
+                  <div className="flex-shrink-0 py-1">
+                    <div className="flex justify-end space-x-3">
+                      <TextButton
+                        className="min-w-[80px]"
+                        disabled={formik.isSubmitting || !flag.defaultVariant}
+                        onClick={() => handleRemove()}
+                      >
+                        Remove
+                      </TextButton>
+                      <TextButton
+                        disabled={
+                          formik.isSubmitting ||
+                          flag.defaultVariant?.id == selectedVariant?.id
+                        }
+                        onClick={() => {
+                          formik.resetForm();
+                          setSelectedVariant(
+                            toFilterableVariant(flag.defaultVariant)
+                          );
+                        }}
+                      >
+                        Reset
+                      </TextButton>
+                      <TextButton
+                        type="submit"
+                        className="min-w-[80px]"
+                        disabled={
+                          !formik.isValid ||
+                          formik.isSubmitting ||
+                          flag.defaultVariant?.id == selectedVariant?.id
+                        }
+                      >
+                        {formik.isSubmitting ? <Loading isPrimary /> : 'Update'}
+                      </TextButton>
+                    </div>
                   </div>
-                </div>
+                </Form>
               </div>
             </div>
-          </Form>
+          </div>
         );
       }}
     </Formik>

diff --git a/ui/src/components/forms/Combobox.tsx b/ui/src/components/forms/Combobox.tsx
@@ -68,6 +68,7 @@ export default function Combobox<T extends IFilterable>(
               }}
               displayValue={(v: T) => v?.key}
               placeholder={placeholder}
+              id={`${id}-select-input`}
             />
             <C.Button
               className="absolute -inset-y-0 right-0 items-center rounded-r-md px-2 focus:outline-none"

diff --git a/ui/src/components/rules/forms/QuickEditRuleForm.tsx b/ui/src/components/rules/forms/QuickEditRuleForm.tsx
@@ -8,7 +8,6 @@ import {
 import { selectReadonly } from '~/app/meta/metaSlice';
 import { selectCurrentNamespace } from '~/app/namespaces/namespacesSlice';
 import TextButton from '~/components/forms/buttons/TextButton';
-import Combobox from '~/components/forms/Combobox';
 import SegmentsPicker from '~/components/forms/SegmentsPicker';
 import Loading from '~/components/Loading';
 import { useError } from '~/data/hooks/error';
@@ -24,8 +23,9 @@ import {
   SegmentOperatorType
 } from '~/types/Segment';
 import { FilterableVariant } from '~/types/Variant';
-import { cls, truncateKey } from '~/utils/helpers';
+import { cls, toFilterableVariant } from '~/utils/helpers';
 import { distTypes } from './RuleForm';
+import SingleDistributionFormInput from '~/components/rules/forms/SingleDistributionForm';
 
 type QuickEditRuleFormProps = {
   flag: IFlag;
@@ -72,15 +72,7 @@ export default function QuickEditRuleForm(props: QuickEditRuleFormProps) {
         return null;
       }
 
-      let selected = rule.rollouts[0].variant;
-      if (selected) {
-        return {
-          ...selected,
-          displayValue: selected.name,
-          filterValue: selected.id
-        };
-      }
-      return null;
+      return toFilterableVariant(rule.rollouts[0].variant);
     });
 
   const readOnly = useSelector(selectReadonly);
@@ -329,30 +321,12 @@ export default function QuickEditRuleForm(props: QuickEditRuleFormProps) {
                 {ruleType === DistributionType.Single &&
                   flag.variants &&
                   flag.variants.length > 0 && (
-                    <div className="space-y-1 px-4 sm:grid sm:grid-cols-3 sm:gap-4 sm:space-y-0 sm:px-6 sm:py-2">
-                      <div>
-                        <label
-                          htmlFor="variantKey"
-                          className="text-gray-900 block text-sm font-medium sm:mt-px sm:pt-2"
-                        >
-                          Variant
-                        </label>
-                      </div>
-                      <div className="sm:col-span-2">
-                        <Combobox<FilterableVariant>
-                          id="variantKey"
-                          name="variantKey"
-                          values={flag.variants?.map((v) => ({
-                            ...v,
-                            filterValue: truncateKey(v.key),
-                            displayValue: v.key
-                          }))}
-                          selected={selectedVariant}
-                          setSelected={setSelectedVariant}
-                          disabled={readOnly}
-                        />
-                      </div>
-                    </div>
+                    <SingleDistributionFormInput
+                      id={`quick-variant-${rule.rank}`}
+                      variants={flag.variants}
+                      selectedVariant={selectedVariant}
+                      setSelectedVariant={setSelectedVariant}
+                    />
                   )}
 
                 {ruleType === DistributionType.Multi && (

diff --git a/ui/src/components/rules/forms/SingleDistributionForm.tsx b/ui/src/components/rules/forms/SingleDistributionForm.tsx
@@ -5,12 +5,14 @@ type SingleDistributionFormInputProps = {
   variants: IVariant[];
   selectedVariant: FilterableVariant | null;
   setSelectedVariant: (variant: FilterableVariant | null) => void;
+  id?: string;
 };
 
 export default function SingleDistributionFormInput(
   props: SingleDistributionFormInputProps
 ) {
   const { variants, selectedVariant, setSelectedVariant } = props;
+  const id = props.id || 'variant';
   return (
     <div className="space-y-1 px-4 sm:grid sm:grid-cols-3 sm:gap-4 sm:space-y-0 sm:px-6 sm:py-5">
       <div>
@@ -23,7 +25,7 @@ export default function SingleDistributionFormInput(
       </div>
       <div className="sm:col-span-2">
         <Combobox<FilterableVariant>
-          id="variant"
+          id={id}
           name="variant"
           placeholder="Select or search for a variant"
           values={

diff --git a/ui/src/utils/helpers.ts b/ui/src/utils/helpers.ts
@@ -3,6 +3,7 @@ import { twMerge } from 'tailwind-merge';
 import { defaultHeaders } from '~/data/api';
 import { ICommand } from '~/types/Cli';
 import { ICurlOptions } from '~/types/Curl';
+import { IVariant } from '~/types/Variant';
 
 export function cls(...args: ClassValue[]) {
   return twMerge(clsx(args));
@@ -121,3 +122,14 @@ export function generateCurlCommand(curlOptions: ICurlOptions) {
 export function generateCliCommand(command: ICommand): string {
   return `flipt ${command.commandName} ${command.arguments?.join(' ')} ${command.options?.map(({ key, value }) => `${key} ${value}`).join(' ')}`;
 }
+
+export function toFilterableVariant(selected: IVariant | undefined) {
+  if (selected) {
+    return {
+      ...selected,
+      displayValue: selected.name,
+      filterValue: selected.id
+    };
+  }
+  return null;
+}
diff --git a/ui/tests/rules.spec.ts b/ui/tests/rules.spec.ts
@@ -80,7 +80,7 @@ test.describe('Rules', () => {
     });
 
     await test.step('can update single-variant rule', async () => {
-      await page.locator('#variantKey-select-button').click();
+      await page.locator('#quick-variant-2-select-button').click();
       await page
         .locator('li')
         .filter({ hasText: 'Single Variant' })