Skip to content

refactor(cms): sortable tables via List.js#1169

Merged
wesleyboar merged 18 commits into
mainfrom
refactor/sortableTable
Jun 8, 2026
Merged

refactor(cms): sortable tables via List.js#1169
wesleyboar merged 18 commits into
mainfrom
refactor/sortableTable

Conversation

@wesleyboar

@wesleyboar wesleyboar commented Jun 4, 2026

Copy link
Copy Markdown
Member

Overview

Sortable CMS tables use List.js 2.3.1 instead of custom sort logic. Editors use js-sortable on the table and not-sortable on non-sort columns; row order stays as authored until a header is clicked.

Related

Changes

  • added List.js 2.3.1 from jsDelivr in delayed assets (SRI)
  • refactored sortableTable.js for List prep, row data-*, aria-sort sync
  • updated sortableTable.css for js-sortable and .sort controls

Testing

  1. Deploy or load delayed assets with List.js + sortableTable on #cms-content.
  2. Use a table with o-fixed-header-table js-sortable and th.not-sortable on Description.
  3. On load: CMS row order, all aria-sort="none".
    ⚠️ CMS strips data-attribute.
  4. Click headers: sort toggles; other columns reset to none.
  5. Column with link in cell sorts on link text.
  6. Add <fieldset> with filter UI (see tmp/pr-1169.html).
  7. Verify filtering works.

UI

It.s.alive.mov

Replace custom sort with List.js 2.3.1 on js-sortable tables; editors use
not-sortable on th. No default sort on load; sync aria-sort from List state.

Co-authored-by: Cursor <cursoragent@cursor.com>
@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Refactor CMS tables to use List.js for sorting

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Replace custom sort logic with List.js 2.3.1 library
• Update table markup from is-sortable to js-sortable
• Implement dynamic row data-* attributes for List.js
• Sync aria-sort state from List.js button classes
• Add fallback sort indicators via List.js asc/desc classes
Diagram
flowchart LR
  A["Custom Sort Logic"] -->|"Replace with"| B["List.js 2.3.1"]
  C["Table Markup<br/>is-sortable"] -->|"Update to"| D["js-sortable"]
  E["Manual Sort<br/>on Click"] -->|"Delegate to"| F["List.js Events"]
  F -->|"Sync aria-sort"| G["Accessibility State"]
  B -->|"Generate"| H["Row data-* Attributes"]

Loading

Grey Divider

File Changes

1. taccsite_cms/templates/assets_core_delayed.html Dependencies +5/-0

Add List.js 2.3.1 library from CDN

• Add List.js 2.3.1 CDN script with SRI integrity hash
• Load library after jQuery and Bootstrap dependencies
• Position before sortableTable module import

taccsite_cms/templates/assets_core_delayed.html


2. taccsite_cms/static/site_cms/css/modules/sortableTable.css ✨ Enhancement +19/-8

Update styles for List.js class names

• Rename selector from is-sortable to js-sortable
• Update button class from is-sortable__sort to sort
• Add fallback styles for List.js asc/desc button classes
• Update comment to reflect List.js as primary sort indicator source

taccsite_cms/static/site_cms/css/modules/sortableTable.css


3. taccsite_cms/static/site_cms/js/modules/sortableTable.js ✨ Enhancement +146/-83

Refactor to integrate List.js sorting library

• Replace custom sort implementation with List.js integration
• Add slugify() and assignSlug() functions for column data attributes
• Implement applyRowDataAttributes() to populate data-* on rows
• Add syncAriaFromListButtons() to sync aria-sort from List.js state
• Rename initSortableTable() to prepSortableTable() and refactor for List.js
• Add List.js availability check with error logging
• Remove default sort on load; preserve authored row order until user interaction
• Update class names and selectors throughout

taccsite_cms/static/site_cms/js/modules/sortableTable.js


Grey Divider

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Jun 4, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (1) 🎨 UX issues (0)

Grey Divider


Action required

1. sortableTable.js still exists 📎 Requirement gap ⚙ Maintainability
Description
The PR keeps and updates the internal taccsite_cms/static/site_cms/js/modules/sortableTable.js
module instead of removing it, so the codebase still includes the custom sortableTable.js
implementation footprint. This fails the requirement to fully replace it with a third-party sorting
solution without retaining sortableTable.js in the repository.
Code

taccsite_cms/static/site_cms/js/modules/sortableTable.js[R236-248]

+  if (typeof window.List !== 'function') {
+    if (!listJsMissingLogged) {
+      listJsMissingLogged = true;
+      console.error(
+        '[sortableTable] List.js is not loaded; sortable tables will not be enhanced.'
+      );
+    }
+    return;
+  }
+
scopeElement.querySelectorAll(tableSelector).forEach((table) => {
  if (table instanceof HTMLTableElement) {
-      initSortableTable(table, notSortableSelector, buttonClass);
+      prepSortableTable(table, notSortableSelector, buttonClass);
Evidence
PR Compliance ID 1 requires that the codebase no longer includes or uses sortableTable.js. The
diff shows new code added to sortableTable.js (including runtime checks for window.List and
continued export/initialization logic), demonstrating the file remains part of the shipped
implementation.

Replace internal sortableTable.js with a third-party table sorting solution
taccsite_cms/static/site_cms/js/modules/sortableTable.js[236-248]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Compliance requires removing the internal `sortableTable.js` file (introduced in #1167) entirely; this PR still modifies and ships it, so the repository still includes `sortableTable.js`.
## Issue Context
Even though List.js is added, the checklist success criteria/failure criteria explicitly require that `sortableTable.js` no longer be present/used.
## Fix Focus Areas
- taccsite_cms/static/site_cms/js/modules/sortableTable.js[236-248]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Header markup removed 🐞 Bug ≡ Correctness
Description
prepSortableTable() wipes each sortable <th> via cell.innerHTML='' and reconstructs the header using
only textContent, permanently removing any CMS-authored header markup (e.g., <abbr>, icons, sr-only
spans). This can silently break accessibility/UX content that editors included in table headers.
Code

taccsite_cms/static/site_cms/js/modules/sortableTable.js[R181-196]

+    const label = (cell.textContent ?? '').trim();
+    if (!label) {
+      return;
+    }
+
+    const slug = assignSlug(slugify(label), usedSlugs);
  cell.dataset.sortLabel = label;
  cell.innerHTML = '';
  const button = document.createElement('button');
  button.type = 'button';
-    button.className = buttonClass + ' ' + SORT_BUTTON_CLASS;
+    button.className = [ buttonClass, SORT_BUTTON_CLASS ].filter(Boolean).join(' ');
  button.textContent = label;
+    button.setAttribute('data-sort', slug);
  cell.append(button);
-    button.addEventListener('click', () => {
-      const current = cell.getAttribute('aria-sort');
-      const next =
-        current === 'ascending' ? 'descending' : 'ascending';
Evidence
The implementation explicitly clears the header cell’s HTML and reconstructs only a new button with
the extracted text label, which necessarily removes any original header markup/content beyond plain
text.

taccsite_cms/static/site_cms/js/modules/sortableTable.js[181-196]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`prepSortableTable()` clears `th` contents (`cell.innerHTML = ''`) and recreates a plain-text button. This drops any existing header DOM (e.g. `<abbr>`, `<span class="sr-only">`, icons), which is both an accessibility regression and a CMS content regression.
### Issue Context
The code currently derives the label from `cell.textContent`, then clears the header cell and inserts a new `<button>`.
### Fix Focus Areas
- taccsite_cms/static/site_cms/js/modules/sortableTable.js[181-199]
### Suggested fix approach
- Create the `<button>` first, and **move** existing child nodes from the `<th>` into the button instead of deleting them.
- Example pattern:
 - `const button = document.createElement('button')`
 - `while (cell.firstChild) button.append(cell.firstChild)`
 - `cell.append(button)`
- Compute the label/slug from `button.textContent` (after moving nodes) or from the original `cell.textContent` before moving nodes.
- Keep setting `cell.dataset.sortLabel = label` and `button.setAttribute('data-sort', slug)` as today.
- Ensure the final DOM remains `th > button.sort` for List.js and CSS selectors.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Blank headers become unsortable ✓ Resolved 🐞 Bug ≡ Correctness
Description
prepSortableTable() now returns early when a header’s trimmed textContent is empty, so
icon-only/symbol-only headers will never receive a sort button unless the editor adds literal text.
This is a behavior change that can make intended sortable columns inert without warning.
Code

taccsite_cms/static/site_cms/js/modules/sortableTable.js[R181-184]

+    const label = (cell.textContent ?? '').trim();
+    if (!label) {
+      return;
+    }
Evidence
The code explicitly skips generating a sort button for headers whose extracted text label is empty,
so those columns are excluded from columns/valueNames and cannot be sorted.

taccsite_cms/static/site_cms/js/modules/sortableTable.js[181-184]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Sortable columns are skipped when `label` is empty (`if (!label) return;`). This makes columns with non-text header content (e.g., icon-only) unsortable.
### Issue Context
The previous implementation used a fallback label (`Column N`). The new implementation drops that fallback and skips.
### Fix Focus Areas
- taccsite_cms/static/site_cms/js/modules/sortableTable.js[173-186]
### Suggested fix approach
- Replace the early-return with a safe fallback label, e.g.:
- `const label = (cell.textContent ?? '').trim() || `Column ${columnIndex + 1}`;`
- Optionally emit a `console.warn` when falling back, so editors can fix the header label.
- Ensure the fallback label is used for:
- `cell.dataset.sortLabel`
- `slugify/assignSlug`
- `button.textContent` (or preserve markup per the other fix).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread taccsite_cms/static/site_cms/js/modules/sortableTable.js Outdated
Comment thread taccsite_cms/static/site_cms/js/modules/sortableTable.js
wesleyboar and others added 11 commits June 4, 2026 16:57
Use col-N keys for List.js valueNames; aria-sort indicators on th button.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
- Added CSS styles for sortable filters and improved layout for filter elements.
- Updated JavaScript to support filtering functionality, including result count display and event handling for filter inputs.
- Refactored `prepSortableTable` to wire up filters and result totals for better user experience.

Co-authored-by: Cursor <cursoragent@cursor.com>
wesleyboar added a commit to TACC/Core-Styles that referenced this pull request Jun 8, 2026
## Overview

ES module to overcome Core-CMS WYSIWYG deleting `aria-*` attributes.

## Related

- supports testing TACC/tup-ui#558

## Changes

- **adds** script

## Testing

Test TACC/Core-CMS#1169 still works via
TACC/tup-ui#558 using new module hack.

```diff
<script type="module">
+ import promoteDataAriaAttributes from 'https://cdn.jsdelivr.net/gh/TACC/Core-Styles@feat/promote-data-aria-attributes/src/lib/_utils/js/promote-data-aria-attributes.js';
  import sortableTable from 'https://cdn.jsdelivr.net/gh/TACC/Core-CMS@58e0875e/taccsite_cms/static/site_cms/js/modules/sortableTable.js';

  const scopeElement = document.getElementById('cms-content');

+ promoteDataAriaAttributes(scopeElement);
  sortableTable({
    scopeElement,
    buttonClass: 'btn btn-link',
  });
</script>
```

> [!TIP]
> Still works!

## UI

Skipped.

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
wesleyboar and others added 6 commits June 8, 2026 14:22
Move existing header children into the sort control instead of clearing
innerHTML. Resolves PR #1169 review on CMS abbr/sr-only header content.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@wesleyboar

Copy link
Copy Markdown
Member Author

QODO Issues resolved. It doesn't know it yet.

Improvements come (no <fieldset> boilerplate) in new PR.

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@wesleyboar wesleyboar merged commit 86ef632 into main Jun 8, 2026
@wesleyboar wesleyboar deleted the refactor/sortableTable branch June 8, 2026 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace sortableTable.js #1167 with 3rd Party Solution

1 participant