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.

Sign up for GitHub

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

Jump to bottom

Follow up misc fixes for pipeline version feature #2352

Merged
merged 2 commits into from
Jan 18, 2024
Merged

Follow up misc fixes for pipeline version feature #2352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cb232ee
Follow up misc fixes for pipeline version feature
DaoDaoNoCode Jan 17, 2024
2917e3f
address comments
DaoDaoNoCode Jan 18, 2024
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
70 changes: 39 additions & 31 deletions frontend/src/components/table/TableBase.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,39 +123,47 @@ const TableBase = <T,>({
: Math.max(0, Math.min(perPage, itemCount - perPage * (page - 1))),
)
.fill(undefined)
.map((_, i) => (
.map((_, i) => {
// Set the height to the last known row height or otherwise the same height as the first row.
// When going to a previous page, the number of rows may be greater than the current.
<Tr
key={`skeleton-${i}`}
style={{ height: rowHeightsRef.current?.[i] || rowHeightsRef.current?.[0] }}
>
{columns.map((col) => (
<Td
key={col.field}
// assign classes to reserve space
className={
col.field === CHECKBOX_FIELD_ID || col.field === EXPAND_FIELD_ID
? 'pf-c-table__toggle'
: col.field === KEBAB_FIELD_ID
? 'pf-c-table__action'
: undefined
}
>
{
// render placeholders to reserve space
col.field === EXPAND_FIELD_ID || col.field === KEBAB_FIELD_ID ? (
<div style={{ width: 46 }} />
) : col.field === CHECKBOX_FIELD_ID ? (
<div style={{ width: 13 }} />
) : (
<Skeleton width="50%" />
)
}
</Td>
))}
</Tr>
))

const getRow = () => (
<Tr
key={`skeleton-${i}`}
style={{ height: rowHeightsRef.current?.[i] || rowHeightsRef.current?.[0] }}
>
{columns.map((col) => (
<Td
key={col.field}
// assign classes to reserve space
className={
col.field === CHECKBOX_FIELD_ID || col.field === EXPAND_FIELD_ID
? 'pf-c-table__toggle'
: col.field === KEBAB_FIELD_ID
? 'pf-c-table__action'
: undefined
}
>
{
// render placeholders to reserve space
col.field === EXPAND_FIELD_ID || col.field === KEBAB_FIELD_ID ? (
<div style={{ width: 46 }} />
) : col.field === CHECKBOX_FIELD_ID ? (
<div style={{ width: 13 }} />
) : (
<Skeleton width="50%" />
)
}
</Td>
))}
</Tr>
);
return disableRowRenderSupport ? (
<Tbody key={`skeleton-tbody-${i}`}>{getRow()}</Tbody>
) : (
getRow()
);
})
: data.map((row, rowIndex) => rowRenderer(row, rowIndex));

return (
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/table/TableRowTitleDescription.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const TableRowTitleDescription: React.FC<TableRowTitleDescriptionProps> = ({
<Title headingLevel="h3" size="md">
{resource ? <ResourceNameTooltip resource={resource}>{title}</ResourceNameTooltip> : title}
</Title>
{subtitle && <Text>{subtitle}</Text>}
{subtitle}
{descriptionNode}
{label}
</>
Expand Down
37 changes: 20 additions & 17 deletions frontend/src/concepts/pipelines/content/PipelineVersionLink.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,42 +1,45 @@
import React from 'react';
import { Link } from 'react-router-dom';

import { Skeleton, Tooltip } from '@patternfly/react-core';

import usePipelineVersionById from '~/concepts/pipelines/apiHooks/usePipelineVersionById';
import { usePipelinesAPI } from '~/concepts/pipelines/context';
import { ResourceReferenceKF } from '~/concepts/pipelines/kfTypes';
import { NoRunContent } from '~/concepts/pipelines/content/tables/renderUtils';
import { PipelineVersionKF } from '~/concepts/pipelines/kfTypes';

interface PipelineVersionLinkProps {
resourceRef: ResourceReferenceKF | undefined;
displayName?: string;
loadingIndicator?: React.ReactElement;
version?: PipelineVersionKF | null;
error?: Error;
loaded: boolean;
}

export const PipelineVersionLink: React.FC<PipelineVersionLinkProps> = ({
resourceRef,
displayName,
loadingIndicator,
version,
error,
loaded,
}) => {
const { namespace } = usePipelinesAPI();
const versionName = resourceRef?.name;
const versionId = resourceRef?.key.id;
const [version, isVersionLoaded, error] = usePipelineVersionById(versionId);

if (!resourceRef) {
return <NoRunContent />;
if (!loaded && !error) {
return loadingIndicator || <Skeleton />;
}

if (!isVersionLoaded && !error) {
return loadingIndicator || <Skeleton />;
if (error) {
return (
<Tooltip content={error.message} position="right">
<div className="pf-v5-u-disabled-color-100 pf-v5-c-truncate__start">{displayName}</div>
</Tooltip>
);
}

if (!version) {
return (
<Tooltip content={<div>&quot;{versionName}&quot; no longer exists.</div>} position="right">
<div className="pf-v5-u-disabled-color-100 pf-v5-c-truncate__start">{versionName}</div>
<Tooltip content={<div>&quot;{displayName}&quot; no longer exists.</div>} position="right">
<div className="pf-v5-u-disabled-color-100 pf-v5-c-truncate__start">{displayName}</div>
</Tooltip>
);
}

return <Link to={`/pipelines/${namespace}/pipeline/view/${versionId}`}>{versionName}</Link>;
return <Link to={`/pipelines/${namespace}/pipeline/view/${version.id}`}>{version.name}</Link>;
};
5 changes: 3 additions & 2 deletions frontend/src/concepts/pipelines/content/pipelineSelector/PipelineSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { pipelineSelectorColumns } from '~/concepts/pipelines/content/pipelineSe
import PipelineViewMoreFooterRow from '~/concepts/pipelines/content/tables/PipelineViewMoreFooterRow';
import { useSelectorSearch } from '~/concepts/pipelines/content/pipelineSelector/utils';
import EmptyTableView from '~/concepts/pipelines/content/tables/EmptyTableView';
import { getTableSortProps } from '~/concepts/pipelines/content/tables/usePipelineTable';

type PipelineSelectorProps = {
selection?: string;
Expand All @@ -37,6 +38,7 @@ const PipelineSelector: React.FC<PipelineSelectorProps> = ({ selection, onSelect
[{ items: initialData, totalSize: fetchedSize, nextPageToken: initialPageToken }, loaded],
{ initialLoaded, ...tableProps },
] = usePipelinesTable();
const sortProps = getTableSortProps(tableProps);
const { setFilter, filter, sortDirection, sortField } = tableProps;

const { totalSize, ...searchProps } = useSelectorSearch({ setFilter, fetchedSize, loaded });
Expand Down Expand Up @@ -68,7 +70,6 @@ const PipelineSelector: React.FC<PipelineSelectorProps> = ({ selection, onSelect
<MenuList>
<div role="menuitem">
<TableBase
{...tableProps}
itemCount={fetchedSize}
loading={!loaded}
data-id="pipeline-selector-table-list"
Expand All @@ -95,7 +96,7 @@ const PipelineSelector: React.FC<PipelineSelectorProps> = ({ selection, onSelect
)}
getColumnSort={getTableColumnSort({
columns: pipelineSelectorColumns,
...tableProps,
...sortProps,
})}
footerRow={() =>
loaded ? (
Expand Down
6 changes: 3 additions & 3 deletions frontend/src/concepts/pipelines/content/pipelineSelector/PipelineVersionSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import usePipelineVersionsTable from '~/concepts/pipelines/content/tables/pipeli
import PipelineViewMoreFooterRow from '~/concepts/pipelines/content/tables/PipelineViewMoreFooterRow';
import { useSelectorSearch } from '~/concepts/pipelines/content/pipelineSelector/utils';
import EmptyTableView from '~/concepts/pipelines/content/tables/EmptyTableView';
import { getTableSortProps } from '~/concepts/pipelines/content/tables/usePipelineTable';

type PipelineVersionSelectorProps = {
pipelineId?: string;
Expand All @@ -45,7 +46,7 @@ const PipelineVersionSelector: React.FC<PipelineVersionSelectorProps> = ({

const toggleRef = React.useRef(null);
const menuRef = React.useRef(null);

const sortProps = getTableSortProps(tableProps);
const { sortDirection, sortField, setFilter, filter } = tableProps;
const { data: versions, onLoadMore } = usePipelineVersionLoadMore({
pipelineId,
Expand Down Expand Up @@ -74,7 +75,6 @@ const PipelineVersionSelector: React.FC<PipelineVersionSelectorProps> = ({
<MenuList>
<div role="menuitem">
<TableBase
{...tableProps}
itemCount={fetchedSize}
loading={!loaded}
data-id="pipeline-version-selector-table-list"
Expand All @@ -101,7 +101,7 @@ const PipelineVersionSelector: React.FC<PipelineVersionSelectorProps> = ({
)}
getColumnSort={getTableColumnSort({
columns: pipelineVersionSelectorColumns,
...tableProps,
...sortProps,
})}
footerRow={() =>
loaded ? (
Expand Down
11 changes: 8 additions & 3 deletions ...end/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunTabDetails.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
} from '~/concepts/pipelines/content/pipelinesDetails/pipelineRun/utils';
import { isPipelineRunJob } from '~/concepts/pipelines/content/utils';
import { PipelineVersionLink } from '~/concepts/pipelines/content/PipelineVersionLink';
import usePipelineVersionById from '~/concepts/pipelines/apiHooks/usePipelineVersionById';

type PipelineRunTabDetailsProps = {
pipelineRunKF?: PipelineRunKF | PipelineRunJobKF;
Expand All @@ -34,7 +35,8 @@ const PipelineRunTabDetails: React.FC<PipelineRunTabDetailsProps> = ({
workflowName,
}) => {
const { namespace, project } = usePipelinesAPI();
const pipelineVersionRef = getPipelineVersionResourceRef(pipelineRunKF);
const versionRef = getPipelineVersionResourceRef(pipelineRunKF);
const [version, loaded, error] = usePipelineVersionById(versionRef?.key.id);

if (!pipelineRunKF || !workflowName) {
return (
Expand All @@ -47,14 +49,17 @@ const PipelineRunTabDetails: React.FC<PipelineRunTabDetailsProps> = ({

const details: DetailItem[] = [
{ key: 'Name', value: <Truncate content={pipelineRunKF.name} /> },
...(pipelineVersionRef
...(versionRef
? [
{
key: 'Pipeline version',
value: (
<PipelineVersionLink
resourceRef={pipelineVersionRef}
displayName={versionRef.name}
loadingIndicator={<Spinner size="sm" />}
loaded={loaded}
version={version}
error={error}
/>
),
},
Expand Down
16 changes: 11 additions & 5 deletions frontend/src/concepts/pipelines/content/tables/pipeline/PipelinesTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import * as React from 'react';
import { PipelineKF } from '~/concepts/pipelines/kfTypes';
import { Table, getTableColumnSort, useCheckboxTableBase } from '~/components/table';
import { Table, TableBase, getTableColumnSort, useCheckboxTableBase } from '~/components/table';
import PipelinesTableRow from '~/concepts/pipelines/content/tables/pipeline/PipelinesTableRow';
import { pipelineColumns } from '~/concepts/pipelines/content/tables/columns';
import { TableBase } from '~/components/table';
import { usePipelinesAPI } from '~/concepts/pipelines/context';
import DeletePipelinesModal from '~/concepts/pipelines/content/DeletePipelinesModal';
import { PipelineAndVersionContext } from '~/concepts/pipelines/content/PipelineAndVersionContext';
Expand Down Expand Up @@ -37,7 +36,11 @@ const PipelinesTable: React.FC<PipelinesTableProps> = ({
setPage,
setPageSize,
pipelineDetailsPath,
...tableProps
enablePagination,
emptyTableView,
toolbarContent,
variant,
...sortProps
}) => {
const { refreshAllAPI } = usePipelinesAPI();
const { pipelineDataSelector } = React.useContext(PipelineAndVersionContext);
Expand All @@ -58,8 +61,11 @@ const PipelinesTable: React.FC<PipelinesTableProps> = ({
return (
<>
<TableBase
{...tableProps}
{...checkboxTableProps}
enablePagination={enablePagination}
emptyTableView={emptyTableView}
toolbarContent={toolbarContent}
variant={variant}
loading={loading}
page={page}
perPage={pageSize}
Expand Down Expand Up @@ -91,7 +97,7 @@ const PipelinesTable: React.FC<PipelinesTableProps> = ({
disableRowRenderSupport
getColumnSort={getTableColumnSort({
columns: pipelineColumns,
...tableProps,
...sortProps,
})}
/>
<DeletePipelinesModal
Expand Down
5 changes: 4 additions & 1 deletion frontend/src/concepts/pipelines/content/tables/pipeline/PipelinesTableExpandedRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { PipelineKF } from '~/concepts/pipelines/kfTypes';
import ImportPipelineVersionButton from '~/concepts/pipelines/content/import/ImportPipelineVersionButton';
import PipelineVersionTable from '~/concepts/pipelines/content/tables/pipelineVersion/PipelineVersionTable';
import usePipelineVersionsTable from '~/concepts/pipelines/content/tables/pipelineVersion/usePipelineVersionsTable';
import { getTableSortProps } from '~/concepts/pipelines/content/tables/usePipelineTable';

type PipelinesTableExpandedRowProps = {
pipeline: PipelineKF;
Expand All @@ -28,6 +29,8 @@ const PipelinesTableExpandedRow: React.FC<PipelinesTableExpandedRowProps> = ({
{ initialLoaded, ...tableProps },
] = usePipelineVersionsTable(pipeline.id)();

const sortProps = getTableSortProps(tableProps);

if (!loaded && !initialLoaded) {
return (
<Tr isExpanded>
Expand Down Expand Up @@ -69,7 +72,7 @@ const PipelinesTableExpandedRow: React.FC<PipelinesTableExpandedRowProps> = ({
<Td className="pf-v5-u-pb-lg" noPadding colSpan={6}>
<ExpandableRowContent>
<PipelineVersionTable
{...tableProps}
{...sortProps}
initialVersions={initialVersions}
loading={!loaded}
totalSize={totalSize}
Expand Down
22 changes: 7 additions & 15 deletions frontend/src/concepts/pipelines/content/tables/pipeline/PipelinesTableRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const PipelinesTableRow: React.FC<PipelinesTableRowProps> = ({
totalSize,
updatedDate,
loading,
refresh: refreshVersions,
refresh: refreshRowData,
} = usePipelineTableRowData(pipeline);
const { versionDataSelector } = React.useContext(PipelineAndVersionContext);
const { selectedVersions } = versionDataSelector(pipeline);
Expand Down Expand Up @@ -95,12 +95,6 @@ const PipelinesTableRow: React.FC<PipelinesTableRowProps> = ({
});
},
},
{
title: 'View all runs',
onClick: () => {
navigate(`/pipelineRuns/${namespace}`);
},
},
{
isSeparator: true,
},
Expand All @@ -116,6 +110,9 @@ const PipelinesTableRow: React.FC<PipelinesTableRowProps> = ({
</Tr>
{isExpanded && (
<PipelinesTableExpandedRow
// Upload a new version will update the pipeline default version
// Which could trigger a re-render of the versions table
key={pipeline.default_version?.id}
pipeline={pipeline}
pipelineDetailsPath={pipelineDetailsPath}
/>
Expand All @@ -126,15 +123,10 @@ const PipelinesTableRow: React.FC<PipelinesTableRowProps> = ({
existingPipeline={importTarget}
onClose={(pipelineVersion) => {
if (pipelineVersion) {
refreshPipelines().then(() =>
refreshVersions().then(() => {
setImportTarget(null);
setExpanded(false);
}),
);
} else {
setImportTarget(null);
refreshPipelines();
refreshRowData();
}
setImportTarget(null);
}}
/>
)}
Expand Down
1 change: 0 additions & 1 deletion frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ const PipelineRunTable: React.FC<PipelineRunTableProps> = ({
return (
<>
<TableBase
{...tableProps}
{...checkboxTableProps}
loading={loading}
page={page}
Expand Down
Loading
Loading