From 364c30196c6b351b94ba89af31ab9bbccb8ac7a4 Mon Sep 17 00:00:00 2001 From: manaswinidas Date: Wed, 10 Apr 2024 19:10:15 +0530 Subject: [PATCH] Fixes Image selection dropdown scroll issue --- .../cypress/e2e/projects/Workbench.cy.ts | 54 ++++++++++++++++- .../cypress/cypress/pages/modelServing.ts | 2 +- .../cypress/cypress/pages/workbench.ts | 7 ++- .../cypress/support/commands/application.ts | 19 +++++- .../src/components/SimpleDropdownSelect.tsx | 58 +++++++++++-------- .../dashboard/DashboardSearchField.tsx | 2 +- .../compareRuns/CompareRunTableToolbar.tsx | 2 +- .../contentSections/TriggerTypeField.tsx | 2 +- .../pipelineRun/runLogs/LogsTab.tsx | 2 +- .../pipelineRun/PipelineRunTableToolbar.tsx | 2 +- .../manage/tolerations/TolerationFields.tsx | 4 +- ...ustomServingRuntimeAPIProtocolSelector.tsx | 2 +- .../CustomServingRuntimePlatformsSelector.tsx | 2 +- .../ServingRuntimeTemplateSection.tsx | 1 + .../imageSelector/ImageStreamSelector.tsx | 3 +- 15 files changed, 121 insertions(+), 41 deletions(-) diff --git a/frontend/src/__tests__/cypress/cypress/e2e/projects/Workbench.cy.ts b/frontend/src/__tests__/cypress/cypress/e2e/projects/Workbench.cy.ts index 7f0b38d221..f43db7f18c 100644 --- a/frontend/src/__tests__/cypress/cypress/e2e/projects/Workbench.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/e2e/projects/Workbench.cy.ts @@ -53,7 +53,56 @@ const initIntercepts = ({ isEmpty = false }: HandlersProps) => { cy.interceptK8sList(PodModel, mockK8sResourceList([mockPodK8sResource({})])); cy.interceptK8sList( ImageStreamModel, - mockK8sResourceList([mockImageStreamK8sResource({ namespace: 'opendatahub' })]), + mockK8sResourceList([ + mockImageStreamK8sResource({ + namespace: 'opendatahub', + }), + mockImageStreamK8sResource({ + namespace: 'opendatahub', + name: 'test-1', + displayName: 'Test image 1', + }), + mockImageStreamK8sResource({ + namespace: 'opendatahub', + name: 'test-2', + displayName: 'Test image 2', + }), + mockImageStreamK8sResource({ + namespace: 'opendatahub', + name: 'test-3', + displayName: 'Test image 3', + }), + mockImageStreamK8sResource({ + namespace: 'opendatahub', + name: 'test-4', + displayName: 'Test image 4', + }), + mockImageStreamK8sResource({ + namespace: 'opendatahub', + name: 'test-5', + displayName: 'Test image 5', + }), + mockImageStreamK8sResource({ + namespace: 'opendatahub', + name: 'test-6', + displayName: 'Test image 6', + }), + mockImageStreamK8sResource({ + namespace: 'opendatahub', + name: 'test-7', + displayName: 'Test image 7', + }), + mockImageStreamK8sResource({ + namespace: 'opendatahub', + name: 'test-8', + displayName: 'Test image 8', + }), + mockImageStreamK8sResource({ + namespace: 'opendatahub', + name: 'test-9', + displayName: 'Test image 9', + }), + ]), ); cy.interceptK8s(SecretModel, mockSecretK8sResource({ name: 'aws-connection-db-1' })); cy.interceptK8s('PATCH', NotebookModel, mockNotebookK8sResource({})).as('stopWorkbench'); @@ -107,7 +156,8 @@ describe('Workbench page', () => { verifyRelativeURL('/projects/test-project/spawner'); createSpawnerPage.findNameInput().fill('test-project'); createSpawnerPage.findDescriptionInput().fill('test-description'); - createSpawnerPage.selectNotebookImage('Test Image Python v3.8'); + //to check scrollable dropdown selection + createSpawnerPage.findNotebookImage('test-9').click(); createSpawnerPage.selectContainerSize( 'XSmall Limits: 0.5 CPU, 500Mi Memory Requests: 0.1 CPU, 100Mi Memory', ); diff --git a/frontend/src/__tests__/cypress/cypress/pages/modelServing.ts b/frontend/src/__tests__/cypress/cypress/pages/modelServing.ts index b1ed5d6079..44f0d9a024 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/modelServing.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/modelServing.ts @@ -146,7 +146,7 @@ class ServingRuntimeModal extends Modal { } findServingRuntimeTemplateDropdown() { - return this.find().find('#serving-runtime-template-selection'); + return this.find().findByTestId('serving-runtime-template-selection'); } findModelRouteCheckbox() { diff --git a/frontend/src/__tests__/cypress/cypress/pages/workbench.ts b/frontend/src/__tests__/cypress/cypress/pages/workbench.ts index 7813d2eb77..5a4c0e64fa 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/workbench.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/workbench.ts @@ -234,8 +234,11 @@ class CreateSpawnerPage { ); } - selectNotebookImage(name: string) { - cy.findByTestId('workbench-image-stream-selection').findDropdownItem(name).click(); + findNotebookImage(name: string) { + return cy + .findByTestId('workbench-image-stream-selection') + .findDropdownItemByTestId(`dropdown-item ${name}`) + .scrollIntoView(); } selectContainerSize(name: string) { diff --git a/frontend/src/__tests__/cypress/cypress/support/commands/application.ts b/frontend/src/__tests__/cypress/cypress/support/commands/application.ts index 2d9b3b5bd4..2dc513c04b 100644 --- a/frontend/src/__tests__/cypress/cypress/support/commands/application.ts +++ b/frontend/src/__tests__/cypress/cypress/support/commands/application.ts @@ -1,6 +1,5 @@ import { MatcherOptions } from '@testing-library/cypress'; import { Matcher, MatcherOptions as DTLMatcherOptions } from '@testing-library/dom'; - /* eslint-disable @typescript-eslint/no-namespace */ declare global { namespace Cypress { @@ -27,6 +26,12 @@ declare global { */ findDropdownItem(name: string | RegExp): Cypress.Chainable; + /** + * Finds a patternfly dropdown item by data-testid, first opening the dropdown if not already opened. + * + * @param testId the name of the item + */ + findDropdownItemByTestId(testId: string): Cypress.Chainable; /** * Finds a patternfly select option by first opening the select menu if not already opened. * @@ -115,7 +120,17 @@ Cypress.Commands.add('findDropdownItem', { prevSubject: 'element' }, (subject, n if ($el.find('[aria-expanded=false]').addBack().length) { cy.wrap($el).click(); } - return cy.wrap($el).findByRole('menuitem', { name }); + return cy.wrap($el).parent().findByRole('menuitem', { name }); + }); +}); + +Cypress.Commands.add('findDropdownItemByTestId', { prevSubject: 'element' }, (subject, testId) => { + Cypress.log({ displayName: 'findDropdownItemByTestId', message: testId }); + return cy.wrap(subject).then(($el) => { + if ($el.find('[aria-expanded=false]').addBack().length) { + cy.wrap($el).click(); + } + return cy.wrap($el).parent().findByTestId(testId); }); }); diff --git a/frontend/src/components/SimpleDropdownSelect.tsx b/frontend/src/components/SimpleDropdownSelect.tsx index ccb06b8280..b812da152d 100644 --- a/frontend/src/components/SimpleDropdownSelect.tsx +++ b/frontend/src/components/SimpleDropdownSelect.tsx @@ -1,6 +1,5 @@ import * as React from 'react'; -import { Truncate } from '@patternfly/react-core'; -import { Dropdown, DropdownItem, DropdownToggle } from '@patternfly/react-core/deprecated'; +import { Truncate, Dropdown, MenuToggle, DropdownList, DropdownItem } from '@patternfly/react-core'; import './SimpleDropdownSelect.scss'; export type SimpleDropdownOption = { @@ -19,6 +18,7 @@ type SimpleDropdownProps = { isFullWidth?: boolean; isDisabled?: boolean; icon?: React.ReactNode; + dataTestId?: string; } & Omit, 'isOpen' | 'toggle' | 'dropdownItems' | 'onChange'>; const SimpleDropdownSelect: React.FC = ({ @@ -29,6 +29,7 @@ const SimpleDropdownSelect: React.FC = ({ value, isFullWidth, icon, + dataTestId, ...props }) => { const [open, setOpen] = React.useState(false); @@ -39,32 +40,41 @@ const SimpleDropdownSelect: React.FC = ({ setOpen(false)} + onOpenChange={(isOpen: boolean) => setOpen(isOpen)} + toggle={(toggleRef) => ( + setOpen(!open)} icon={icon} + isFullWidth={isFullWidth} + onClick={() => setOpen(!open)} + isExpanded={open} > - - } - dropdownItems={[...options] - .sort((a, b) => (a.isPlaceholder === b.isPlaceholder ? 0 : a.isPlaceholder ? -1 : 1)) - .map(({ key, dropdownLabel, label, description, isPlaceholder }) => ( - { - onChange(key, !!isPlaceholder); - setOpen(false); - }} - > - {dropdownLabel ?? label} - - ))} - /> + + )} + shouldFocusToggleOnSelect + > + + {[...options] + .sort((a, b) => (a.isPlaceholder === b.isPlaceholder ? 0 : a.isPlaceholder ? -1 : 1)) + .map(({ key, dropdownLabel, label, description, isPlaceholder }) => ( + { + onChange(key, !!isPlaceholder); + setOpen(false); + }} + > + {dropdownLabel ?? label} + + ))} + + ); }; diff --git a/frontend/src/concepts/dashboard/DashboardSearchField.tsx b/frontend/src/concepts/dashboard/DashboardSearchField.tsx index 70e200c223..18d45e1da6 100644 --- a/frontend/src/concepts/dashboard/DashboardSearchField.tsx +++ b/frontend/src/concepts/dashboard/DashboardSearchField.tsx @@ -41,7 +41,7 @@ const DashboardSearchField: React.FC = ({ ({ key, label: key, diff --git a/frontend/src/concepts/pipelines/content/compareRuns/CompareRunTableToolbar.tsx b/frontend/src/concepts/pipelines/content/compareRuns/CompareRunTableToolbar.tsx index e6869c129a..fa617c215a 100644 --- a/frontend/src/concepts/pipelines/content/compareRuns/CompareRunTableToolbar.tsx +++ b/frontend/src/concepts/pipelines/content/compareRuns/CompareRunTableToolbar.tsx @@ -91,7 +91,7 @@ const CompareRunTableToolbar: React.FC = ({ ...toolbarProps }) => { label: v, }))} onChange={(v) => onChange(v)} - data-testid="runtime-status-dropdown" + dataTestId="runtime-status-dropdown" /> ), }} diff --git a/frontend/src/concepts/pipelines/content/createRun/contentSections/TriggerTypeField.tsx b/frontend/src/concepts/pipelines/content/createRun/contentSections/TriggerTypeField.tsx index 39e91838bb..4078328a03 100644 --- a/frontend/src/concepts/pipelines/content/createRun/contentSections/TriggerTypeField.tsx +++ b/frontend/src/concepts/pipelines/content/createRun/contentSections/TriggerTypeField.tsx @@ -89,7 +89,7 @@ const TriggerTypeField: React.FC = ({ data, onChange }) = = ( {!showSearchbar && ( ({ key: podStepState.stepName, diff --git a/frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableToolbar.tsx b/frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableToolbar.tsx index bb2f56f7fe..6d70fd9152 100644 --- a/frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableToolbar.tsx +++ b/frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableToolbar.tsx @@ -95,7 +95,7 @@ const PipelineRunTableToolbar: React.FC = ({ label: v, }))} onChange={(v) => onChange(v)} - data-testid="runtime-status-dropdown" + dataTestId="runtime-status-dropdown" /> ), }} diff --git a/frontend/src/pages/acceleratorProfiles/screens/manage/tolerations/TolerationFields.tsx b/frontend/src/pages/acceleratorProfiles/screens/manage/tolerations/TolerationFields.tsx index 35b302a734..47b0012a07 100644 --- a/frontend/src/pages/acceleratorProfiles/screens/manage/tolerations/TolerationFields.tsx +++ b/frontend/src/pages/acceleratorProfiles/screens/manage/tolerations/TolerationFields.tsx @@ -41,7 +41,7 @@ const TolerationFields: React.FC = ({ toleration, onUpdat options={operatorDropdownOptions} value={toleration.operator || ''} onChange={(key) => handleFieldUpdate('operator', key)} - data-testid="toleration-operator-select" + dataTestId="toleration-operator-select" /> @@ -51,7 +51,7 @@ const TolerationFields: React.FC = ({ toleration, onUpdat options={effectDropdownOptions} value={toleration.effect || ''} onChange={(key) => handleFieldUpdate('effect', key)} - data-testid="toleration-effect-select" + dataTestId="toleration-effect-select" /> diff --git a/frontend/src/pages/modelServing/customServingRuntimes/CustomServingRuntimeAPIProtocolSelector.tsx b/frontend/src/pages/modelServing/customServingRuntimes/CustomServingRuntimeAPIProtocolSelector.tsx index 447a192f64..2b9d485279 100644 --- a/frontend/src/pages/modelServing/customServingRuntimes/CustomServingRuntimeAPIProtocolSelector.tsx +++ b/frontend/src/pages/modelServing/customServingRuntimes/CustomServingRuntimeAPIProtocolSelector.tsx @@ -44,7 +44,7 @@ const CustomServingRuntimeAPIProtocolSelector: React.FC< isRequired > = ({ return (