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

Improve Catalog Performance #1367

Merged
merged 1 commit into from
Sep 3, 2024
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
20 changes: 20 additions & 0 deletions packages/ui-tests/stories/catalog/PropertiesModal.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,34 @@
import { ITile, PropertiesModal } from '@kaoto/kaoto';
import {
CatalogLoaderProvider,
CatalogSchemaLoader,
CatalogTilesProvider,
RuntimeProvider,
SchemasLoaderProvider,
} from '@kaoto/kaoto/testing';
import { Meta, StoryFn } from '@storybook/react';
import { useState } from 'react';
import aggregate from '../../cypress/fixtures/aggregate.json';
import cronSource from '../../cypress/fixtures/cronSource.json';
import activeMq from '../../cypress/fixtures/activeMq.json';
import box from '../../cypress/fixtures/box.json';

const ContextDecorator = (Story: StoryFn) => (
<RuntimeProvider catalogUrl={CatalogSchemaLoader.DEFAULT_CATALOG_PATH}>
<SchemasLoaderProvider>
<CatalogLoaderProvider>
<CatalogTilesProvider>
<Story />
</CatalogTilesProvider>
</CatalogLoaderProvider>
</SchemasLoaderProvider>
</RuntimeProvider>
);

export default {
title: 'Components/PropertiesModal',
component: PropertiesModal,
decorators: [ContextDecorator],
} as Meta<typeof PropertiesModal>;

const Template: StoryFn<typeof PropertiesModal> = (args) => {
Expand Down
15 changes: 15 additions & 0 deletions packages/ui/src/camel-utils/camel-to-tabs.adapter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,11 @@ describe('camelComponentToTab', () => {

expect(tab).toHaveLength(2);
});

it('should return empty tab when component definition is undefined', () => {
const tab = transformCamelComponentIntoTab(undefined);
expect(tab).toHaveLength(0);
});
});

describe('camelProcessorToTab', () => {
Expand Down Expand Up @@ -187,6 +192,11 @@ describe('camelProcessorToTab', () => {
const tab = transformCamelProcessorComponentIntoTab(processDef);
expect(tab).toHaveLength(0);
});

it('should return empty tab when processor definition is undefined', () => {
const tab = transformCamelProcessorComponentIntoTab(undefined);
expect(tab).toHaveLength(0);
});
});

describe('kameletToTab', () => {
Expand Down Expand Up @@ -236,4 +246,9 @@ describe('kameletToTab', () => {
const tab = transformKameletComponentIntoTab(kameletDef);
expect(tab).toHaveLength(0);
});

it('should return empty tab when kamelet definition is undefined', () => {
const tab = transformKameletComponentIntoTab(undefined);
expect(tab).toHaveLength(0);
});
});
18 changes: 14 additions & 4 deletions packages/ui/src/camel-utils/camel-to-tabs.adapter.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { IPropertiesTab, IPropertiesTable } from '../components/PropertiesModal';
import { ICamelComponentDefinition, ICamelProcessorDefinition, IKameletDefinition } from '../models';
import { isDefined } from '../utils';
import {
IPropertiesTableFilter,
camelComponentApisToTable,
Expand Down Expand Up @@ -46,9 +47,12 @@ const transformPropertiesIntoTab = <K, T>(
};
};

export const transformCamelComponentIntoTab = (componentDef: ICamelComponentDefinition): IPropertiesTab[] => {
const finalTabs: IPropertiesTab[] = [];
export const transformCamelComponentIntoTab = (
componentDef: ICamelComponentDefinition | undefined,
): IPropertiesTab[] => {
if (!isDefined(componentDef)) return [];

const finalTabs: IPropertiesTab[] = [];
let tab = transformPropertiesIntoTab(
[
{
Expand Down Expand Up @@ -116,7 +120,11 @@ export const transformCamelComponentIntoTab = (componentDef: ICamelComponentDefi
return finalTabs;
};

export const transformCamelProcessorComponentIntoTab = (processorDef: ICamelProcessorDefinition): IPropertiesTab[] => {
export const transformCamelProcessorComponentIntoTab = (
processorDef: ICamelProcessorDefinition | undefined,
): IPropertiesTab[] => {
if (!isDefined(processorDef)) return [];

const finalTabs: IPropertiesTab[] = [];
const tab = transformPropertiesIntoTab(
[
Expand All @@ -131,7 +139,9 @@ export const transformCamelProcessorComponentIntoTab = (processorDef: ICamelProc
return finalTabs;
};

export const transformKameletComponentIntoTab = (kameletDef: IKameletDefinition): IPropertiesTab[] => {
export const transformKameletComponentIntoTab = (kameletDef: IKameletDefinition | undefined): IPropertiesTab[] => {
if (!isDefined(kameletDef)) return [];

const finalTabs: IPropertiesTab[] = [];
const tab = transformPropertiesIntoTab(
[
Expand Down
3 changes: 0 additions & 3 deletions packages/ui/src/camel-utils/camel-to-tile.adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export const camelComponentToTile = (componentDef: ICamelComponentDefinition): I
tags,
provider,
version,
rawObject: componentDef,
Copy link
Member

Choose a reason for hiding this comment

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

Great 🚀

};
};

Expand All @@ -45,7 +44,6 @@ export const camelProcessorToTile = (processorDef: ICamelProcessorDefinition): I
description,
headerTags: ['Processor'],
tags,
rawObject: processorDef,
};
};

Expand Down Expand Up @@ -73,6 +71,5 @@ export const kameletToTile = (kameletDef: IKameletDefinition): ITile => {
headerTags,
tags,
version,
rawObject: kameletDef,
};
};
108 changes: 108 additions & 0 deletions packages/ui/src/components/Catalog/BaseCatalog.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import { act, fireEvent, render, screen } from '@testing-library/react';
import { CatalogLayout } from './Catalog.models';
import { BaseCatalog } from './BaseCatalog';
import { longTileList } from '../../stubs';

describe('BaseCatalog', () => {
it('renders correctly with Gallery Layout', () => {
const { container } = render(
<BaseCatalog
className="catalog__base"
tiles={longTileList}
catalogLayout={CatalogLayout.Gallery}
onTagClick={jest.fn()}
/>,
);

expect(container).toMatchSnapshot();
});

it('renders correctly with List Layout', () => {
const { container } = render(
<BaseCatalog
className="catalog__base"
tiles={longTileList}
catalogLayout={CatalogLayout.List}
onTagClick={jest.fn()}
/>,
);

expect(container).toMatchSnapshot();
});

it('Render BaseCatalog with 60 tiles, 2 pages with 50 tiles on the 1st page and 10 tiles on the 2nd page', async () => {
render(
<BaseCatalog
className="catalog__base"
tiles={longTileList}
catalogLayout={CatalogLayout.List}
onTagClick={jest.fn()}
/>,
);

expect(screen.getByRole('spinbutton', { name: 'Current page' })).toHaveValue(1);

expect(
screen.getAllByRole('listitem').filter((li) => li.classList.contains('catalog-data-list-item')),
).toHaveLength(50);

const nextPageButton = screen.getByRole('button', { name: 'Go to next page' });
act(() => {
fireEvent.click(nextPageButton);
});
expect(screen.getByRole('spinbutton', { name: 'Current page' })).toHaveValue(2);

expect(
screen.getAllByRole('listitem').filter((li) => li.classList.contains('catalog-data-list-item')),
).toHaveLength(10);

expect(screen.getByRole('button', { name: 'Go to next page' })).toBeDisabled();
});

it('Render BaseCatalog with 60 tiles, change per page setting to 20', async () => {
render(
<BaseCatalog
className="catalog__base"
tiles={longTileList}
catalogLayout={CatalogLayout.List}
onTagClick={jest.fn()}
/>,
);

expect(screen.getByRole('spinbutton', { name: 'Current page' })).toHaveValue(1);

const pageSetting = screen.getAllByRole('button').filter((btn) => btn.id === 'catalog-pagination-top-toggle');
act(() => {
fireEvent.click(pageSetting[0]);
});

act(() => {
fireEvent.click(screen.getByText('20 per page'));
});

expect(
screen.getAllByRole('listitem').filter((li) => li.classList.contains('catalog-data-list-item')),
).toHaveLength(20);

const nextPageButton = screen.getByRole('button', { name: 'Go to next page' });
act(() => {
fireEvent.click(nextPageButton);
});
expect(screen.getByRole('spinbutton', { name: 'Current page' })).toHaveValue(2);

expect(
screen.getAllByRole('listitem').filter((li) => li.classList.contains('catalog-data-list-item')),
).toHaveLength(20);

act(() => {
fireEvent.click(nextPageButton);
});
expect(screen.getByRole('spinbutton', { name: 'Current page' })).toHaveValue(3);

expect(
screen.getAllByRole('listitem').filter((li) => li.classList.contains('catalog-data-list-item')),
).toHaveLength(20);

expect(screen.getByRole('button', { name: 'Go to next page' })).toBeDisabled();
});
});
52 changes: 42 additions & 10 deletions packages/ui/src/components/Catalog/BaseCatalog.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { DataList, Gallery, Title } from '@patternfly/react-core';
import { FunctionComponent, useCallback, useEffect, useRef } from 'react';
import { DataList, Gallery, Pagination } from '@patternfly/react-core';
import { FunctionComponent, useCallback, useEffect, useRef, useState } from 'react';
import './BaseCatalog.scss';
import { CatalogLayout, ITile } from './Catalog.models';
import { CatalogDataListItem } from './DataListItem';
Expand All @@ -15,13 +15,28 @@

export const BaseCatalog: FunctionComponent<BaseCatalogProps> = (props) => {
const catalogBodyRef = useRef<HTMLDivElement>(null);
const itemCount = props.tiles?.length;
const [page, setPage] = useState(1);
const [perPage, setPerPage] = useState(50);

const onTileClick = useCallback(
(tile: ITile) => {
props.onTileClick?.(tile);
},
[props],
);

const startIndex = (page - 1) * perPage < 0 ? 0 : (page - 1) * perPage;
const endIndex = page * perPage;
useEffect(() => {
// Handling the scenario where the item count is less the page selected.
if (startIndex + 1 > itemCount) {
setPage(Math.ceil(itemCount / perPage));

Check warning on line 34 in packages/ui/src/components/Catalog/BaseCatalog.tsx

View check run for this annotation

Codecov / codecov/patch

packages/ui/src/components/Catalog/BaseCatalog.tsx#L34

Added line #L34 was not covered by tests
} else if (page === 0 && itemCount > 0) setPage(1);

catalogBodyRef.current!.scrollTop = 0;
}, [props.tiles]);

const onSelectDataListItem = useCallback(
(_event: React.MouseEvent | React.KeyboardEvent, id: string) => {
const tile = props.tiles.find((tile) => tile.name + '-' + tile.type === id);
Expand All @@ -30,26 +45,43 @@
[onTileClick, props.tiles],
);

useEffect(() => {
catalogBodyRef.current!.scrollTop = 0;
}, [props.tiles]);
const onSetPage = (_event: React.MouseEvent | React.KeyboardEvent | MouseEvent, newPage: number) => {
setPage(newPage);
};

const onPerPageSelect = (
_event: React.MouseEvent | React.KeyboardEvent | MouseEvent,
newPerPage: number,
newPage: number,
) => {
setPerPage(newPerPage);
setPage(newPage);
};

const paginatedCards = props.tiles?.slice(startIndex, endIndex);

return (
<>
<Title headingLevel="h2" size="xl">
Showing {props.tiles?.length} elements
</Title>
<Pagination
itemCount={itemCount}
perPage={perPage}
page={page}
onSetPage={onSetPage}
widgetId="catalog-pagination"
onPerPageSelect={onPerPageSelect}
ouiaId="CatalogPagination"
/>
<div id="catalog-list" className="catalog-list" ref={catalogBodyRef}>
{props.catalogLayout == CatalogLayout.List && (
<DataList aria-label="Catalog list" onSelectDataListItem={onSelectDataListItem} isCompact>
{props.tiles?.map((tile) => (
{paginatedCards.map((tile) => (
<CatalogDataListItem key={`${tile.name}-${tile.type}`} tile={tile} onTagClick={props.onTagClick} />
))}
</DataList>
)}
{props.catalogLayout == CatalogLayout.Gallery && (
<Gallery hasGutter>
{props.tiles?.map((tile) => (
{paginatedCards.map((tile) => (
<Tile key={`${tile.name}-${tile.type}`} tile={tile} onClick={onTileClick} onTagClick={props.onTagClick} />
))}
</Gallery>
Expand Down
2 changes: 0 additions & 2 deletions packages/ui/src/components/Catalog/Catalog.models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ export interface ITile {
tags: string[];
version?: string;
provider?: string;
/** @deprecated Please relay on name property instead */
rawObject?: unknown;
}

export type TileFilter = (item: ITile) => boolean;
Expand Down
1 change: 0 additions & 1 deletion packages/ui/src/components/Catalog/DataListItem.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ describe('DataListItem', () => {
tags: ['tag1', 'tag2'],
headerTags: ['header-tag1', 'header-tag2'],
version: '1.0',
rawObject: {},
};

it('renders correctly', () => {
Expand Down
1 change: 0 additions & 1 deletion packages/ui/src/components/Catalog/Tile.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ describe('Tile', () => {
description: 'tile-description',
tags: ['tag1', 'tag2'],
headerTags: ['header-tag1', 'header-tag2'],
rawObject: {},
};

it('renders correctly', () => {
Expand Down
Loading
Loading