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

Projects list view: expand workbenches column to a table #3207

Merged
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
26 changes: 20 additions & 6 deletions frontend/src/__tests__/cypress/cypress/pages/projects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,35 @@ class NotebookRow extends TableRow {
}
}

class ProjectNotebookRow extends TableRow {
findNotebookRouteLink() {
return this.find().findByTestId('notebook-route-link');
}

findNotebookStatusText() {
return this.find().findByTestId('notebook-status-text');
}
}

class ProjectRow extends TableRow {
findDescription() {
return this.find().findByTestId('table-row-title-description');
}

findEnableSwitch() {
return this.find().pfSwitch('notebook-status-switch');
findNotebookColumn() {
return this.find().findByTestId('notebook-column-expand');
}

findNotebookRouteLink() {
return this.find().findByTestId('notebook-route-link');
findNotebookTable() {
return this.find().parents('tbody').findByTestId('project-notebooks-table');
}

findNotebookStatusText() {
return this.find().findByTestId('notebook-status-text');
getNotebookRow(notebookName: string) {
return new ProjectNotebookRow(() => this.findNotebookLink(notebookName).parents('tr'));
}

findNotebookLink(notebookName: string) {
return this.findNotebookTable().findByRole('link', { name: notebookName });
}
}

Expand Down
4 changes: 0 additions & 4 deletions frontend/src/__tests__/cypress/cypress/pages/workbench.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,6 @@ class NotebookRow extends TableRow {
.findByTestId('add-storage-button');
}

findEnableSwitch() {
return this.find().pfSwitch('notebook-status-switch');
}

shouldHaveContainerSize(name: string) {
this.find().find(`[data-label="Container size"]`).contains(name).should('exist');
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,6 @@ describe('Data science projects details', () => {
cy.url().should('include', '/projects/test-project');
});

it('should test url for workbench creation', () => {
initIntercepts();
projectListPage.visit();
projectListPage.findCreateWorkbenchButton().click();

cy.url().should('include', '/projects/test-project/spawner');
});

it('should list the new project', () => {
initIntercepts();
projectListPage.visit();
Expand Down Expand Up @@ -218,41 +210,81 @@ describe('Data science projects details', () => {
deleteProject.should('have.attr', 'aria-disabled', 'true');
});

describe('Table filter', () => {
it('filter by name', () => {
initIntercepts();
projectListPage.visit();

// Select the "Name" filter
const projectListToolbar = projectListPage.getTableToolbar();
projectListToolbar.findFilterMenuOption('filter-toolbar-dropdown', 'Name').click();
projectListToolbar.findFilterInput('name').type('Test Project');
// Verify only rows with the typed run name exist
projectListPage.getProjectRow('Test Project').find().should('exist');
});
it('should filter by name', () => {
initIntercepts();
projectListPage.visit();

it('filter by user', () => {
initIntercepts();
projectListPage.visit();
// Select the "Name" filter
const projectListToolbar = projectListPage.getTableToolbar();
projectListToolbar.findFilterMenuOption('filter-toolbar-dropdown', 'Name').click();
projectListToolbar.findFilterInput('name').type('Test Project');
// Verify only rows with the typed run name exist
projectListPage.getProjectRow('Test Project').find().should('exist');
});

// Select the "User" filter
const projectListToolbar = projectListPage.getTableToolbar();
projectListToolbar.findFilterMenuOption('filter-toolbar-dropdown', 'User').click();
projectListToolbar.findFilterInput('user').type('test-user');
// Verify only rows with the typed run user exist
projectListPage.getProjectRow('Test Project').find().should('exist');
});
it('should filter by user', () => {
initIntercepts();
projectListPage.visit();

// Select the "User" filter
const projectListToolbar = projectListPage.getTableToolbar();
projectListToolbar.findFilterMenuOption('filter-toolbar-dropdown', 'User').click();
projectListToolbar.findFilterInput('user').type('test-user');
// Verify only rows with the typed run user exist
projectListPage.getProjectRow('Test Project').find().should('exist');
});

it('Validate that clicking on switch toggle will open modal to stop workbench', () => {
it('should show list of workbenches when the column is expanded', () => {
cy.interceptK8sList(ProjectModel, mockK8sResourceList([mockProjectK8sResource({})]));
cy.interceptK8s(RouteModel, mockRouteK8sResource({ notebookName: 'test-notebook' })).as(
'getWorkbench',
);
cy.interceptK8sList(
{ model: NotebookModel },
mockK8sResourceList([
mockNotebookK8sResource({
opts: {
spec: {
template: {
spec: {
containers: [
{
name: 'test-notebook',
image: 'test-image:latest',
},
],
},
},
},
metadata: {
name: 'test-notebook',
namespace: 'test-project',
labels: {
'opendatahub.io/notebook-image': 'true',
},
annotations: {
'opendatahub.io/image-display-name': 'Test image',
},
},
},
}),
]),
);
projectListPage.visit();
const projectTableRow = projectListPage.getProjectRow('Test Project');
projectTableRow.findNotebookColumn().click();
cy.wait('@getWorkbench');
});

it('should open the modal to stop workbench when user stops the workbench', () => {
cy.interceptK8sList(ProjectModel, mockK8sResourceList([mockProjectK8sResource({})]));
cy.interceptK8s('PATCH', NotebookModel, mockNotebookK8sResource({})).as('stopWorkbench');
cy.interceptK8sList(PodModel, mockK8sResourceList([mockPodK8sResource({})]));
cy.interceptK8s(RouteModel, mockRouteK8sResource({ notebookName: 'test-notebook' })).as(
'getWorkbench',
);
cy.interceptK8sList(
{ model: NotebookModel, ns: 'test-project' },
NotebookModel,
mockK8sResourceList([
mockNotebookK8sResource({
opts: {
Expand All @@ -270,6 +302,7 @@ describe('Data science projects details', () => {
},
metadata: {
name: 'test-notebook',
namespace: 'test-project',
labels: {
'opendatahub.io/notebook-image': 'true',
},
Expand All @@ -282,9 +315,14 @@ describe('Data science projects details', () => {
]),
);
projectListPage.visit();
cy.wait('@getWorkbench');
const projectTableRow = projectListPage.getProjectRow('Test Project');
projectTableRow.findEnableSwitch().click();
projectTableRow.findNotebookColumn().click();

const notebookRow = projectTableRow.getNotebookRow('Test Notebook');
notebookRow.findNotebookRouteLink().should('have.attr', 'aria-disabled', 'false');

notebookRow.findKebabAction('Start').should('be.disabled');
notebookRow.findKebabAction('Stop').click();

//stop workbench
notebookConfirmModal.findStopWorkbenchButton().should('be.enabled');
Expand Down Expand Up @@ -315,8 +353,8 @@ describe('Data science projects details', () => {
},
]);
});
projectTableRow.findNotebookStatusText().should('have.text', 'Stopped ');
projectTableRow.findNotebookRouteLink().should('have.attr', 'aria-disabled', 'true');
notebookRow.findNotebookStatusText().should('have.text', 'Stopped');
notebookRow.findNotebookRouteLink().should('have.attr', 'aria-disabled', 'true');
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ describe('Workbench page', () => {
const notebookRow = workbenchPage.getNotebookRow('Test Notebook');
notebookRow.shouldHaveNotebookImageName('Test Image');
notebookRow.shouldHaveContainerSize('Small');
notebookRow.findHaveNotebookStatusText().should('have.text', 'Running ');
notebookRow.findHaveNotebookStatusText().should('have.text', 'Running');
notebookRow.findNotebookRouteLink().should('have.attr', 'aria-disabled', 'false');

//Name sorting
Expand Down Expand Up @@ -451,7 +451,7 @@ describe('Workbench page', () => {
const notebookRow = workbenchPage.getNotebookRow('Test Notebook');

//stop Workbench
notebookRow.findEnableSwitch().click();
notebookRow.findKebabAction('Stop').click();
notebookConfirmModal.findStopWorkbenchButton().should('be.enabled');
cy.interceptK8s(
NotebookModel,
Expand Down Expand Up @@ -480,7 +480,7 @@ describe('Workbench page', () => {
},
]);
});
notebookRow.findHaveNotebookStatusText().should('have.text', 'Stopped ');
notebookRow.findHaveNotebookStatusText().should('have.text', 'Stopped');
notebookRow.findNotebookRouteLink().should('have.attr', 'aria-disabled', 'true');

cy.interceptK8s('PATCH', NotebookModel, mockNotebookK8sResource({})).as('startWorkbench');
Expand All @@ -501,8 +501,8 @@ describe('Workbench page', () => {
}),
);

notebookRow.findEnableSwitch().click();
notebookRow.findHaveNotebookStatusText().should('have.text', 'Starting... ');
notebookRow.findKebabAction('Start').click();
notebookRow.findHaveNotebookStatusText().should('have.text', 'Starting');
notebookRow.findHaveNotebookStatusText().click();

cy.wait('@startWorkbench').then((interception) => {
Expand Down
4 changes: 4 additions & 0 deletions frontend/src/app/App.scss
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ body,
}
}

.odh-u-spin {
animation: pf-v5-c-spinner-animation-rotate 3s linear infinite;
}

// specificity targeting form elements to override --pf-v5-global--FontSize--md
.pf-v5-c-page,
.pf-v5-c-modal-box {
Expand Down
13 changes: 11 additions & 2 deletions frontend/src/components/table/TableBase.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,16 @@ import {
Td,
TbodyProps,
InnerScrollContainer,
TrProps,
} from '@patternfly/react-table';
import { EitherNotBoth } from '~/typeHelpers';
import { GetColumnSort, SortableData } from './types';
import { CHECKBOX_FIELD_ID, EXPAND_FIELD_ID, KEBAB_FIELD_ID } from './const';

type Props<DataType> = {
loading?: boolean;
skeletonRowCount?: number;
skeletonRowProps?: TrProps;
data: DataType[];
columns: SortableData<DataType>[];
subColumns?: SortableData<DataType>[];
Expand Down Expand Up @@ -111,6 +114,8 @@ const TableBase = <T,>({
getColumnSort,
itemCount = 0,
loading,
skeletonRowCount,
skeletonRowProps = {},
toggleTemplate,
disableItemCount = false,
hasStickyColumns,
Expand Down Expand Up @@ -209,7 +214,7 @@ const TableBase = <T,>({
? // compute the number of items in the upcoming page
new Array(
itemCount === 0
? rowHeightsRef.current?.length || MIN_PAGE_SIZE
? skeletonRowCount || rowHeightsRef.current?.length || MIN_PAGE_SIZE
: Math.max(0, Math.min(perPage, itemCount - perPage * (page - 1))),
)
.fill(undefined)
Expand All @@ -220,7 +225,11 @@ const TableBase = <T,>({
const getRow = () => (
<Tr
key={`skeleton-${i}`}
style={{ height: rowHeightsRef.current?.[i] || rowHeightsRef.current?.[0] }}
{...skeletonRowProps}
style={{
...(skeletonRowProps.style || {}),
height: rowHeightsRef.current?.[i] || rowHeightsRef.current?.[0],
}}
>
{columns.map((col) => (
<Td
Expand Down
13 changes: 13 additions & 0 deletions frontend/src/images/icons/NotebookIcon.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { createIcon } from '@patternfly/react-icons/dist/esm/createIcon';

const NotebookIcon = createIcon({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the mocks, it looks like this icon should be solid. Wondering if we can update the svg path here? WDYT @jeff-phillips-18

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jenny-s51 You're looking at the old mocks, In the updatedmocks (which are ready for dev), we're using the outlined icon, just like Jeff added.

One small difference is that the icon appears slightly smaller than it should be (compared to the mocks).
@jeff-phillips-18 Could you take a look at this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The size in the table is based on the font size (14px) while the mocks show 16px. I have updated the code to set the icons size to 16px. Screen shot above has been updated.

name: 'NotebookIcon',
width: 32,
height: 32,
svgPath:
'M30.0823 5.41458C30.5653 5.50638 30.9696 5.82808 31.1634 6.27538C31.7494 7.62408 32.0335 9.06458 32.0071 10.5558C31.9094 16.0861 27.1594 20.7467 21.6282 20.7467H21.5994C20.6482 20.7438 19.7102 20.6105 18.8025 20.3502L8.79906 30.3536C7.73606 31.4166 6.35086 31.9566 4.98706 31.9566C3.79176 31.9571 2.61256 31.5425 1.67946 30.7022C0.635455 29.7617 0.0402555 28.4727 0.00365549 27.0718C-0.0324445 25.6909 0.507056 24.3389 1.48366 23.3628L11.6492 13.1968C11.3894 12.2906 11.2561 11.3536 11.2527 10.4024C11.2351 4.85888 15.9007 0.0917815 21.4407 -0.00781855C22.9397 -0.0322186 24.3728 0.249481 25.7243 0.836482C26.1711 1.02988 26.4929 1.43418 26.5847 1.91708C26.6784 2.40928 26.5236 2.91418 26.1701 3.26718L21.007 8.43028C20.3601 9.07728 20.2263 10.0885 20.6955 10.7819C21.0124 11.2506 21.489 11.5378 22.0373 11.59C22.5813 11.6422 23.1101 11.4518 23.4934 11.0685L28.7322 5.82918C29.0847 5.47558 29.5911 5.32078 30.0823 5.41458ZM21.6058 18.7466H21.6287C26.094 18.7466 29.9284 14.9849 30.007 10.5205C30.0241 9.57818 29.8873 8.66208 29.6012 7.78858L24.9074 12.4824C24.1003 13.29 22.9856 13.6933 21.8469 13.5805C20.7102 13.4721 19.6868 12.8603 19.0393 11.9023C18.0261 10.4053 18.259 8.35008 19.593 7.01608L24.2107 2.39788C23.3841 2.12738 22.5203 1.99068 21.6336 1.99068C21.5814 1.99068 21.5291 1.99118 21.4769 1.99218C17.0048 2.07228 13.2386 5.92038 13.2528 10.3955C13.2563 11.335 13.4169 12.2578 13.7299 13.1377C13.8588 13.501 13.7674 13.9068 13.4945 14.1797L2.89776 24.7769C2.29866 25.376 1.98076 26.1724 2.00276 27.0196C2.02516 27.8663 2.38556 28.6466 3.01786 29.2159C4.20436 30.2847 6.16386 30.1612 7.38506 28.9395L17.8197 18.5049C18.0922 18.2314 18.4979 18.1401 18.8617 18.2695C19.7435 18.583 20.6663 18.7437 21.6058 18.7466ZM7.5 26.0098C7.5 26.8382 6.82843 27.5098 6 27.5098C5.17157 27.5098 4.5 26.8382 4.5 26.0098C4.5 25.1813 5.17157 24.5098 6 24.5098C6.82843 24.5098 7.5 25.1813 7.5 26.0098Z',
xOffset: 0,
yOffset: 0,
});

export default NotebookIcon;
Loading
Loading