diff --git a/frontend/.eslintrc b/frontend/.eslintrc index 5120c919b5..41680a0887 100755 --- 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/pages/modelRegistry/modelVersionDeployModal.ts b/frontend/src/__tests__/cypress/cypress/pages/modelRegistry/modelVersionDeployModal.ts index 9ab23f9738..8887055e77 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/modelRegistry/modelVersionDeployModal.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/modelRegistry/modelVersionDeployModal.ts @@ -11,7 +11,7 @@ class ModelVersionDeployModal extends Modal { selectProjectByName(name: string) { this.findProjectSelector().click(); - this.find().findByRole('menuitem', { name }).click(); + this.find().findByRole('option', { name }).click(); } } diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/acceleratorProfiles/manageAcceleratorProfiles.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/acceleratorProfiles/manageAcceleratorProfiles.cy.ts index 5c03c785eb..9f9f525387 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/acceleratorProfiles/manageAcceleratorProfiles.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/acceleratorProfiles/manageAcceleratorProfiles.cy.ts @@ -181,8 +181,10 @@ describe('Manage Accelerator Profile', () => { it('one preset identifier is auto filled and disabled', () => { initIntercepts({}); identifierAcceleratorProfile.visit(); - identifierAcceleratorProfile.findIdentifierInput().should('have.value', 'test-identifier'); - identifierAcceleratorProfile.findIdentifierInput().should('be.disabled'); + identifierAcceleratorProfile + .findAcceleratorIdentifierSelect() + .should('contain.text', 'test-identifier'); + identifierAcceleratorProfile.findAcceleratorIdentifierSelect().should('be.disabled'); }); it('multiple preset identifiers show dropdown', () => { diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/modelServingGlobal.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/modelServingGlobal.cy.ts index 2fc869b93f..bf1308f081 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/modelServingGlobal.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/modelServingGlobal.cy.ts @@ -352,10 +352,12 @@ describe('Model Serving Global', () => { // test filling in minimum required fields inferenceServiceModal.findModelNameInput().type('Test Name'); - inferenceServiceModal.findServingRuntimeSelect().findSelectOption('OVMS Model Serving').click(); + inferenceServiceModal.findServingRuntimeSelect().should('contain.text', 'OVMS Model Serving'); + inferenceServiceModal.findServingRuntimeSelect().should('be.disabled'); inferenceServiceModal.findModelFrameworkSelect().findSelectOption('onnx - 1').click(); inferenceServiceModal.findSubmitButton().should('be.disabled'); - inferenceServiceModal.findExistingConnectionSelect().findSelectOption('Test Secret').click(); + inferenceServiceModal.findExistingConnectionSelect().should('contain.text', 'Test Secret'); + inferenceServiceModal.findExistingConnectionSelect().should('be.disabled'); inferenceServiceModal.findLocationPathInput().type('test-model/'); inferenceServiceModal.findSubmitButton().should('be.enabled'); inferenceServiceModal.findNewDataConnectionOption().click(); @@ -436,10 +438,12 @@ describe('Model Serving Global', () => { // test filling in minimum required fields inferenceServiceModal.findModelNameInput().type('trigger-error'); - inferenceServiceModal.findServingRuntimeSelect().findSelectOption('OVMS Model Serving').click(); + inferenceServiceModal.findServingRuntimeSelect().should('contain.text', 'OVMS Model Serving'); + inferenceServiceModal.findServingRuntimeSelect().should('be.disabled'); inferenceServiceModal.findModelFrameworkSelect().findSelectOption('onnx - 1').click(); inferenceServiceModal.findSubmitButton().should('be.disabled'); - inferenceServiceModal.findExistingConnectionSelect().findSelectOption('Test Secret').click(); + inferenceServiceModal.findExistingConnectionSelect().should('contain.text', 'Test Secret'); + inferenceServiceModal.findExistingConnectionSelect().should('be.disabled'); inferenceServiceModal.findLocationPathInput().type('test-model/'); inferenceServiceModal.findSubmitButton().should('be.enabled'); inferenceServiceModal.findLocationPathInput().type('test-model/'); 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 index 003a02ae74..8c04e31a04 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/servingRuntimeList.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/servingRuntimeList.cy.ts @@ -421,7 +421,8 @@ describe('Serving Runtime List', () => { inferenceServiceModal.findModelNameInput().type('Test Name'); inferenceServiceModal.findModelFrameworkSelect().findSelectOption('onnx - 1').click(); inferenceServiceModal.findSubmitButton().should('be.disabled'); - inferenceServiceModal.findExistingConnectionSelect().findSelectOption('Test Secret').click(); + inferenceServiceModal.findExistingConnectionSelect().should('contain.text', 'Test Secret'); + inferenceServiceModal.findExistingConnectionSelect().should('be.disabled'); inferenceServiceModal.findLocationPathInput().type('test-model/'); inferenceServiceModal.findSubmitButton().should('be.enabled'); inferenceServiceModal.findNewDataConnectionOption().click(); @@ -501,7 +502,7 @@ describe('Serving Runtime List', () => { inferenceServiceModalEdit .findExistingConnectionSelect() .should('have.text', 'Test Secret') - .should('be.enabled'); + .should('be.disabled'); }); it('ModelMesh ServingRuntime list', () => { @@ -715,7 +716,8 @@ describe('Serving Runtime List', () => { kserveModal.findAuthenticationCheckbox().check(); kserveModal.findExternalRouteError().should('not.exist'); kserveModal.findServiceAccountNameInput().should('have.value', 'default-name'); - kserveModal.findExistingConnectionSelect().findSelectOption('Test Secret').click(); + kserveModal.findExistingConnectionSelect().should('contain.text', 'Test Secret'); + kserveModal.findExistingConnectionSelect().should('be.disabled'); kserveModal.findLocationPathInput().type('test-model/'); kserveModal.findSubmitButton().should('be.enabled'); kserveModal.findNewDataConnectionOption().click(); @@ -880,7 +882,8 @@ describe('Serving Runtime List', () => { kserveModal.findModelNameInput().type('Test Name'); kserveModal.findServingRuntimeTemplateDropdown().findSelectOption('Caikit').click(); kserveModal.findModelFrameworkSelect().findSelectOption('onnx - 1').click(); - kserveModal.findExistingConnectionSelect().findSelectOption('Test Secret').click(); + kserveModal.findExistingConnectionSelect().should('contain.text', 'Test Secret'); + kserveModal.findExistingConnectionSelect().should('be.disabled'); kserveModal.findNewDataConnectionOption().click(); kserveModal.findLocationNameInput().type('Test Name'); kserveModal.findLocationAccessKeyInput().type('test-key'); @@ -1105,7 +1108,8 @@ describe('Serving Runtime List', () => { kserveModal.findServingRuntimeTemplateDropdown().findSelectOption('Caikit').click(); kserveModal.findModelFrameworkSelect().findSelectOption('onnx - 1').click(); kserveModal.findSubmitButton().should('be.disabled'); - kserveModal.findExistingConnectionSelect().findSelectOption('Test Secret').click(); + kserveModal.findExistingConnectionSelect().should('contain.text', 'Test Secret'); + kserveModal.findExistingConnectionSelect().should('be.disabled'); kserveModal.findLocationPathInput().type('test-model/'); kserveModal.findSubmitButton().should('be.enabled'); @@ -1839,7 +1843,8 @@ describe('Serving Runtime List', () => { kserveModal.findServingRuntimeTemplateDropdown().findSelectOption('Caikit').click(); kserveModal.findModelFrameworkSelect().findSelectOption('onnx - 1').click(); kserveModal.findSubmitButton().should('be.disabled'); - kserveModal.findExistingConnectionSelect().findSelectOption('Test Secret').click(); + kserveModal.findExistingConnectionSelect().should('contain.text', 'Test Secret'); + kserveModal.findExistingConnectionSelect().should('be.disabled'); kserveModal.findLocationPathInput().type('test-model/'); kserveModal.findSubmitButton().should('be.enabled'); kserveModal.findSubmitButton().click(); @@ -1872,7 +1877,8 @@ describe('Serving Runtime List', () => { kserveModal.findServingRuntimeTemplateDropdown().findSelectOption('Caikit').click(); kserveModal.findModelFrameworkSelect().findSelectOption('onnx - 1').click(); kserveModal.findSubmitButton().should('be.disabled'); - kserveModal.findExistingConnectionSelect().findSelectOption('Test Secret').click(); + kserveModal.findExistingConnectionSelect().should('contain.text', 'Test Secret'); + kserveModal.findExistingConnectionSelect().should('be.disabled'); kserveModal.findLocationPathInput().type('test-model/'); kserveModal.findSubmitButton().should('be.enabled'); kserveModal.findSubmitButton().click(); diff --git a/frontend/src/components/MultiSelection.tsx b/frontend/src/components/MultiSelection.tsx index 666e180bc9..772425fa81 100644 --- 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 index 27960895ff..ec123cce3a 100644 --- a/frontend/src/components/SimpleSelect.tsx +++ b/frontend/src/components/SimpleSelect.tsx @@ -2,15 +2,20 @@ import * as React from 'react'; import { Truncate, MenuToggle, + // eslint-disable-next-line no-restricted-imports Select, SelectList, SelectOption, SelectGroup, Divider, MenuToggleProps, + FormHelperText, + HelperText, + HelperTextItem, } from '@patternfly/react-core'; import './SimpleSelect.scss'; +import TruncatedText from '~/components/TruncatedText'; export type SimpleSelectOption = { key: string; @@ -19,6 +24,8 @@ export type SimpleSelectOption = { dropdownLabel?: React.ReactNode; isPlaceholder?: boolean; isDisabled?: boolean; + isFavorited?: boolean; + dataTestId?: string; }; export type SimpleGroupSelectOption = { @@ -39,6 +46,7 @@ type SimpleSelectProps = { isDisabled?: boolean; icon?: React.ReactNode; dataTestId?: string; + previewDescription?: boolean; } & Omit< React.ComponentProps, 'isOpen' | 'toggle' | 'dropdownItems' | 'onChange' | 'selected' @@ -56,88 +64,140 @@ const SimpleSelect: React.FC = ({ icon, dataTestId, toggleProps, + previewDescription = true, + popperProps, ...props }) => { const [open, setOpen] = React.useState(false); + const groupedOptionsFlat = React.useMemo( + () => + groupedOptions?.reduce((acc, group) => [...acc, ...group.options], []), + [groupedOptions], + ); + const findOptionForKey = (key: string) => - options?.find((option) => option.key === key) || - groupedOptions - ?.reduce((acc, group) => [...acc, ...group.options], []) - .find((o) => o.key === key); + options?.find((option) => option.key === key) || groupedOptionsFlat?.find((o) => o.key === key); const selectedOption = value ? findOptionForKey(value) : undefined; const selectedLabel = selectedOption?.label ?? placeholder; + const totalOptions = React.useMemo( + () => [...(options || []), ...(groupedOptionsFlat || [])], + [options, groupedOptionsFlat], + ); + + // If there is only one option, call the onChange function + const totalOptionsKey = totalOptions.length === 1 ? totalOptions[0].key : null; + React.useEffect(() => { + 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]); + return ( - { + onChange( + String(selectValue), + !!selectValue && (findOptionForKey(String(selectValue))?.isPlaceholder ?? false), + ); + setOpen(false); + }} + onOpenChange={setOpen} + toggle={(toggleRef) => ( + setOpen(!open)} + icon={icon} + isExpanded={open} + isDisabled={totalOptions.length <= 1 || isDisabled} + isFullWidth={isFullWidth} + {...toggleProps} + > + {toggleLabel || } + + )} + shouldFocusToggleOnSelect + popperProps={{ maxWidth: 'trigger', ...popperProps }} + > + {groupedOptions?.map((group, index) => ( + <> + {index > 0 ? : null} + + + {group.options.map( + ({ + key, + label, + dropdownLabel, + description, + isFavorited, + isDisabled: optionDisabled, + dataTestId: optionDataTestId, + }) => ( + } + isDisabled={optionDisabled} + isFavorited={isFavorited} + data-testid={optionDataTestId || key} + > + {dropdownLabel || label} + + ), + )} + + + + )) ?? null} + {options?.length ? ( + + {groupedOptions?.length ? : null} + {options.map( + ({ + key, + label, + dropdownLabel, + description, + isFavorited, + isDisabled: optionDisabled, + dataTestId: optionDataTestId, + }) => ( + } + isFavorited={isFavorited} + isDisabled={optionDisabled} + data-testid={optionDataTestId || key} + > + {dropdownLabel || label} + + ), + )} + + ) : null} + + {previewDescription && selectedOption?.description ? ( + + + + + + + ) : null} - + ); }; diff --git a/frontend/src/components/TruncatedText.tsx b/frontend/src/components/TruncatedText.tsx index ea18037181..47f4e54f80 100644 --- a/frontend/src/components/TruncatedText.tsx +++ b/frontend/src/components/TruncatedText.tsx @@ -3,8 +3,8 @@ import { Tooltip } from '@patternfly/react-core'; type TruncatedTextProps = { maxLines: number; - content: string; -} & React.HTMLProps; + content: React.ReactNode; +} & Omit, 'content'>; const TruncatedText: React.FC = ({ maxLines, content, ...props }) => { const outerElementRef = React.useRef(null); diff --git a/frontend/src/components/TypeaheadSelect.tsx b/frontend/src/components/TypeaheadSelect.tsx index 1d9aab25c3..fe89450e8d 100644 --- 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 index 0143c97be4..bf8db6c7b5 100644 --- 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/frontend/src/concepts/pipelines/content/compareRuns/metricsSection/PipelineRunArtifactSelect.tsx b/frontend/src/concepts/pipelines/content/compareRuns/metricsSection/PipelineRunArtifactSelect.tsx index 39367e9c3d..bf68cbc125 100644 --- a/frontend/src/concepts/pipelines/content/compareRuns/metricsSection/PipelineRunArtifactSelect.tsx +++ b/frontend/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/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableToolbarBase.tsx b/frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableToolbarBase.tsx index 98ec2f55ba..b3ef82c1fb 100644 --- a/frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableToolbarBase.tsx +++ b/frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableToolbarBase.tsx @@ -78,7 +78,7 @@ const PipelineRunTableToolbarBase: React.FC = [FilterOptions.STATUS]: ({ value, onChange, ...props }) => ( ({ key: v, diff --git a/frontend/src/concepts/roleBinding/RoleBindingPermissionsPermissionSelection.tsx b/frontend/src/concepts/roleBinding/RoleBindingPermissionsPermissionSelection.tsx index be2e6ae045..fc093a8f32 100644 --- a/frontend/src/concepts/roleBinding/RoleBindingPermissionsPermissionSelection.tsx +++ b/frontend/src/concepts/roleBinding/RoleBindingPermissionsPermissionSelection.tsx @@ -28,6 +28,7 @@ const RoleBindingPermissionsPermissionSelection: React.FC< onSelect(castRoleBindingPermissionsRoleType(newSelection)); }} popperProps={{ direction: 'down' }} + previewDescription={false} /> ); diff --git a/frontend/src/pages/BYONImages/BYONImageModal/AcceleratorIdentifierMultiselect.tsx b/frontend/src/pages/BYONImages/BYONImageModal/AcceleratorIdentifierMultiselect.tsx index 5e646a6c60..ec6b79d13b 100644 --- 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 { const { dashboardNamespace } = useDashboardNamespace(); - const [acceleratorProfiles, loaded, loadError] = useAcceleratorProfiles(dashboardNamespace); + const [acceleratorProfiles] = useAcceleratorProfiles(dashboardNamespace); - const [isOpen, setIsOpen] = React.useState(false); - const [inputValue, setInputValue] = React.useState(''); - const [selectOptions, setSelectOptions] = useState([]); - const [onCreation, setOnCreation] = React.useState(false); // Boolean to refresh filter state after new option is created + const identifiers = React.useMemo(() => { + const uniqueIdentifiers = new Set(data); - const textInputRef = React.useRef(); + // Add identifiers from accelerators + acceleratorProfiles.forEach((cr) => { + uniqueIdentifiers.add(cr.spec.identifier); + }); - useEffect(() => { - if (loaded && !loadError) { - const uniqueIdentifiers = new Set(); - - // 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, 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) => ( - - - - - {data.map((selection, index) => ( - { - ev.stopPropagation(); - onSelect(selection); - }} - > - {selection} - - ))} - - - - {data.length > 0 && ( - - )} - - - - ); + return Array.from(uniqueIdentifiers); + }, [acceleratorProfiles, data]); return ( - + ({ + 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}"`} + /> ); }; diff --git a/frontend/src/pages/acceleratorProfiles/screens/manage/IdentifierSelectField.tsx b/frontend/src/pages/acceleratorProfiles/screens/manage/IdentifierSelectField.tsx index 1dc7cdf275..d3f69da45a 100644 --- a/frontend/src/pages/acceleratorProfiles/screens/manage/IdentifierSelectField.tsx +++ b/frontend/src/pages/acceleratorProfiles/screens/manage/IdentifierSelectField.tsx @@ -1,5 +1,6 @@ -import { MenuToggle, Select, SelectList, SelectOption, TextInput } from '@patternfly/react-core'; -import React, { useEffect, useMemo } from 'react'; +import React, { useMemo } from 'react'; +import { TextInput } from '@patternfly/react-core'; +import SimpleSelect from '~/components/SimpleSelect'; type IdentifierSelectFieldProps = { value: string; @@ -12,54 +13,28 @@ export const IdentifierSelectField: React.FC = ({ onChange, identifierOptions = [], }) => { - const [isOpen, setIsOpen] = React.useState(false); - // remove possible duplicates - const options = useMemo(() => Array.from(new Set(identifierOptions)), [identifierOptions]); - - // auto-select if there is only one option - useEffect(() => { - if (options.length === 1) { - onChange(options[0]); - } - // Do not include onChange callback as dependency - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [options]); + const options = useMemo( + () => + Array.from(new Set(identifierOptions)).map((option) => ({ + key: option, + label: option, + })), + [identifierOptions], + ); - if (options.length > 1) { + if (options.length >= 1) { return ( - + placeholder="Select an identifier" + /> ); } @@ -69,7 +44,6 @@ export const IdentifierSelectField: React.FC = ({ value={value} id="accelerator-identifier" name="accelerator-identifier" - isDisabled={options.length === 1} onChange={(_, identifier) => onChange(identifier)} placeholder="Example, nvidia.com/gpu" aria-label="Identifier" diff --git a/frontend/src/pages/connectionTypes/manage/ConnectionTypeDataFieldModal.tsx b/frontend/src/pages/connectionTypes/manage/ConnectionTypeDataFieldModal.tsx index 370114a393..5c6d20216a 100644 --- a/frontend/src/pages/connectionTypes/manage/ConnectionTypeDataFieldModal.tsx +++ b/frontend/src/pages/connectionTypes/manage/ConnectionTypeDataFieldModal.tsx @@ -6,12 +6,8 @@ import { FormHelperText, HelperText, HelperTextItem, - MenuToggle, Modal, Popover, - Select, - SelectList, - SelectOption, TextArea, TextInput, } from '@patternfly/react-core'; @@ -38,6 +34,7 @@ import DashboardPopupIconButton from '~/concepts/dashboard/DashboardPopupIconBut import DataFieldPropertiesForm from '~/pages/connectionTypes/manage/DataFieldPropertiesForm'; import { prepareFieldForSave } from '~/pages/connectionTypes/manage/manageFieldUtils'; import useGenericObjectState from '~/utilities/useGenericObjectState'; +import SimpleSelect from '~/components/SimpleSelect'; const isConnectionTypeFieldType = ( fieldType: string | number | undefined, @@ -74,7 +71,6 @@ export const ConnectionTypeDataFieldModal: React.FC = ({ const canSubmit = React.useRef(data).current !== data || !isEdit; const { name, description, envVar, fieldType, required, properties } = data; - const [isTypeSelectOpen, setIsTypeSelectOpen] = React.useState(false); const [isPropertiesValid, setPropertiesValid] = React.useState(true); const [autoGenerateEnvVar, setAutoGenerateEnvVar] = React.useState(!envVar); @@ -209,47 +205,31 @@ export const ConnectionTypeDataFieldModal: React.FC = ({ - + options={connectionTypeDataFields + .map((value) => ({ label: fieldTypeToString(value), value })) + .toSorted((a, b) => a.label.localeCompare(b.label)) + .map(({ value, label }) => ({ + key: value, + label: value, + dropdownLabel: label, + dataTestId: `field-${value}-select`, + }))} + shouldFocusToggleOnSelect + isFullWidth + value={fieldType} + toggleLabel={fieldTypeToString(fieldType)} + /> = ({ value={selectedSection?.key} onChange={(key) => setSelectedSection(options.find((s) => s.key === key))} isFullWidth - isDisabled={options.length === 1} /> diff --git a/frontend/src/pages/modelRegistry/screens/ModelRegistrySelector.tsx b/frontend/src/pages/modelRegistry/screens/ModelRegistrySelector.tsx index 5f54484c7c..a07c0f7c55 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelRegistrySelector.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelRegistrySelector.tsx @@ -6,28 +6,19 @@ import { DescriptionListDescription, DescriptionListGroup, DescriptionListTerm, - Divider, Flex, FlexItem, Icon, - MenuToggle, Popover, - Select, - SelectGroup, - SelectList, - SelectOption, Tooltip, } from '@patternfly/react-core'; import truncateStyles from '@patternfly/react-styles/css/components/Truncate/truncate'; import { InfoCircleIcon, BlueprintIcon } from '@patternfly/react-icons'; import { useBrowserStorage } from '~/components/browserStorage'; import { ModelRegistrySelectorContext } from '~/concepts/modelRegistry/context/ModelRegistrySelectorContext'; -import { - getDescriptionFromK8sResource, - getDisplayNameFromK8sResource, - getResourceNameFromK8sResource, -} from '~/concepts/k8s/utils'; +import { getDescriptionFromK8sResource, getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; import { ServiceKind } from '~/k8sTypes'; +import SimpleSelect, { SimpleSelectOption } from '~/components/SimpleSelect'; const MODEL_REGISTRY_FAVORITE_STORAGE_KEY = 'odh.dashboard.model.registry.favorite'; @@ -46,7 +37,6 @@ const ModelRegistrySelector: React.FC = ({ ModelRegistrySelectorContext, ); const selection = modelRegistryServices.find((mr) => mr.metadata.name === modelRegistry); - const [isOpen, setIsOpen] = React.useState(false); const [favorites, setFavorites] = useBrowserStorage( MODEL_REGISTRY_FAVORITE_STORAGE_KEY, [], @@ -81,72 +71,49 @@ const ModelRegistrySelector: React.FC = ({ ); }; - const options = [ - - - {modelRegistryServices.map((mr) => ( - - {getDisplayNameFromK8sResource(mr)} - - ))} - - , - ]; - - const createFavorites = (favIds: string[]) => { - const favorite: JSX.Element[] = []; + const allOptions: SimpleSelectOption[] = modelRegistryServices.map((mr) => ({ + key: mr.metadata.name, + label: mr.metadata.name, + dropdownLabel: getDisplayNameFromK8sResource(mr), + description: getMRSelectDescription(mr), + isFavorited: favorites.includes(mr.metadata.name), + })); - options.forEach((item) => { - if (item.type === SelectList) { - item.props.children.filter( - (child: JSX.Element) => favIds.includes(child.props.value) && favorite.push(child), - ); - } else if (item.type === SelectGroup) { - item.props.children.props.children.filter( - (child: JSX.Element) => favIds.includes(child.props.value) && favorite.push(child), - ); - } else if (favIds.includes(item.props.value)) { - favorite.push(item); - } - }); - - return favorite; - }; + const favoriteOptions = (favIds: string[]) => + allOptions.filter((option) => favIds.includes(option.key)); const selector = ( - + /> ); if (primary) { diff --git a/frontend/src/pages/modelRegistry/screens/components/DeployRegisteredModelModal.tsx b/frontend/src/pages/modelRegistry/screens/components/DeployRegisteredModelModal.tsx index 7f5e0d4e0b..f6c655c8dd 100644 --- a/frontend/src/pages/modelRegistry/screens/components/DeployRegisteredModelModal.tsx +++ b/frontend/src/pages/modelRegistry/screens/components/DeployRegisteredModelModal.tsx @@ -25,7 +25,6 @@ const DeployRegisteredModelModal: React.FC = ({ onCancel, onSubmit, }) => { - const [isProjectSelectorOpen, setProjectSelectorOpen] = React.useState(false); const { servingRuntimeTemplates: [templates], servingRuntimeTemplateOrder: { data: templateOrder }, @@ -66,8 +65,6 @@ const DeployRegisteredModelModal: React.FC = ({ selectedProject={selectedProject} setSelectedProject={setSelectedProject} error={error} - isOpen={isProjectSelectorOpen} - setOpen={setProjectSelectorOpen} /> ); diff --git a/frontend/src/pages/modelServing/customServingRuntimes/CustomServingRuntimeAPIProtocolSelector.tsx b/frontend/src/pages/modelServing/customServingRuntimes/CustomServingRuntimeAPIProtocolSelector.tsx index 95ca5982ff..e6d1b61ba2 100644 --- a/frontend/src/pages/modelServing/customServingRuntimes/CustomServingRuntimeAPIProtocolSelector.tsx +++ b/frontend/src/pages/modelServing/customServingRuntimes/CustomServingRuntimeAPIProtocolSelector.tsx @@ -17,12 +17,6 @@ const CustomServingRuntimeAPIProtocolSelector: React.FC< selectedPlatforms.includes(ServingRuntimePlatform.MULTI) && !selectedPlatforms.includes(ServingRuntimePlatform.SINGLE); - React.useEffect(() => { - if (isOnlyModelMesh) { - setSelectedAPIProtocol(ServingRuntimeAPIProtocol.REST); - } - }, [isOnlyModelMesh, setSelectedAPIProtocol]); - const options = [ { key: ServingRuntimeAPIProtocol.REST, @@ -48,7 +42,6 @@ const CustomServingRuntimeAPIProtocolSelector: React.FC< dataTestId="custom-serving-api-protocol-selection" aria-label="Select a model serving api protocol" placeholder="Select a value" - isDisabled={isOnlyModelMesh} options={options} value={selectedAPIProtocol || ''} onChange={(key) => { diff --git a/frontend/src/pages/modelServing/screens/metrics/bias/BiasConfigurationPage/BiasConfigurationModal/MetricTypeField.tsx b/frontend/src/pages/modelServing/screens/metrics/bias/BiasConfigurationPage/BiasConfigurationModal/MetricTypeField.tsx index 526fb225d8..0a43e33635 100644 --- a/frontend/src/pages/modelServing/screens/metrics/bias/BiasConfigurationPage/BiasConfigurationModal/MetricTypeField.tsx +++ b/frontend/src/pages/modelServing/screens/metrics/bias/BiasConfigurationPage/BiasConfigurationModal/MetricTypeField.tsx @@ -24,11 +24,12 @@ const MetricTypeField: React.FC = ({ fieldId, value, onCha onChange(selectedValue); } }} - options={enumIterator(BiasMetricType).map(([type, enumValue]) => ({ - key: enumValue, + options={enumIterator(BiasMetricType).map(([, type]) => ({ + key: type, label: METRIC_TYPE_DISPLAY_NAME[type], description: METRIC_TYPE_DESCRIPTION[type], }))} + value={value} placeholder="Select" toggleLabel={value} toggleProps={{ id: fieldId }} diff --git a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/DataConnectionExistingField.tsx b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/DataConnectionExistingField.tsx index 0f95a8a1aa..c5f6d050ea 100644 --- a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/DataConnectionExistingField.tsx +++ b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/DataConnectionExistingField.tsx @@ -32,10 +32,8 @@ const DataConnectionExistingField: React.FC = ( dataConnections, }) => { const connectionsWithoutBucket = filterOutConnectionsWithoutBucket(dataConnections); - const isDataConnectionsEmpty = connectionsWithoutBucket.length === 0; - const placeholderText = isDataConnectionsEmpty - ? 'No data connections available to select' - : 'Select...'; + const placeholderText = + connectionsWithoutBucket.length === 0 ? 'No data connections available to select' : 'Select...'; const selectedDataConnection = connectionsWithoutBucket.find( (connection) => connection.dataConnection.data.metadata.name === data.storage.dataConnection, @@ -77,7 +75,6 @@ const DataConnectionExistingField: React.FC = ( ({ key: connection.dataConnection.data.metadata.name, dropdownLabel: getLabeledOption(connection), diff --git a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/InferenceServiceFrameworkSection.tsx b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/InferenceServiceFrameworkSection.tsx index 25a7bfdd9d..d7acb40085 100644 --- a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/InferenceServiceFrameworkSection.tsx +++ b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/InferenceServiceFrameworkSection.tsx @@ -59,7 +59,6 @@ const InferenceServiceFrameworkSection: React.FC { const name = framework.version diff --git a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/InferenceServiceServingRuntimeSection.tsx b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/InferenceServiceServingRuntimeSection.tsx index e3a947f01a..8289e5aa88 100644 --- a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/InferenceServiceServingRuntimeSection.tsx +++ b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/InferenceServiceServingRuntimeSection.tsx @@ -52,7 +52,6 @@ const InferenceServiceServingRuntimeSection: React.FC< ({ key: servingRuntime.metadata.name, label: getDisplayNameFromK8sResource(servingRuntime), diff --git a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ProjectSelector.tsx b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ProjectSelector.tsx index f3102a8b93..8098506172 100644 --- a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ProjectSelector.tsx +++ b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ProjectSelector.tsx @@ -1,34 +1,22 @@ import React from 'react'; -import { - Alert, - FormGroup, - MenuToggle, - Select, - SelectList, - SelectOption, - Stack, - StackItem, -} from '@patternfly/react-core'; +import { Alert, FormGroup, Stack, StackItem } from '@patternfly/react-core'; import { Link } from 'react-router-dom'; import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; import { byName, ProjectsContext } from '~/concepts/projects/ProjectsContext'; import { ProjectKind } from '~/k8sTypes'; import { ProjectSectionID } from '~/pages/projects/screens/detail/types'; +import SimpleSelect from '~/components/SimpleSelect'; type ProjectSelectorProps = { selectedProject: ProjectKind | null; setSelectedProject: (project: ProjectKind | null) => void; error?: Error; - isOpen: boolean; - setOpen: (open: boolean) => void; }; const ProjectSelector: React.FC = ({ selectedProject, setSelectedProject, error, - isOpen, - setOpen, }) => { const { projects } = React.useContext(ProjectsContext); @@ -36,49 +24,31 @@ const ProjectSelector: React.FC = ({ - + value={selectedProject?.metadata.name} + options={projects.map((project) => ({ + key: project.metadata.name, + value: project.metadata.name, + label: getDisplayNameFromK8sResource(project), + }))} + dataTestId="deploy-model-project-selector" + toggleLabel={ + selectedProject + ? getDisplayNameFromK8sResource(selectedProject) + : 'Select target project' + } + toggleProps={{ id: 'deploy-model-project-selector' }} + /> {error && selectedProject && ( diff --git a/frontend/src/pages/modelServing/screens/projects/NIMServiceModal/NIMModelListSection.tsx b/frontend/src/pages/modelServing/screens/projects/NIMServiceModal/NIMModelListSection.tsx index 0bc98bfea1..fdbadcfb3d 100644 --- a/frontend/src/pages/modelServing/screens/projects/NIMServiceModal/NIMModelListSection.tsx +++ b/frontend/src/pages/modelServing/screens/projects/NIMServiceModal/NIMModelListSection.tsx @@ -85,7 +85,7 @@ const NIMModelListSection: React.FC = ({ ({ dataTestId="model-server-size-selection" isFullWidth options={sizeOptions()} + value={data.modelSize.name} toggleProps={{ id: 'model-server-size-selection' }} toggleLabel={data.modelSize.name || 'Select a model server size'} onChange={(option) => { diff --git a/frontend/src/pages/modelServing/screens/projects/ServingRuntimeModal/ServingRuntimeTemplateSection.tsx b/frontend/src/pages/modelServing/screens/projects/ServingRuntimeModal/ServingRuntimeTemplateSection.tsx index 9b20468700..ded7b4b7e6 100644 --- a/frontend/src/pages/modelServing/screens/projects/ServingRuntimeModal/ServingRuntimeTemplateSection.tsx +++ b/frontend/src/pages/modelServing/screens/projects/ServingRuntimeModal/ServingRuntimeTemplateSection.tsx @@ -62,7 +62,7 @@ const ServingRuntimeTemplateSection: React.FC = ({ onChange={(selection) => { setStorageClassName(selection); }} - isDisabled={ - disableStorageClassSelect || !storageClassesLoaded || enabledStorageClasses.length <= 1 - } + isDisabled={disableStorageClassSelect || !storageClassesLoaded} placeholder="Select storage class" popperProps={{ appendTo: menuAppendTo }} />