Add linter rule and make changes to adapt to the new linter rule

DaoDaoNoCode · Oct 28, 2024 · 47349ea · 47349ea
1 parent 27466a2
commit 47349ea
Show file tree

Hide file tree

Showing 9 changed files with 102 additions and 252 deletions.
diff --git a/frontend/.eslintrc b/frontend/.eslintrc
@@ -138,6 +138,11 @@
           {
             "group": ["~/__mocks__/third_party/mlmd", "~/__mocks__/third_party/mlmd/*"],
             "message": "Importing from '~/__mocks__/third_party/mlmd/' is restricted to '~/__mocks__/mlmd/'."
+          },
+          {
+            "group": ["@patternfly/react-core"],
+            "importNames": ["Select"],
+            "message": "Import 'SimpleSelect', 'MultiSelection' or 'TypeaheadSelect' from '~/components' instead."
           }
         ]
       }

diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/servingRuntimeList.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/servingRuntimeList.cy.ts
@@ -502,7 +502,7 @@ describe('Serving Runtime List', () => {
       inferenceServiceModalEdit
         .findExistingConnectionSelect()
         .should('have.text', 'Test Secret')
-        .should('be.enabled');
+        .should('be.disabled');
     });
 
     it('ModelMesh ServingRuntime list', () => {

diff --git a/frontend/src/components/MultiSelection.tsx b/frontend/src/components/MultiSelection.tsx
@@ -1,5 +1,9 @@
 import * as React from 'react';
 import {
+  /**
+   * The Select component is used to build another generic component here
+   */
+  // eslint-disable-next-line no-restricted-imports
   Select,
   SelectOption,
   SelectList,

diff --git a/frontend/src/components/SimpleSelect.tsx b/frontend/src/components/SimpleSelect.tsx
@@ -2,6 +2,7 @@ import * as React from 'react';
 import {
   Truncate,
   MenuToggle,
+  // eslint-disable-next-line no-restricted-imports
   Select,
   SelectList,
   SelectOption,
@@ -23,6 +24,7 @@ export type SimpleSelectOption = {
   dropdownLabel?: React.ReactNode;
   isPlaceholder?: boolean;
   isDisabled?: boolean;
+  isFavorited?: boolean;
   dataTestId?: string;
 };
 
@@ -91,6 +93,7 @@ const SimpleSelect: React.FC<SimpleSelectProps> = ({
     if (totalOptionsKey) {
       onChange(totalOptionsKey, false);
     }
+    // We don't want the callback function to be a dependency
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [totalOptionsKey]);
 
@@ -137,6 +140,7 @@ const SimpleSelect: React.FC<SimpleSelectProps> = ({
                     label,
                     dropdownLabel,
                     description,
+                    isFavorited,
                     isDisabled: optionDisabled,
                     dataTestId: optionDataTestId,
                   }) => (
@@ -145,6 +149,7 @@ const SimpleSelect: React.FC<SimpleSelectProps> = ({
                       value={key}
                       description={<TruncatedText maxLines={2} content={description} />}
                       isDisabled={optionDisabled}
+                      isFavorited={isFavorited}
                       data-testid={optionDataTestId || key}
                     >
                       {dropdownLabel || label}
@@ -164,13 +169,15 @@ const SimpleSelect: React.FC<SimpleSelectProps> = ({
                 label,
                 dropdownLabel,
                 description,
+                isFavorited,
                 isDisabled: optionDisabled,
                 dataTestId: optionDataTestId,
               }) => (
                 <SelectOption
                   key={key}
                   value={key}
                   description={<TruncatedText maxLines={2} content={description} />}
+                  isFavorited={isFavorited}
                   isDisabled={optionDisabled}
                   data-testid={optionDataTestId || key}
                 >

diff --git a/frontend/src/components/TypeaheadSelect.tsx b/frontend/src/components/TypeaheadSelect.tsx
@@ -1,5 +1,9 @@
 import React from 'react';
 import {
+  /**
+   * The Select component is used to build another generic component here
+   */
+  // eslint-disable-next-line no-restricted-imports
   Select,
   SelectOption,
   SelectList,

diff --git a/frontend/src/concepts/connectionTypes/fields/DropdownFormField.tsx b/frontend/src/concepts/connectionTypes/fields/DropdownFormField.tsx
@@ -1,5 +1,18 @@
 import * as React from 'react';
-import { Badge, MenuToggle, Select, SelectList, SelectOption } from '@patternfly/react-core';
+
+import {
+  Badge,
+  MenuToggle,
+  /**
+   * This is a special use case to use the Select component to dynamically generate either single/multi dropdown component
+   * And it allows user to de-select options
+   * No need to replace it with SimpleSelect or MultiSelection component here
+   */
+  // eslint-disable-next-line no-restricted-imports
+  Select,
+  SelectList,
+  SelectOption,
+} from '@patternfly/react-core';
 import { DropdownField } from '~/concepts/connectionTypes/types';
 import { FieldProps } from '~/concepts/connectionTypes/fields/types';
 import DefaultValueTextRenderer from '~/concepts/connectionTypes/fields/DefaultValueTextRenderer';

diff --git a/...d/src/concepts/pipelines/content/compareRuns/metricsSection/PipelineRunArtifactSelect.tsx b/...d/src/concepts/pipelines/content/compareRuns/metricsSection/PipelineRunArtifactSelect.tsx
@@ -1,5 +1,10 @@
 import React from 'react';
 import {
+  /** Special use case for Select in this file
+   * It allows multi-selection in the dropdown while keeps the toggle unchanged
+   * Don't use SimpleSelect here
+   */
+  // eslint-disable-next-line no-restricted-imports
   Select,
   SelectOption,
   SelectList,

diff --git a/frontend/src/pages/BYONImages/BYONImageModal/AcceleratorIdentifierMultiselect.tsx b/frontend/src/pages/BYONImages/BYONImageModal/AcceleratorIdentifierMultiselect.tsx
@@ -1,21 +1,7 @@
-import React, { useEffect, useState } from 'react';
-import {
-  Button,
-  Chip,
-  ChipGroup,
-  MenuToggle,
-  MenuToggleElement,
-  Select,
-  SelectList,
-  SelectOption,
-  SelectOptionProps,
-  TextInputGroup,
-  TextInputGroupMain,
-  TextInputGroupUtilities,
-} from '@patternfly/react-core';
-import { TimesIcon } from '@patternfly/react-icons';
+import React from 'react';
 import useAcceleratorProfiles from '~/pages/notebookController/screens/server/useAcceleratorProfiles';
 import { useDashboardNamespace } from '~/redux/selectors';
+import { MultiSelection } from '~/components/MultiSelection';
 
 type AcceleratorIdentifierMultiselectProps = {
   data: string[];
@@ -27,162 +13,31 @@ export const AcceleratorIdentifierMultiselect: React.FC<AcceleratorIdentifierMul
   setData,
 }) => {
   const { dashboardNamespace } = useDashboardNamespace();
-  const [acceleratorProfiles, loaded, loadError] = useAcceleratorProfiles(dashboardNamespace);
+  const [acceleratorProfiles] = useAcceleratorProfiles(dashboardNamespace);
 
-  const [isOpen, setIsOpen] = React.useState(false);
-  const [inputValue, setInputValue] = React.useState<string>('');
-  const [selectOptions, setSelectOptions] = useState<SelectOptionProps[]>([]);
-  const [onCreation, setOnCreation] = React.useState(false); // Boolean to refresh filter state after new option is created
+  const identifiers = React.useMemo(() => {
+    const uniqueIdentifiers = new Set<string>(data);
 
-  const textInputRef = React.useRef<HTMLInputElement>();
+    // Add identifiers from accelerators
+    acceleratorProfiles.forEach((cr) => {
+      uniqueIdentifiers.add(cr.spec.identifier);
+    });
 
-  useEffect(() => {
-    if (loaded && !loadError) {
-      const uniqueIdentifiers = new Set<string>();
-
-      // Add identifiers from accelerators
-      acceleratorProfiles.forEach((cr) => {
-        uniqueIdentifiers.add(cr.spec.identifier);
-      });
-
-      // Add identifiers from initial data
-      data.forEach((identifier) => {
-        uniqueIdentifiers.add(identifier);
-      });
-
-      // Convert unique identifiers to SelectOptionProps
-      let newOptions = Array.from(uniqueIdentifiers).map((identifier) => ({
-        value: identifier,
-        children: identifier,
-      }));
-
-      // Filter menu items based on the text input value when one exists
-      if (inputValue) {
-        newOptions = newOptions.filter((menuItem) =>
-          String(menuItem.children).toLowerCase().includes(inputValue.toLowerCase()),
-        );
-
-        // When no options are found after filtering, display creation option
-        if (!newOptions.length) {
-          newOptions = [{ children: `Create new option "${inputValue}"`, value: 'create' }];
-        }
-
-        // Open the menu when the input value changes and the new value is not empty
-        if (!isOpen) {
-          setIsOpen(true);
-        }
-      }
-
-      setSelectOptions(newOptions);
-    }
-  }, [acceleratorProfiles, loaded, loadError, data, onCreation, inputValue, isOpen]);
-
-  const onToggleClick = () => {
-    setIsOpen(!isOpen);
-  };
-
-  const onTextInputChange = (_event: React.FormEvent<HTMLInputElement>, value: string) => {
-    setInputValue(value);
-  };
-
-  const onSelect = (value: string) => {
-    if (value) {
-      if (value === 'create') {
-        // Check if the input value already exists in selectOptions
-        if (!selectOptions.some((option) => option.value === inputValue)) {
-          // Add the new option to selectOptions
-          setSelectOptions([...selectOptions, { value: inputValue, children: inputValue }]);
-        }
-        // Update the selected values
-        setData(
-          data.includes(inputValue)
-            ? data.filter((selection) => selection !== inputValue)
-            : [...data, inputValue],
-        );
-        setOnCreation(!onCreation);
-        setInputValue('');
-      } else {
-        // Handle selecting an existing option
-        setData(
-          data.includes(value) ? data.filter((selection) => selection !== value) : [...data, value],
-        );
-      }
-    }
-    textInputRef.current?.focus();
-  };
-
-  const toggle = (toggleRef: React.Ref<MenuToggleElement>) => (
-    <MenuToggle
-      variant="typeahead"
-      onClick={onToggleClick}
-      innerRef={toggleRef}
-      isExpanded={isOpen}
-      isFullWidth
-    >
-      <TextInputGroup isPlain>
-        <TextInputGroupMain
-          value={inputValue}
-          onClick={onToggleClick}
-          onChange={onTextInputChange}
-          id="multi-create-typeahead-select-input"
-          autoComplete="off"
-          innerRef={textInputRef}
-          placeholder="Example, nvidia.com/gpu"
-          role="combobox"
-          isExpanded={isOpen}
-          aria-controls="select-multi-create-typeahead-listbox"
-        >
-          <ChipGroup aria-label="Current selections">
-            {data.map((selection, index) => (
-              <Chip
-                key={index}
-                onClick={(ev) => {
-                  ev.stopPropagation();
-                  onSelect(selection);
-                }}
-              >
-                {selection}
-              </Chip>
-            ))}
-          </ChipGroup>
-        </TextInputGroupMain>
-        <TextInputGroupUtilities>
-          {data.length > 0 && (
-            <Button
-              variant="plain"
-              onClick={() => {
-                setInputValue('');
-                setData([]);
-                textInputRef.current?.focus();
-              }}
-              aria-label="Clear input value"
-            >
-              <TimesIcon aria-hidden />
-            </Button>
-          )}
-        </TextInputGroupUtilities>
-      </TextInputGroup>
-    </MenuToggle>
-  );
+    return Array.from(uniqueIdentifiers);
+  }, [acceleratorProfiles, data]);
 
   return (
-    <Select
-      id="multi-create-typeahead-select"
-      isOpen={isOpen}
-      selected={data}
-      onSelect={(ev, selection) => {
-        if (typeof selection === 'string') {
-          onSelect(selection);
-        }
-      }}
-      onOpenChange={() => setIsOpen(false)}
-      toggle={toggle}
-    >
-      <SelectList isAriaMultiselectable id="select-multi-create-typeahead-listbox">
-        {selectOptions.map((option) => (
-          <SelectOption key={option.value} {...option} ref={null} />
-        ))}
-      </SelectList>
-    </Select>
+    <MultiSelection
+      ariaLabel="Accelerator Identifier Select"
+      value={identifiers.map((identifier) => ({
+        id: identifier,
+        name: identifier,
+        selected: data.includes(identifier),
+      }))}
+      setValue={(newState) => setData(newState.filter((n) => n.selected).map((n) => String(n.id)))}
+      placeholder="Example, nvidia.com/gpu"
+      isCreatable
+      createOptionMessage={(newValue) => `Create new option "${newValue}"`}
+    />
   );
 };