Feature/1364 data grid review (#1423)

* removed default value, added tests

* added tests for column-group

* fixed column width and made sl-column next to sl-column-group possible

* changed around the odd and even row colours to better match header

* added comments, attempt to improve sorter

* attempt to write tests for sorter + clean up code

* changeset added

* filter column tests (in progress)

* filter column tests changes, initial test for filter

* filter and filter column unit tests changes

* filter tests changes

* filter tests updates (not working yet)

* trying to fix filter tests

* filter column tests update, still cannot check options

* filter and filter column tests changes

* 🚂

* 🛥

* filter column tests changes

* filter column tests, filter test change

* Trying to change filter test example

* trying to improve the example in filter tests

* filter test updated

* new tests, removed unnecessary elements

* cleanup

* missing export

* filter and filter column changes after review

* review comments

* fixed columns widths in non-grouped columns next to grouped columns

* fix build

---------

Co-authored-by: anna-lach <a.lach@nowaera.pl>
Co-authored-by: Jeroen Zwartepoorte <jeroen.zwartepoorte@iddinkgroup.com>

packages/components/grid/src/column-group.spec.ts

@@ -0,0 +1,104 @@
+import { setupIgnoreWindowResizeObserverLoopErrors } from '@lit-labs/virtualizer/support/resize-observer-errors.js';
+import { expect, fixture } from '@open-wc/testing';
+import { html } from 'lit';
+import '../register.js';
+import { type Grid } from './grid.js';
+
+setupIgnoreWindowResizeObserverLoopErrors(beforeEach, afterEach);
+
+describe('sl-column-group', () => {
+  let el: Grid;
+
+  describe('defaults', () => {
+    beforeEach(async () => {
+      el = await fixture(html`
+        <sl-grid>
+          <sl-grid-column-group header="Name">
+            <sl-grid-column path="firstName"></sl-grid-column>
+            <sl-grid-column path="lastName"></sl-grid-column>
+          </sl-grid-column-group>
+          <sl-grid-column-group header="Grades">
+            <sl-grid-column path="grades.biology"></sl-grid-column>
+            <sl-grid-column path="grades.maths"></sl-grid-column>
+            <sl-grid-column path="grades.english"></sl-grid-column>
+            <sl-grid-column path="age"></sl-grid-column>
+          </sl-grid-column-group>
+        </sl-grid>
+      `);
+      el.items = [{ firstName: 'John', lastName: 'Doe', grades: { biology: 'A', maths: 'B', english: 'B+' } }];
+      await el.updateComplete;
+
+      // Give grid time to render the table structure
+      await new Promise(resolve => setTimeout(resolve, 100));
+      await el.updateComplete;
+    });
+
+    it('should render column headers', () => {
+      const columns = Array.from(el.renderRoot.querySelectorAll('th')).map(col => col.textContent);
+
+      expect(columns).to.deep.equal([
+        'Name',
+        'Grades',
+        'First name',
+        'Last name',
+        'Biology',
+        'Maths',
+        'English',
+        'Age'
+      ]);
+    });
+
+    it('should have the correct width', () => {
+      const cells = Array.from(el.renderRoot.querySelectorAll('th'));
+      expect(cells.map(cell => Math.floor(parseFloat(getComputedStyle(cell).width)))).to.deep.equal([
+        300, 481, 151, 148, 128, 120, 128, 103
+      ]);
+    });
+
+    // it('should emit an sl-column-update event after clicking the checkbox', async () => {
+    //   const columnUpdateEvent = spy();
+    //   const columnGoup = el.querySelector('sl-grid-column-group:first-of-type') as GridColumnGroup;
+    //   columnGoup?.addEventListener('sl-column-update', columnUpdateEvent);
+
+    //   const newColumn = document.createElement('sl-grid-column');
+    //   await new Promise(resolve => setTimeout(resolve, 100));
+
+    //   columnGoup?.appendChild(newColumn);
+
+    //   // expect(columnUpdateEvent).to.have.been.called;
+    //   expect(columnGoup?.columns).to.equal(3);
+    // });
+  });
+
+  describe('explicit width', () => {
+    beforeEach(async () => {
+      el = await fixture(html`
+        <sl-grid>
+          <sl-grid-column-group header="Name">
+            <sl-grid-column path="firstName"></sl-grid-column>
+            <sl-grid-column path="lastName"></sl-grid-column>
+          </sl-grid-column-group>
+          <sl-grid-column-group header="Grades" width="600">
+            <sl-grid-column path="grades.biology"></sl-grid-column>
+            <sl-grid-column path="grades.maths"></sl-grid-column>
+            <sl-grid-column path="grades.english"></sl-grid-column>
+          </sl-grid-column-group>
+        </sl-grid>
+      `);
+
+      el.items = [{ firstName: 'John', lastName: 'Doe', grades: { biology: 'A', maths: 'B', english: 'B+' } }];
+      await el.updateComplete;
+
+      // Give grid time to render the table structure
+      await new Promise(resolve => setTimeout(resolve, 100));
+      await el.updateComplete;
+    });
+
+    it('should have the correct width when one is set explicitly', () => {
+      const cells = Array.from(el.renderRoot.querySelectorAll('th'));
+      expect(cells.map(cell => Math.floor(parseFloat(getComputedStyle(cell).width)))).to.deep.equal([
+        209, 600, 177, 175, 155, 147, 155
+      ]);
+    });
+  });
+});

packages/components/grid/src/column-group.ts

@@ -1,4 +1,3 @@
-import { getNameByPath } from '@sl-design-system/shared';
 import { type PropertyValues, type TemplateResult, html } from 'lit';
 import { state } from 'lit/decorators.js';
 import { GridColumn } from './column.js';
@@ -35,7 +34,7 @@ export class GridColumnGroup<T = unknown> extends GridColumn<T> {
   }
 
   override renderHeader(): TemplateResult {
-    return html`<th colspan=${this.columns.length}>${this.header ?? getNameByPath(this.path)}</th>`;
+    return html`<th colspan=${Math.max(this.columns.length, 1)}>${this.header}</th>`;
   }
 
   #onSlotchange(event: Event & { target: HTMLSlotElement }): void {

packages/components/grid/src/column.spec.ts

@@ -0,0 +1,104 @@
+import { setupIgnoreWindowResizeObserverLoopErrors } from '@lit-labs/virtualizer/support/resize-observer-errors.js';
+import { expect, fixture } from '@open-wc/testing';
+import { Avatar } from '@sl-design-system/avatar';
+import '@sl-design-system/avatar/register.js';
+import { html } from 'lit';
+import { Person } from 'tools/example-data/index.js';
+import '../register.js';
+import { GridColumnDataRenderer } from './column.js';
+import { type Grid } from './grid.js';
+
+setupIgnoreWindowResizeObserverLoopErrors(beforeEach, afterEach);
+
+describe('sl-column', () => {
+  let el: Grid;
+  let cells: HTMLElement[];
+
+  describe('defaults', () => {
+    beforeEach(async () => {
+      el = await fixture(html`
+        <sl-grid>
+          <sl-grid-column path="firstName"></sl-grid-column>
+          <sl-grid-column path="lastName"></sl-grid-column>
+          <sl-grid-column path="age" align="end" grow="3" header="Current age"></sl-grid-column>
+        </sl-grid>
+      `);
+      el.items = [
+        { firstName: 'John', lastName: 'Doe', age: 20 },
+        { firstName: 'Jane', lastName: 'Smith', age: 40 }
+      ];
+      await el.updateComplete;
+
+      // Give grid time to render the table structure
+      await new Promise(resolve => setTimeout(resolve, 100));
+      await el.updateComplete;
+
+      cells = Array.from(el.renderRoot.querySelectorAll('tbody tr:first-of-type td'));
+    });
+
+    it('should render column headers', () => {
+      const columns = Array.from(el.renderRoot.querySelectorAll('th')).map(col => col.textContent);
+
+      expect(columns).to.deep.equal(['First name', 'Last name', 'Current age']);
+    });
+
+    it('should have the right justify-content value', () => {
+      expect(cells.map(cell => getComputedStyle(cell).justifyContent)).to.deep.equal(['start', 'start', 'end']);
+    });
+
+    it('should have the right grow value', () => {
+      expect(cells.map(cell => getComputedStyle(cell).flexGrow)).to.deep.equal(['1', '1', '3']);
+    });
+
+    it('should have the right parts', () => {
+      expect(cells.map(cell => cell.getAttribute('part'))).to.deep.equal([
+        'data first-name',
+        'data last-name',
+        'data age'
+      ]);
+    });
+  });
+
+  describe('custom renderer', () => {
+    beforeEach(async () => {
+      const avatarRenderer: GridColumnDataRenderer<Person> = ({ firstName, lastName }) => {
+        return html`<sl-avatar .displayName=${[firstName, lastName].join(' ')} size="sm"></sl-avatar>`;
+      };
+
+      el = await fixture(html`
+        <sl-grid>
+          <sl-grid-column
+            header="Person"
+            .renderer=${avatarRenderer}
+            .scopedElements=${{
+              'sl-avatar': Avatar
+            }}
+          ></sl-grid-column>
+          <sl-grid-column path="age" parts="number"></sl-grid-column>
+        </sl-grid>
+      `);
+      el.items = [
+        { firstName: 'John', lastName: 'Doe', age: 20 },
+        { firstName: 'Jane', lastName: 'Smith', age: 40 }
+      ];
+      await el.updateComplete;
+
+      // Give grid time to render the table structure
+      await new Promise(resolve => setTimeout(resolve, 100));
+      await el.updateComplete;
+
+      cells = Array.from(el.renderRoot.querySelectorAll('tbody tr:first-of-type td'));
+    });
+
+    it('should render the elements set with the custom renderer', () => {
+      const avatar = cells[0].querySelector('sl-avatar') as Avatar;
+
+      expect(avatar).to.exist;
+      expect(avatar?.shadowRoot?.querySelector('[part="name"]')?.textContent).to.equal('John Doe');
+    });
+
+    it('should have the right parts, including one set on the column', () => {
+      expect(cells.map(cell => cell.getAttribute('part'))).to.deep.equal(['data', 'data number age']);
+    });
+  });
+});

packages/components/grid/src/column.ts

@@ -43,7 +43,7 @@ export class GridColumn<T = any> extends LitElement {
   #width?: number;
 
   /** The alignment of the content within the column. */
-  @property() align: GridColumnAlignment = 'start';
+  @property() align?: GridColumnAlignment;
 
   /**
    * Automatically sets the width of the column based on the column contents when this is set to `true`.
@@ -160,6 +160,7 @@ export class GridColumn<T = any> extends LitElement {
     if (typeof this.parts === 'string') {
       parts = this.parts.split(' ');
     } else if (typeof this.parts === 'function' && item) {
+      // TODO: what does this do? How can parts ever be a function? According to the typing this should not be possible.
       parts = this.parts(item)?.split(' ') ?? [];
     }