Skip to content

Commit

Permalink
fix: table component should be scoped when searching items (#10211)
Browse files Browse the repository at this point in the history
* fix: table component should be scoped when searching items

it was searching items on the whole document/page
it should only search in its children

fixes podman-desktop/podman-desktop#10210

Signed-off-by: Florent Benoit <[email protected]>
  • Loading branch information
benoitf authored Dec 4, 2024
1 parent 4113046 commit cac1d19
Show file tree
Hide file tree
Showing 5 changed files with 164 additions and 4 deletions.
23 changes: 23 additions & 0 deletions packages/ui/src/lib/table/Table.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,3 +380,26 @@ test('Expect duration cell to be empty when undefined', async () => {
expect(cells[6].textContent?.trim()).toBe(expected[i - 1]);
}
});

test('Expect table is scoped for css manipulation', async () => {
render(TestTable, {});

// Wait for the table to update
await tick();

const table = await screen.findByRole('table');
expect(table).toBeDefined();
// get the elements having the role "row" inside the table html element
const rows = await within(table).findAllByRole('row');
// expect each row of the table has the grid-template-columns style applied
for (const element of rows) {
expect(element.style.gridTemplateColumns).toBe('20px 32px 1fr 3fr 1fr 1fr 1fr 5px');
}

// ok so now look at our dummy component
const dummyComponent = await screen.findByRole('group', { name: 'dummy component' });
expect(dummyComponent).toBeDefined();

// and there should be no style applied to this group as it's not part of the table
expect(dummyComponent.style.gridTemplateColumns).toBe('');
});
12 changes: 8 additions & 4 deletions packages/ui/src/lib/table/Table.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ export let data: { selected?: boolean; name?: string }[];
export let defaultSortColumn: string | undefined = undefined;
export let collapsed: string[] = [];
let tableHtmlDivElement: HTMLDivElement | undefined = undefined;
// number of selected items in the list
export let selectedItemsNumber: number = 0;
$: selectedItemsNumber = row.info.selectable
Expand Down Expand Up @@ -145,9 +147,11 @@ function setGridColumns(): void {
columnWidths.push('5px');
let wid = columnWidths.join(' ');
let grids: HTMLCollection = document.getElementsByClassName('grid-table');
for (const element of grids) {
(element as HTMLElement).style.setProperty('grid-template-columns', wid);
if (tableHtmlDivElement) {
let grids: HTMLCollection = tableHtmlDivElement.getElementsByClassName('grid-table');
for (const element of grids) {
(element as HTMLElement).style.setProperty('grid-template-columns', wid);
}
}
}
Expand Down Expand Up @@ -180,7 +184,7 @@ function toggleChildren(name: string | undefined): void {
}
</script>

<div class="w-full mx-5" class:hidden={data.length === 0} role="table" aria-label={kind}>
<div class="w-full mx-5" class:hidden={data.length === 0} role="table" aria-label={kind} bind:this={tableHtmlDivElement}>
<!-- Table header -->
<div role="rowgroup">
<div
Expand Down
64 changes: 64 additions & 0 deletions packages/ui/src/lib/table/TestMultiTables.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/**********************************************************************
* Copyright (C) 2024 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 { render, screen, within } from '@testing-library/svelte';
import { tick } from 'svelte';
import { expect, test } from 'vitest';

import TestMultiTables from './TestMultiTables.svelte';

test('Expect each table receive its own grid-tables-column values', async () => {
const extractGridTableColumnsWidth = async (tableName: string): Promise<string> => {
const table = await screen.findByRole('table', { name: tableName });
expect(table).toBeDefined();

// get the elements having the role "row" inside the table html element
const rows = await within(table).findAllByRole('row');
const gridTableColumnsValuesSet = new Set<string>();
for (const element of rows) {
gridTableColumnsValuesSet.add(element.style.gridTemplateColumns);
}

// all values should be the same in the set gridTableColumnsValues
const items = Array.from(gridTableColumnsValuesSet.values());
expect(items.length).toBe(1);

const item = items[0];

// split by space and keep the second value
const values = item.split(' ');

expect(values.length).toBe(3);

return values[1];
};

render(TestMultiTables, {});

// Wait for the tables to update
await tick();

// expect to receive for each table, the good values of the width (which is different for each table)
const gridTableColumnsValuesPersonWidth = await extractGridTableColumnsWidth('person');
expect(gridTableColumnsValuesPersonWidth).toBe('3fr');

const gridTableColumnsValuesBookWidth = await extractGridTableColumnsWidth('book');
expect(gridTableColumnsValuesBookWidth).toBe('2fr');
});
64 changes: 64 additions & 0 deletions packages/ui/src/lib/table/TestMultiTables.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<script lang="ts">
import SimpleColumn from './SimpleColumn.svelte';
import { Column, Row } from './table';
import Table from './Table.svelte';
let personTable: Table;
let bookTable: Table;
type Person = {
name: string;
};
type Book = {
title: string;
name?: string;
};
const persons: Person[] = [{ name: 'James' }, { name: 'John' }, { name: 'Robert' }];
const books: Book[] = [
{ title: 'Twenty Thousand Leagues Under the Seas' },
{ title: 'From the Earth to the Moon' },
{ title: 'Around the World in Eighty Days' },
{ title: 'From the Earth to the Moon' },
];
export const nameColPerson: Column<Person, string> = new Column('Name', {
width: '3fr',
renderMapping: obj => obj.name,
renderer: SimpleColumn,
comparator: (a, b) => a.name.localeCompare(b.name),
});
const columnsPerson = [nameColPerson];
const rowPerson = new Row<Person>({});
export const titleColBook: Column<Book, string> = new Column('Title', {
width: '2fr',
renderMapping: obj => obj.title,
renderer: SimpleColumn,
comparator: (a, b) => a.title.localeCompare(b.title),
});
const columnsBook = [titleColBook];
const rowBook = new Row<Book>({});
</script>

<Table
kind="person"
bind:this={personTable}
data={persons}
columns={columnsPerson}
row={rowPerson}
defaultSortColumn="Name">
</Table>

<Table
kind="book"
bind:this={bookTable}
data={books}
columns={columnsBook}
row={rowBook}
defaultSortColumn="Title">
</Table>
5 changes: 5 additions & 0 deletions packages/ui/src/lib/table/TestTable.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,8 @@ const row = new Row<Person>({
defaultSortColumn="Id"
on:update>
</Table>

<!-- Dummy component to check if the table component is not updating this object as it contains grid-table css property -->
<div class="grid-table" role="group" aria-label="dummy component">

</div>

0 comments on commit cac1d19

Please sign in to comment.