fix: table headers shouldn't allow text selection

Our column headers shouldn't allow you to select the text, and sortable
columns should have a pointer cursor.

Added another unsortable column to test table to extend tests.

Fixes #5114.

Signed-off-by: Tim deBoer <git@tdeboer.ca>

packages/renderer/src/lib/table/Table.spec.ts

@@ -44,26 +44,44 @@ test('Expect basic table layout', async () => {
   expect(rows[3].textContent).toContain('43');
 });
 
-test('Expect column headers with sort indicators', async () => {
+test('Expect basic column headers', async () => {
   render(TestTable, {});
 
   const headers = await screen.findAllByRole('columnheader');
   expect(headers).toBeDefined();
-  expect(headers.length).toBe(5);
+  expect(headers.length).toBe(6);
   expect(headers[2].textContent).toContain('Id');
-  expect(headers[2].innerHTML).toContain('fa-sort');
+  expect(headers[2]).toHaveClass('select-none');
   expect(headers[3].textContent).toContain('Name');
-  expect(headers[3].innerHTML).toContain('fa-sort');
+  expect(headers[3]).toHaveClass('select-none');
   expect(headers[4].textContent).toContain('Age');
+  expect(headers[4]).toHaveClass('select-none');
+  expect(headers[5].textContent).toContain('Hobby');
+  expect(headers[5]).toHaveClass('select-none');
+});
+
+test('Expect column sort indicators', async () => {
+  render(TestTable, {});
+
+  const headers = await screen.findAllByRole('columnheader');
+  expect(headers).toBeDefined();
+  expect(headers.length).toBe(6);
+  expect(headers[2].innerHTML).toContain('fa-sort');
+  expect(headers[2]).toHaveClass('cursor-pointer');
+  expect(headers[3].innerHTML).toContain('fa-sort');
+  expect(headers[3]).toHaveClass('cursor-pointer');
   expect(headers[4].innerHTML).toContain('fa-sort');
+  expect(headers[4]).toHaveClass('cursor-pointer');
+  expect(headers[5].innerHTML).not.toContain('fa-sort');
+  expect(headers[5]).not.toHaveClass('cursor-pointer');
 });
 
 test('Expect default sort indicator', async () => {
   render(TestTable, {});
 
   const headers = await screen.findAllByRole('columnheader');
   expect(headers).toBeDefined();
-  expect(headers.length).toBe(5);
+  expect(headers.length).toBe(6);
   expect(headers[2].textContent).toContain('Id');
   expect(headers[2].innerHTML).toContain('fa-sort-up');
 });
@@ -73,7 +91,7 @@ test('Expect no default sort indicator on other columns', async () => {
 
   const headers = await screen.findAllByRole('columnheader');
   expect(headers).toBeDefined();
-  expect(headers.length).toBe(5);
+  expect(headers.length).toBe(6);
   expect(headers[3].innerHTML).not.toContain('fa-sort-up');
   expect(headers[3].innerHTML).not.toContain('fa-sort-down');
   expect(headers[4].innerHTML).not.toContain('fa-sort-up');
@@ -158,7 +176,7 @@ test('Expect correct aria roles', async () => {
   // there should be 5 column headers (expander, checkbox, 3 columns)
   const headers = await screen.findAllByRole('columnheader');
   expect(headers).toBeDefined();
-  expect(headers.length).toBe(5);
+  expect(headers.length).toBe(6);
   expect(headers[2].textContent).toContain('Id');
   expect(headers[3].textContent).toContain('Name');
   expect(headers[4].textContent).toContain('Age');
@@ -168,11 +186,11 @@ test('Expect correct aria roles', async () => {
   expect(rows).toBeDefined();
   expect(rows.length).toBe(4);
 
-  // and each non-header row should have 5 cells (expander, checkbox, 3 cells)
-  for (let i = 1; i < 4; i++) {
+  // and each non-header row should have 6 cells (expander, checkbox, 4 cells)
+  for (let i = 1; i < 5; i++) {
     const cells = await within(rows[i]).findAllByRole('cell');
     expect(cells).toBeDefined();
-    expect(cells.length).toBe(5);
+    expect(cells.length).toBe(6);
   }
 });
 
@@ -187,7 +205,7 @@ test('Expect rowgroups', async () => {
   // one for the header row
   const headers = await within(rowgroups[0]).findAllByRole('columnheader');
   expect(headers).toBeDefined();
-  expect(headers.length).toBe(5);
+  expect(headers.length).toBe(6);
 
   // and one for the data rows
   const dataRows = await within(rowgroups[1]).findAllByRole('row');

packages/renderer/src/lib/table/Table.svelte

@@ -145,7 +145,8 @@ function setGridColumns() {
             ? 'justify-self-end'
             : column.info.align === 'center'
               ? 'justify-self-center'
-              : 'justify-self-start'} self-center"
+              : 'justify-self-start'} self-center select-none"
+          class:cursor-pointer="{column.info.comparator}"
           on:click="{() => sort(column)}"
           role="columnheader">
           {column.title}{#if column.info.comparator}<i

packages/renderer/src/lib/table/TestTable.svelte

@@ -10,12 +10,13 @@ type Person = {
   id: number;
   name: string;
   age: number;
+  hobby: string;
 };
 
 const people: Person[] = [
-  { id: 1, name: 'John', age: 57 },
-  { id: 2, name: 'Henry', age: 27 },
-  { id: 3, name: 'Charlie', age: 43 },
+  { id: 1, name: 'John', age: 57, hobby: 'Skydiving' },
+  { id: 2, name: 'Henry', age: 27, hobby: 'Cooking' },
+  { id: 3, name: 'Charlie', age: 43, hobby: 'Biking' },
 ];
 
 const idCol: Column<Person, string> = new Column('Id', {
@@ -40,7 +41,12 @@ const ageCol: Column<Person, string> = new Column('Age', {
   initialOrder: 'descending',
 });
 
-const columns: Column<Person, string>[] = [idCol, nameCol, ageCol];
+const hobbyCol: Column<Person, string> = new Column('Hobby', {
+  renderMapping: obj => obj.hobby,
+  renderer: SimpleColumn,
+});
+
+const columns: Column<Person, string>[] = [idCol, nameCol, ageCol, hobbyCol];
 
 const row = new Row<Person>({
   selectable: person => person.age < 50,