sortable action use data-sort-value HTML attributes for sorting w…

…hen present (#16)

* sortable action ignores formatted numbers as strings when table cells provide data-sort-value HTML attribute for sorting

- Implement data-sort-value attribute support for more robust ordering
- Add get_html_sort_value function for recursive sort value extraction
- Introduce configurable CSS classes and styles for sorted headers
- Improve sorting logic to handle empty values and numeric comparisons

* add sortable unit tests

* more unit tests in actions/highlight-matches.test.ts

* skip eslint in pre-commit.ci + only deploy to GH pages on push to main

.github/workflows/gh-pages.yml

@@ -1,8 +1,6 @@
 name: GitHub Pages
 
 on:
-  pull_request:
-    branches: [main]
   push:
     branches: [main]
   workflow_dispatch:

.pre-commit-config.yaml

@@ -1,5 +1,6 @@
 ci:
   autoupdate_schedule: quarterly
+  skip: [eslint]
 
 default_stages: [commit]
 

eslint.config.js

@@ -11,7 +11,7 @@ export default [
         `error`,
         { argsIgnorePattern: `^_`, varsIgnorePattern: `^_` },
       ],
-      '@typescript-eslint/quotes': [`error`, `backtick`, { avoidEscape: true }],
+      '@/quotes': [`error`, `backtick`, { avoidEscape: true }],
       'svelte/no-at-html-tags': `off`,
     },
   },

src/lib/actions.ts

@@ -1,6 +1,24 @@
+export function get_html_sort_value(element: Element): string {
+  if (element.dataset.sortValue !== undefined) {
+    return element.dataset.sortValue
+  }
+  for (const child of element.children) {
+    const child_val = get_html_sort_value(child)
+    if (child_val !== ``) {
+      return child_val
+    }
+  }
+  return element.textContent ?? ``
+}
+
 export function sortable(
   node: HTMLElement,
-  { header_selector = `thead th` } = {},
+  {
+    header_selector = `thead th`,
+    asc_class = `table-sort-asc`,
+    desc_class = `table-sort-desc`,
+    sorted_style = { backgroundColor: `rgba(255, 255, 255, 0.1)` },
+  } = {},
 ) {
   // this action can be applied to bob-standard HTML tables to make them sortable by
   // clicking on column headers (and clicking again to toggle sorting direction)
@@ -9,37 +27,50 @@ export function sortable(
   let sort_dir = 1 // 1 = asc, -1 = desc
 
   for (const [idx, header] of headers.entries()) {
-    // add cursor pointer to headers
-    header.style.cursor = `pointer`
-    const init_bg = header.style.backgroundColor
+    header.style.cursor = `pointer` // add cursor pointer to headers
+
+    const init_styles = { ...header.style }
     header.addEventListener(`click`, () => {
+      // reset all headers to initial state
       for (const header of headers) {
-        // removing any existing arrows
-        header.textContent = header.textContent?.replace(/ ↑| ↓/, ``)
-        header.classList.remove(`asc`, `desc`)
-        header.style.backgroundColor = init_bg
+        header.textContent = header.textContent?.replace(/ ↑| ↓/, ``) ?? ``
+        header.classList.remove(asc_class, desc_class)
+        Object.assign(header.style, init_styles)
       }
-      header.classList.toggle(sort_dir > 0 ? `asc` : `desc`)
-      header.style.backgroundColor = `rgba(255, 255, 255, 0.1)`
-      // append arrow to header text
       if (idx === sort_col_idx) {
         sort_dir *= -1 // reverse sort direction
       } else {
         sort_col_idx = idx // set new sort column index
         sort_dir = 1 // reset sort direction
       }
-      header.innerHTML = `${header.textContent} ${sort_dir > 0 ? `↑` : `↓`}`
+      header.classList.add(sort_dir > 0 ? asc_class : desc_class)
+      Object.assign(header.style, sorted_style)
+      header.textContent = `${header.textContent?.replace(/ ↑| ↓/, ``)} ${sort_dir > 0 ? `↑` : `↓`}`
+
       const table_body = node.querySelector(`tbody`)
       if (!table_body) return
 
       // re-sort table
       const rows = Array.from(table_body.querySelectorAll(`tr`))
       rows.sort((row_1, row_2) => {
-        const val_1 = row_1.cells[sort_col_idx].textContent ?? ``
-        const val_2 = row_2.cells[sort_col_idx].textContent ?? ``
-        return (
-          sort_dir * val_1.localeCompare(val_2, undefined, { numeric: true })
-        )
+        const cell_1 = row_1.cells[sort_col_idx]
+        const cell_2 = row_2.cells[sort_col_idx]
+        const val_1 = get_html_sort_value(cell_1)
+        const val_2 = get_html_sort_value(cell_2)
+
+        if (val_1 === val_2) return 0
+        if (val_1 === ``) return 1 // treat empty string as lower than any value
+        if (val_2 === ``) return -1 // any value is considered higher than empty string
+
+        const num_1 = Number(val_1)
+        const num_2 = Number(val_2)
+
+        if (isNaN(num_1) && isNaN(num_2)) {
+          return (
+            sort_dir * val_1.localeCompare(val_2, undefined, { numeric: true })
+          )
+        }
+        return sort_dir * (num_1 - num_2)
       })
 
       for (const row of rows) table_body.appendChild(row)

tests/actions/highlight-matches.test.ts

@@ -0,0 +1,87 @@
+import { highlight_matches } from '$lib/actions'
+import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'
+
+const mockHighlightsSet = vi.fn()
+const mockHighlightsClear = vi.fn()
+global.CSS = {
+  highlights: { set: mockHighlightsSet, clear: mockHighlightsClear },
+} as unknown
+
+const RangeMock = vi
+  .fn()
+  .mockImplementation(() => ({ setStart: vi.fn(), setEnd: vi.fn() }))
+global.Range = RangeMock as unknown
+global.Highlight = class {} as unknown
+
+describe(`highlight_matches`, () => {
+  let testNode: HTMLElement
+
+  beforeEach(() => {
+    testNode = document.createElement(`div`)
+    testNode.innerHTML = `<p>This is a test paragraph with some text to highlight.</p>`
+    document.body.appendChild(testNode)
+  })
+
+  afterEach(() => {
+    document.body.removeChild(testNode)
+    vi.clearAllMocks()
+  })
+
+  test.each([
+    [`applies highlights correctly`, { query: `test` }, true, 1],
+    [
+      `does not apply highlights when disabled`,
+      { query: `test`, disabled: true },
+      false,
+      0,
+    ],
+    [
+      `uses custom CSS class`,
+      { query: `test`, css_class: `custom-highlight` },
+      true,
+      1,
+      `custom-highlight`,
+    ],
+    [`handles empty query`, { query: `` }, false, 0],
+  ])(
+    `%s`,
+    (_, options, shouldSet, setCallCount, cssClass = `highlight-match`) => {
+      highlight_matches(testNode, options)
+      expect(mockHighlightsClear).toHaveBeenCalled()
+      if (shouldSet) {
+        expect(mockHighlightsSet).toHaveBeenCalledWith(
+          cssClass,
+          expect.any(Highlight),
+        )
+      } else expect(mockHighlightsSet).not.toHaveBeenCalled()
+      expect(mockHighlightsSet).toHaveBeenCalledTimes(setCallCount)
+    },
+  )
+
+  test(`handles node_filter`, () => {
+    const nodeFilter = vi.fn().mockReturnValue(NodeFilter.FILTER_ACCEPT)
+    highlight_matches(testNode, { query: `test`, node_filter: nodeFilter })
+    expect(nodeFilter).toHaveBeenCalled()
+    expect(mockHighlightsSet).toHaveBeenCalled()
+  })
+
+  test(`update function works correctly`, () => {
+    const action = highlight_matches(testNode, { query: `test` })
+    action.update({ query: `paragraph` })
+    expect(mockHighlightsClear).toHaveBeenCalledTimes(2)
+    expect(mockHighlightsSet).toHaveBeenCalledTimes(2)
+  })
+
+  test.each([
+    [`handles multiple occurrences`, `<p>Test test test</p>`, 3],
+    [`is case insensitive`, `<p>Test TEST test</p>`, 3],
+  ])(`%s`, (_, innerHTML, expectedRanges) => {
+    testNode.innerHTML = innerHTML
+    highlight_matches(testNode, { query: `test` })
+    expect(mockHighlightsSet).toHaveBeenCalledWith(
+      `highlight-match`,
+      expect.any(Highlight),
+    )
+    expect(RangeMock).toHaveBeenCalledTimes(expectedRanges)
+  })
+})

tests/actions/sortable.test.ts

@@ -0,0 +1,161 @@
+import { get_html_sort_value, sortable } from '$lib/actions'
+import { afterEach, beforeEach, describe, expect, test } from 'vitest'
+
+function make_table(rows: string[][]): HTMLTableElement {
+  const table = document.createElement(`table`)
+  table.innerHTML = `
+    <thead>
+      <tr>${rows[0].map((cell) => `<th>${cell}</th>`).join(``)}</tr>
+    </thead>
+    <tbody>
+      ${rows
+        .slice(1)
+        .map(
+          (row) => `
+        <tr>${row.map((cell) => `<td>${cell}</td>`).join(``)}</tr>
+      `,
+        )
+        .join(``)}
+    </tbody>
+  `
+  return table
+}
+
+function get_sorted_values(
+  table: HTMLTableElement,
+  column_index: number,
+): string[] {
+  return Array.from(table.querySelectorAll(`tbody tr`)).map(
+    (row) => row.cells[column_index].textContent || ``,
+  )
+}
+
+function click_header(table: HTMLTableElement, column_index: number) {
+  const header = table.querySelector(
+    `th:nth-child(${column_index + 1})`,
+  ) as HTMLTableCellElement
+  header.click()
+}
+
+let table: HTMLTableElement
+
+beforeEach(() => {
+  table = make_table([
+    [`Name`, `Age`, `Score`],
+    [`Alice`, `30`, `95`],
+    [`Bob`, `25`, `88`],
+    [`Charlie`, `35`, `92`],
+  ])
+  document.body.appendChild(table)
+})
+
+afterEach(() => {
+  document.body.removeChild(table)
+})
+
+describe(`get_html_sort_value`, () => {
+  test.each([
+    [`<span data-sort-value="10">Ten</span>`, `10`],
+    [`<span>No sort value</span>`, `No sort value`],
+    [`<span><em data-sort-value="20">Twenty</em></span>`, `20`],
+    [``, ``],
+  ])(`returns correct value for %s`, (html, expected) => {
+    const element = document.createElement(`div`)
+    element.innerHTML = html
+    expect(get_html_sort_value(element)).toBe(expected)
+  })
+})
+
+describe(`sortable function`, () => {
+  test(`sorts table correctly`, () => {
+    sortable(table)
+
+    click_header(table, 1)
+    expect(get_sorted_values(table, 1)).toEqual([`25`, `30`, `35`])
+
+    click_header(table, 1)
+    expect(get_sorted_values(table, 1)).toEqual([`35`, `30`, `25`])
+  })
+
+  test(`handles empty values correctly`, () => {
+    table = make_table([
+      [`Name`, `Age`],
+      [`Alice`, `30`],
+      [`Bob`, ``],
+      [`Charlie`, `25`],
+    ])
+    document.body.replaceChild(table, document.body.firstChild as Node)
+
+    sortable(table)
+    click_header(table, 1)
+
+    expect(get_sorted_values(table, 1)).toEqual([`25`, `30`, ``])
+  })
+
+  test(`handles data-sort-value attribute`, () => {
+    table = make_table([
+      [`Name`, `Score`],
+      [`Alice`, `<span data-sort-value="95">A</span>`],
+      [`Bob`, `<span data-sort-value="88">B</span>`],
+      [`Charlie`, `<span data-sort-value="92">A-</span>`],
+    ])
+    document.body.replaceChild(table, document.body.firstChild as Node)
+
+    sortable(table)
+    click_header(table, 1)
+
+    expect(get_sorted_values(table, 1)).toEqual([`B`, `A-`, `A`])
+  })
+
+  test(`handles zero values correctly`, () => {
+    table = make_table([
+      [`Name`, `Score`],
+      [`Alice`, `0`],
+      [`Bob`, `10`],
+      [`Charlie`, ``],
+    ])
+    document.body.replaceChild(table, document.body.firstChild as Node)
+
+    sortable(table)
+    click_header(table, 1)
+
+    expect(get_sorted_values(table, 1)).toEqual([`0`, `10`, ``])
+  })
+
+  test(`updates header styles correctly`, () => {
+    sortable(table)
+    const header = table.querySelector(
+      `th:nth-child(2)`,
+    ) as HTMLTableCellElement
+
+    header.click()
+    expect(header.classList.contains(`table-sort-asc`)).toBe(true)
+    expect(header.style.backgroundColor).toBe(`rgba(255, 255, 255, 0.1)`)
+    expect(header.textContent).toContain(`↑`)
+
+    header.click()
+    expect(header.classList.contains(`table-sort-desc`)).toBe(true)
+    expect(header.textContent).toContain(`↓`)
+  })
+
+  test(`applies custom classes and styles`, () => {
+    sortable(table, {
+      asc_class: `custom-asc`,
+      desc_class: `custom-desc`,
+      sorted_style: { color: `red`, fontWeight: `bold` },
+    })
+    const header = table.querySelector(
+      `th:nth-child(2)`,
+    ) as HTMLTableCellElement
+
+    header.click()
+    expect(header.classList.contains(`custom-asc`)).toBe(true)
+    expect(header.style.color).toBe(`red`)
+    expect(header.style.fontWeight).toBe(`bold`)
+
+    header.click()
+    expect(header.classList.contains(`custom-desc`)).toBe(true)
+    expect(header.style.color).toBe(`red`)
+    expect(header.style.fontWeight).toBe(`bold`)
+  })
+})