From e3a635c0e4788c5c9c6902258dbf8051edd070bf Mon Sep 17 00:00:00 2001 From: Tim deBoer Date: Thu, 9 Nov 2023 13:59:51 -0500 Subject: [PATCH] feat: table component DRAFT. We have impending technical debt where our expanding number of table components (images, volumes, pods, containers, soon adding Kubernetes objects) will cross with our future design ideas (tables should be sortable, columns configurable, environments groupable, etc). We can't copy/paste these features into every page, and deal with differences between them. This is the best I've come up with so far. Each page passes an array of objects to a Table component, which is responsible for creating the basic layout and calling back a Row component to render each cell in the table. This seems like just enough abstraction b/c all of the basic/common table capability can be handled in that component, and each Row doesn't know or care if columns are missing or sorted differently. A TableHelper class providers a few functions like whether objects can be selected & sorting. Switched to grid layout, which also helped to reduce code in *Row components. I am still getting bizarre changes when I try to tweak the column widths, but it appears to be in a happy spot here. Added sorting with a sort indicator in the header, and tests. Sorting by image and volume size was left out for now b/c it shouldn't be alphabetical. Still to do before removing draft status: - Filtering isn't working great... but maybe that's how it was working before. - Volume* and Image* will be extracted to their own PRs, but I felt like it helps to see how they change here. Support for 'object containers' in order to support Pod and Container lists will be done separately since this is already a big PR. Fixes #4365. Signed-off-by: Tim deBoer --- .../renderer/src/lib/image/ImageRow.svelte | 91 ++++++++ .../renderer/src/lib/image/ImagesList.svelte | 196 +++++------------- packages/renderer/src/lib/table/Table.spec.ts | 87 ++++++++ packages/renderer/src/lib/table/Table.svelte | 134 ++++++++++++ .../renderer/src/lib/table/TestRow.svelte | 12 ++ .../renderer/src/lib/table/TestTable.svelte | 44 ++++ packages/renderer/src/lib/table/table.ts | 50 +++++ .../renderer/src/lib/volume/VolumeRow.svelte | 34 +++ .../src/lib/volume/VolumesList.svelte | 150 +++++--------- 9 files changed, 561 insertions(+), 237 deletions(-) create mode 100644 packages/renderer/src/lib/image/ImageRow.svelte create mode 100644 packages/renderer/src/lib/table/Table.spec.ts create mode 100644 packages/renderer/src/lib/table/Table.svelte create mode 100644 packages/renderer/src/lib/table/TestRow.svelte create mode 100644 packages/renderer/src/lib/table/TestTable.svelte create mode 100644 packages/renderer/src/lib/table/table.ts create mode 100644 packages/renderer/src/lib/volume/VolumeRow.svelte diff --git a/packages/renderer/src/lib/image/ImageRow.svelte b/packages/renderer/src/lib/image/ImageRow.svelte new file mode 100644 index 0000000000000..080c35f0019fc --- /dev/null +++ b/packages/renderer/src/lib/image/ImageRow.svelte @@ -0,0 +1,91 @@ + + +{#if column === 'Status'} + +{:else if column === 'Name'} + + +
+
+
{image.name}
+
+
+
{image.shortId}
+
{image.tag}
+
+
+ + {#if multipleEngines} +
+ {image.engineName} +
+ {/if} +
+
+{:else if column === 'Age'} +
+ {image.age} +
+{:else if column === 'Size'} +
+ {image.humanSize} +
+{:else} + +{/if} +{#if pushImageModal && pushImageModalImageInfo} + +{/if} +{#if renameImageModal && renameImageModalImageInfo} + +{/if} diff --git a/packages/renderer/src/lib/image/ImagesList.svelte b/packages/renderer/src/lib/image/ImagesList.svelte index a28a9ee05d9d3..54bf807c55a87 100644 --- a/packages/renderer/src/lib/image/ImagesList.svelte +++ b/packages/renderer/src/lib/image/ImagesList.svelte @@ -6,58 +6,35 @@ import FilteredEmptyScreen from '../ui/FilteredEmptyScreen.svelte'; import { router } from 'tinro'; import type { ImageInfoUI } from './ImageInfoUI'; -import ImageActions from './ImageActions.svelte'; import type { ImageInfo } from '../../../../main/src/plugin/api/image-info'; import NoContainerEngineEmptyScreen from './NoContainerEngineEmptyScreen.svelte'; import { providerInfos } from '../../stores/providers'; -import PushImageModal from './PushImageModal.svelte'; -import RenameImageModal from './RenameImageModal.svelte'; import { ImageUtils } from './image-utils'; import NavPage from '../ui/NavPage.svelte'; import ImageIcon from '../images/ImageIcon.svelte'; -import StatusIcon from '../images/StatusIcon.svelte'; import type { Unsubscriber } from 'svelte/store'; import { containersInfos } from '../../stores/containers'; import type { ContainerInfo } from '../../../../main/src/plugin/api/container-info'; import moment from 'moment'; import Prune from '../engine/Prune.svelte'; import type { EngineInfoUI } from '../engine/EngineInfoUI'; -import Checkbox from '../ui/Checkbox.svelte'; import Button from '../ui/Button.svelte'; import { faArrowCircleDown, faCube, faTrash } from '@fortawesome/free-solid-svg-icons'; +import ImageRow from './ImageRow.svelte'; +import Table from '../table/Table.svelte'; +import { Column, TableHelper } from '../table/table'; export let searchTerm = ''; $: searchPattern.set(searchTerm); let images: ImageInfoUI[] = []; -let multipleEngines = false; let enginesList: EngineInfoUI[]; -let pushImageModal = false; -let pushImageModalImageInfo: ImageInfoUI | undefined = undefined; -function handlePushImageModal(imageInfo: ImageInfoUI) { - pushImageModalImageInfo = imageInfo; - pushImageModal = true; -} - -let renameImageModal = false; -let renameImageModalImageInfo: ImageInfoUI | undefined = undefined; -function handleRenameImageModal(imageInfo: ImageInfoUI) { - renameImageModalImageInfo = imageInfo; - renameImageModal = true; -} - $: providerConnections = $providerInfos .map(provider => provider.containerConnections) .flat() .filter(providerContainerConnection => providerContainerConnection.status === 'started'); -// number of selected items in the list -$: selectedItemsNumber = images.filter(image => !image.inUse).filter(image => image.selected).length; - -// do we need to unselect all checkboxes if we don't have all items being selected ? -$: selectedAllCheckboxes = images.filter(image => !image.inUse).every(image => image.selected); - const imageUtils = new ImageUtils(); function updateImages() { @@ -88,12 +65,6 @@ function updateImages() { // Remove duplicates from engines by name const uniqueEngines = engines.filter((engine, index, self) => index === self.findIndex(t => t.name === engine.name)); - if (uniqueEngines.length > 1) { - multipleEngines = true; - } else { - multipleEngines = false; - } - // Set the engines to the global variable for the Prune functionality button enginesList = uniqueEngines; @@ -133,11 +104,6 @@ onDestroy(() => { } }); -function closeModals() { - pushImageModal = false; - renameImageModal = false; -} - function gotoBuildImage(): void { router.goto('/images/build'); } @@ -146,17 +112,6 @@ function gotoPullImage(): void { router.goto('/images/pull'); } -function openDetailsImage(image: ImageInfoUI) { - router.goto(`/images/${image.id}/${image.engineId}/${image.base64RepoTag}/summary`); -} - -function toggleAllImages(checked: boolean) { - const toggleImages = images; - // filter out all images used by a container - toggleImages.filter(image => !image.inUse).forEach(image => (image.selected = checked)); - images = toggleImages; -} - // delete the items selected in the list let bulkDeleteInProgress = false; async function deleteSelectedImages() { @@ -214,6 +169,50 @@ function computeInterval(): number { // every day return 60 * 60 * 24 * SECOND; } + +let selectedItemsNumber: number; +let table: Table; + +const columns: Column[] = [ + new Column('Status', 'center', '70px'), + new Column('Name', 'left', '3fr'), + new Column('Age', 'left', '1fr'), + new Column('Size', 'right', '1fr'), +]; + +const tableHelper = new (class extends TableHelper { + getKey(object: any): any { + return object.name + object.shortId + object.tag; + } + + selectable(image: any): boolean { + return image.inUse; + } + + disabledText(): string { + return 'Image is used by a container'; + } + + compare(column: string): ((object1: any, object2: any) => number) | undefined { + if (column === 'Status') { + return (a, b) => { + let au: boolean = a.inUse; + let bu: boolean = b.inUse; + return au === bu ? 0 : au ? -1 : 1; + }; + } else if (column === 'Name') { + return (a, b) => a.name.localeCompare(b.name); + } else if (column === 'Age') { + return (a, b) => { + return b.createdAt - a.createdAt; + }; + } else if (column === 'Size') { + // TODO: should sort by value, not alphabetically (e.g. 1 Gb > 2 Mb) + //return (a, b) => a.humanSize.localeCompare(b.humanSize); + } + return undefined; + } +})('image'); @@ -239,84 +238,14 @@ function computeInterval(): number {
- - - - - - - - - - - - - - - {#each images as image} - - - - - - - - - - - {/each} - -
- - statusNameagesizeActions
- - -
- -
- 		-
-
-
-
{image.name}
-
-
-
{image.shortId}
-
{image.tag}
-
-
- - {#if multipleEngines} -
- {image.engineName} -
- {/if} -
-
-
- 		-
-
{image.age}
-
- 		-
-
{image.humanSize}
-
- 		- -
 
+ +
{#if providerConnections.length === 0} @@ -327,20 +256,5 @@ function computeInterval(): number { {/if} {/if} - - {#if pushImageModal && pushImageModalImageInfo} - - {/if} - {#if renameImageModal && renameImageModalImageInfo} - - {/if}
diff --git a/packages/renderer/src/lib/table/Table.spec.ts b/packages/renderer/src/lib/table/Table.spec.ts new file mode 100644 index 0000000000000..b9770728d51cd --- /dev/null +++ b/packages/renderer/src/lib/table/Table.spec.ts @@ -0,0 +1,87 @@ +/********************************************************************** + * Copyright (C) 2023 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + ***********************************************************************/ + +import '@testing-library/jest-dom/vitest'; +import { test, expect } from 'vitest'; +import { fireEvent, render, screen } from '@testing-library/svelte'; + +import TestTable from './TestTable.svelte'; + +test('Expect basic table layout', async () => { + // render the component + render(TestTable, {}); + + // 3 people = header + 3 rows + const rows = await screen.findAllByRole('row'); + expect(rows).toBeDefined(); + expect(rows.length).toBe(4); + + // first data row should contain John and his age + expect(rows[1].textContent).toContain('John'); + expect(rows[1].textContent).toContain('57'); + + // second data row should contain Henry and his age + expect(rows[2].textContent).toContain('Henry'); + expect(rows[2].textContent).toContain('27'); + + // last data row should contain Charlie and his age + expect(rows[3].textContent).toContain('Charlie'); + expect(rows[3].textContent).toContain('43'); +}); + +test('Expect sorting by name works', async () => { + render(TestTable, {}); + + const nameCol = screen.getByText('Name'); + expect(nameCol).toBeInTheDocument(); + + let rows = await screen.findAllByRole('row'); + expect(rows).toBeDefined(); + expect(rows.length).toBe(4); + expect(rows[1].textContent).toContain('John'); + expect(rows[2].textContent).toContain('Henry'); + expect(rows[3].textContent).toContain('Charlie'); + + await fireEvent.click(nameCol); + + rows = await screen.findAllByRole('row'); + expect(rows[1].textContent).toContain('Charlie'); + expect(rows[2].textContent).toContain('Henry'); + expect(rows[3].textContent).toContain('John'); +}); + +test('Expect sorting by age works', async () => { + render(TestTable, {}); + + const ageCol = screen.getByText('Age'); + expect(ageCol).toBeInTheDocument(); + + let rows = await screen.findAllByRole('row'); + expect(rows).toBeDefined(); + expect(rows.length).toBe(4); + expect(rows[1].textContent).toContain('John'); + expect(rows[2].textContent).toContain('Henry'); + expect(rows[3].textContent).toContain('Charlie'); + + await fireEvent.click(ageCol); + + rows = await screen.findAllByRole('row'); + expect(rows[1].textContent).toContain('Henry'); + expect(rows[2].textContent).toContain('Charlie'); + expect(rows[3].textContent).toContain('John'); +}); diff --git a/packages/renderer/src/lib/table/Table.svelte b/packages/renderer/src/lib/table/Table.svelte new file mode 100644 index 0000000000000..c5f6fdd19c5b4 --- /dev/null +++ b/packages/renderer/src/lib/table/Table.svelte @@ -0,0 +1,134 @@ + + +
+ +
+
+
+ +
+ {#each columns as col} + + +
+ {col.title}{#if helper.compare(col.title)}{/if} +
+ {/each} +
Actions
+
+ + {#each objects as object (object)} +
+
+
+ +
+ {#each columns as col} +
+ +
+ {/each} + +
+ +
+
+ {/each} +
diff --git a/packages/renderer/src/lib/table/TestRow.svelte b/packages/renderer/src/lib/table/TestRow.svelte new file mode 100644 index 0000000000000..15aef6cd45cc7 --- /dev/null +++ b/packages/renderer/src/lib/table/TestRow.svelte @@ -0,0 +1,12 @@ + + +{#if column === 'Name'} + {object.name} +{:else if column === 'Age'} + {object.age} +{:else} + Actions here +{/if} diff --git a/packages/renderer/src/lib/table/TestTable.svelte b/packages/renderer/src/lib/table/TestTable.svelte new file mode 100644 index 0000000000000..8bea66082cdcd --- /dev/null +++ b/packages/renderer/src/lib/table/TestTable.svelte @@ -0,0 +1,44 @@ + + + +
diff --git a/packages/renderer/src/lib/table/table.ts b/packages/renderer/src/lib/table/table.ts new file mode 100644 index 0000000000000..efe1c1652066f --- /dev/null +++ b/packages/renderer/src/lib/table/table.ts @@ -0,0 +1,50 @@ +/********************************************************************** + * Copyright (C) 2023 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + ***********************************************************************/ + +export type Alignment = 'left' | 'center' | 'right'; + +export class Column { + constructor( + readonly title: string, + readonly align: Alignment, + readonly width: string, + ) {} +} + +export class TableHelper { + objectKind: string; + + constructor(kind: string) { + this.objectKind = kind; + } + + selectable(_object: any): boolean { + // is this object selectable? + return true; + } + + disabledText(): string { + // text to display as tooltip when selection is disabled + return ''; + } + + compare(_column: string): ((object1: any, object2: any) => number) | undefined { + // no sorting of any column + return undefined; + } +} diff --git a/packages/renderer/src/lib/volume/VolumeRow.svelte b/packages/renderer/src/lib/volume/VolumeRow.svelte new file mode 100644 index 0000000000000..7a66f67ba0ec3 --- /dev/null +++ b/packages/renderer/src/lib/volume/VolumeRow.svelte @@ -0,0 +1,34 @@ + + +{#if column === 'Status'} + +{:else if column === 'Name'} + + +
+ {object.shortName} +
+{:else if column === 'Age'} +
+ {object.age} +
+{:else if column === 'Size'} +
+ {object.humanSize} +
+{:else} + +{/if} diff --git a/packages/renderer/src/lib/volume/VolumesList.svelte b/packages/renderer/src/lib/volume/VolumesList.svelte index 4077d0b08f8f1..083a9febed699 100644 --- a/packages/renderer/src/lib/volume/VolumesList.svelte +++ b/packages/renderer/src/lib/volume/VolumesList.svelte @@ -11,21 +11,20 @@ import { VolumeUtils } from './volume-utils'; import NoContainerEngineEmptyScreen from '../image/NoContainerEngineEmptyScreen.svelte'; import VolumeEmptyScreen from './VolumeEmptyScreen.svelte'; import FilteredEmptyScreen from '../ui/FilteredEmptyScreen.svelte'; -import VolumeActions from './VolumeActions.svelte'; import VolumeIcon from '../images/VolumeIcon.svelte'; -import StatusIcon from '../images/StatusIcon.svelte'; import Prune from '../engine/Prune.svelte'; import moment from 'moment'; import type { EngineInfoUI } from '../engine/EngineInfoUI'; -import Checkbox from '../ui/Checkbox.svelte'; import Button from '../ui/Button.svelte'; import { faPieChart, faPlusCircle, faTrash } from '@fortawesome/free-solid-svg-icons'; +import Table from '../table/Table.svelte'; +import VolumeRow from './VolumeRow.svelte'; +import { Column, TableHelper } from '../table/table'; export let searchTerm = ''; $: searchPattern.set(searchTerm); let volumes: VolumeInfoUI[] = []; -let multipleEngines = false; let enginesList: EngineInfoUI[]; $: providerConnections = $providerInfos @@ -33,12 +32,6 @@ $: providerConnections = $providerInfos .flat() .filter(providerContainerConnection => providerContainerConnection.status === 'started'); -// number of selected items in the list -$: selectedItemsNumber = volumes.filter(volume => !volume.inUse).filter(volume => volume.selected).length; - -// do we need to unselect all checkboxes if we don't have all items being selected ? -$: selectedAllCheckboxes = volumes.filter(volume => !volume.inUse).every(volume => volume.selected); - const volumeUtils = new VolumeUtils(); let volumesUnsubscribe: Unsubscriber; @@ -60,11 +53,7 @@ onMount(async () => { const uniqueEngines = engines.filter( (engine, index, self) => index === self.findIndex(t => t.name === engine.name), ); - if (uniqueEngines.length > 1) { - multipleEngines = true; - } else { - multipleEngines = false; - } + // Set the engines to the global variable for the Prune functionality button enginesList = uniqueEngines; @@ -96,13 +85,6 @@ onDestroy(() => { } }); -function toggleAllVolumes(checked: boolean) { - const toggleVolumes = volumes; - // filter out all volumes used by a container - toggleVolumes.filter(volume => !volume.inUse).forEach(volume => (volume.selected = checked)); - volumes = toggleVolumes; -} - // delete the items selected in the list let bulkDeleteInProgress = false; async function deleteSelectedVolumes() { @@ -123,10 +105,6 @@ async function deleteSelectedVolumes() { } } -function openDetailsVolume(volume: VolumeInfoUI) { - router.goto(`/volumes/${encodeURI(volume.name)}/${encodeURI(volume.engineId)}/summary`); -} - let refreshTimeouts: NodeJS.Timeout[] = []; const SECOND = 1000; function refreshAge() { @@ -185,6 +163,48 @@ async function fetchUsageData() { function gotoCreateVolume(): void { router.goto('/volumes/create'); } + +let selectedItemsNumber: number; +let table: Table; + +const columns: Column[] = [ + new Column('Status', 'center', '70px'), + new Column('Name', 'left', '3fr'), + new Column('Age', 'left', '1fr'), + new Column('Size', 'right', '1fr'), +]; + +const tableHelper = new (class extends TableHelper { + selectable(volume: any): boolean { + return volume.inUse; + } + + disabledText(): string { + return 'Volume is used by a container'; + } + + compare(column: string): ((object1: any, object2: any) => number) | undefined { + if (column === 'Status') { + return (a, b) => { + let au: boolean = a.inUse; + let bu: boolean = b.inUse; + return au === bu ? 0 : au ? -1 : 1; + }; + } else if (column === 'Name') { + return (a, b) => a.shortName.localeCompare(b.shortName); + } else if (column === 'Age') { + return (a, b) => { + const au = moment().diff(a.created); + const bu = moment().diff(b.created); + return au - bu; + }; + } else if (column === 'Size') { + // TODO: should sort by value, not alphabetically (e.g. 1 Gb > 2 Mb) + //return (a, b) => a.humanSize.localeCompare(b.humanSize); + } + return undefined; + } +})('volume'); @@ -215,76 +235,14 @@ function gotoCreateVolume(): void {
- - - - - - - - - - - - - - - {#each volumes as volume} - - - - - - - - - - - {/each} - -
- - statusNameagesizeActions
- - -
- -
- 		-
-
-
-
{volume.shortName}
-
-
- - {#if multipleEngines} -
- {volume.engineName} -
- {/if} -
-
-
- 		-
-
{volume.age}
-
- 		-
-
{volume.humanSize}
-
- 		- -
 
+ +
{#if providerConnections.length === 0}