Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes Image selection dropdown scroll issue #2696

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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',
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
7 changes: 5 additions & 2 deletions frontend/src/__tests__/cypress/cypress/pages/workbench.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -27,6 +26,12 @@ declare global {
*/
findDropdownItem(name: string | RegExp): Cypress.Chainable<JQuery>;

/**
* 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<JQuery>;
/**
* Finds a patternfly select option by first opening the select menu if not already opened.
*
Expand Down Expand Up @@ -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);
});
});

Expand Down
58 changes: 34 additions & 24 deletions frontend/src/components/SimpleDropdownSelect.tsx
Original file line number Diff line number Diff line change
@@ -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 = {
Expand All @@ -19,6 +18,7 @@ type SimpleDropdownProps = {
isFullWidth?: boolean;
isDisabled?: boolean;
icon?: React.ReactNode;
dataTestId?: string;
} & Omit<React.ComponentProps<typeof Dropdown>, 'isOpen' | 'toggle' | 'dropdownItems' | 'onChange'>;

const SimpleDropdownSelect: React.FC<SimpleDropdownProps> = ({
Expand All @@ -29,6 +29,7 @@ const SimpleDropdownSelect: React.FC<SimpleDropdownProps> = ({
value,
isFullWidth,
icon,
dataTestId,
...props
}) => {
const [open, setOpen] = React.useState(false);
Expand All @@ -39,32 +40,41 @@ const SimpleDropdownSelect: React.FC<SimpleDropdownProps> = ({
<Dropdown
{...props}
isOpen={open}
className={isFullWidth ? 'full-width' : undefined}
toggle={
<DropdownToggle
onSelect={() => setOpen(false)}
onOpenChange={(isOpen: boolean) => setOpen(isOpen)}
toggle={(toggleRef) => (
<MenuToggle
data-testid={dataTestId}
ref={toggleRef}
isDisabled={isDisabled}
className={isFullWidth ? 'full-width' : undefined}
onToggle={() => setOpen(!open)}
icon={icon}
isFullWidth={isFullWidth}
onClick={() => setOpen(!open)}
isExpanded={open}
>
<Truncate content={selectedLabel} className="truncate-no-min-width" />
</DropdownToggle>
}
dropdownItems={[...options]
.sort((a, b) => (a.isPlaceholder === b.isPlaceholder ? 0 : a.isPlaceholder ? -1 : 1))
.map(({ key, dropdownLabel, label, description, isPlaceholder }) => (
<DropdownItem
key={key}
description={description}
onClick={() => {
onChange(key, !!isPlaceholder);
setOpen(false);
}}
>
{dropdownLabel ?? label}
</DropdownItem>
))}
/>
</MenuToggle>
)}
shouldFocusToggleOnSelect
>
<DropdownList>
{[...options]
.sort((a, b) => (a.isPlaceholder === b.isPlaceholder ? 0 : a.isPlaceholder ? -1 : 1))
.map(({ key, dropdownLabel, label, description, isPlaceholder }) => (
<DropdownItem
data-testid={`dropdown-item ${key}`}
key={key}
description={description}
onClick={() => {
onChange(key, !!isPlaceholder);
setOpen(false);
}}
>
{dropdownLabel ?? label}
</DropdownItem>
))}
</DropdownList>
</Dropdown>
);
};

Expand Down
2 changes: 1 addition & 1 deletion frontend/src/concepts/dashboard/DashboardSearchField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const DashboardSearchField: React.FC<DashboardSearchFieldProps> = ({
<InputGroupItem>
<SimpleDropdownSelect
aria-label="Filter type"
data-testid="filter-dropdown-select"
dataTestId="filter-dropdown-select"
options={types.map((key) => ({
key,
label: key,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ const CompareRunTableToolbar: React.FC<FilterProps> = ({ ...toolbarProps }) => {
label: v,
}))}
onChange={(v) => onChange(v)}
data-testid="runtime-status-dropdown"
dataTestId="runtime-status-dropdown"
/>
),
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ const TriggerTypeField: React.FC<TriggerTypeFieldProps> = ({ data, onChange }) =
<StackItem>
<FormGroup label="Trigger type">
<SimpleDropdownSelect
data-testid="triggerTypeSelector"
dataTestId="triggerTypeSelector"
isFullWidth
options={[
{ key: ScheduledType.PERIODIC, label: 'Periodic' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ const LogsTabForPodName: React.FC<{ podName: string; isFailedPod: boolean }> = (
{!showSearchbar && (
<ToolbarItem spacer={{ default: 'spacerSm' }} style={{ maxWidth: '200px' }}>
<SimpleDropdownSelect
data-testid="logs-step-select"
dataTestId="logs-step-select"
isDisabled={podStepStates.length <= 1}
options={podStepStates.map((podStepState) => ({
key: podStepState.stepName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ const PipelineRunTableToolbar: React.FC<PipelineRunTableToolbarProps> = ({
label: v,
}))}
onChange={(v) => onChange(v)}
data-testid="runtime-status-dropdown"
dataTestId="runtime-status-dropdown"
/>
),
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const TolerationFields: React.FC<TolerationFieldsProps> = ({ toleration, onUpdat
options={operatorDropdownOptions}
value={toleration.operator || ''}
onChange={(key) => handleFieldUpdate('operator', key)}
data-testid="toleration-operator-select"
dataTestId="toleration-operator-select"
/>
</FormGroup>

Expand All @@ -51,7 +51,7 @@ const TolerationFields: React.FC<TolerationFieldsProps> = ({ toleration, onUpdat
options={effectDropdownOptions}
value={toleration.effect || ''}
onChange={(key) => handleFieldUpdate('effect', key)}
data-testid="toleration-effect-select"
dataTestId="toleration-effect-select"
/>
</FormGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const CustomServingRuntimeAPIProtocolSelector: React.FC<
isRequired
>
<SimpleDropdownSelect
data-testid="custom-serving-api-protocol-selection"
dataTestId="custom-serving-api-protocol-selection"
aria-label="Select a model serving api protocol"
placeholder="Select a value"
isDisabled={isOnlyModelMesh}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const CustomServingRuntimePlatformsSelector: React.FC<
isRequired
>
<SimpleDropdownSelect
data-testid="custom-serving-runtime-selection"
dataTestId="custom-serving-runtime-selection"
aria-label="Select a model serving runtime platform"
placeholder="Select a value"
options={options}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ const ServingRuntimeTemplateSection: React.FC<ServingRuntimeTemplateSectionProps
isFullWidth
isDisabled={isEditing || filteredTemplates.length === 0}
id="serving-runtime-template-selection"
dataTestId="serving-runtime-template-selection"
aria-label="Select a template"
options={options}
placeholder={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,10 @@ const ImageStreamSelector: React.FC<ImageStreamSelectorProps> = ({
return (
<FormGroup isRequired label="Image selection" fieldId="workbench-image-stream-selection">
<SimpleDropdownSelect
isScrollable
isFullWidth
id="workbench-image-stream-selection"
data-testid="workbench-image-stream-selection"
dataTestId="workbench-image-stream-selection"
aria-label="Select an image"
options={options}
placeholder="Select one"
Expand Down
Loading