Skip to content

πŸ› οΈ Refactor Sphinx-Needs DataTables integration to use GridJS - #1464

Open
iSOLveIT wants to merge 3 commits into
masterfrom
datatables_fix
Open

πŸ› οΈ Refactor Sphinx-Needs DataTables integration to use GridJS#1464
iSOLveIT wants to merge 3 commits into
masterfrom
datatables_fix

Conversation

@iSOLveIT

@iSOLveIT iSOLveIT commented Jun 21, 2025

Copy link
Copy Markdown
Contributor

Changes for issue #122:

  • Removed Data tables and all its dependencies
  • Use GridJS to handle data table integration
  • Fix some testcases
  • Fix data table CSS to support both light and dark modes

Motivation for this change:

Our used DataTables version is quite old (>~4 years) and is still based on jQuery, which was also removed from Sphinx some long time ago.
Also, the JS world has changed, docs get presented in React/Angular/Svelte apps, in which DataTables is causing some problems or at least is adding some unnecessary dependencies (jQuery).
Therefore, this PR cleans up our used JS environment and feels more modern now.

@iSOLveIT iSOLveIT self-assigned this Jun 21, 2025
@iSOLveIT
iSOLveIT requested a review from danwos June 25, 2025 09:22

@danwos danwos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks already promising.
After building the official docs I found some problems:

  • Pagination works fine, but we need an option how much elements shall be shown: 10, 25, 50, All.
    Otherwise dealing with big tables will not be so comfortable.

  • The sticky head is a nice idea.
    But it is confusing, as the header is transparent so that it shows row data as well:
    image

  • If possible, let's remove the icons, as they may not match a specific theme and their look-alike depends on os/browser:
    image

@danwos

danwos commented Jun 26, 2025

Copy link
Copy Markdown
Member

Here is a comparison of both.
I guess the following could be changed:

  • No border around the table-element (incl. the buttons)
  • Buttons less prominent: Less rounded corners, a little smaller (size & font)
  • Pagination maybe also without border, or a more transparent border color.

The goal is to have the table data itself quite prominent, but all buttons and co. in a less prominent style.

But that's it, the diffs are already not so much.
Good work.

image
image

@danwos
danwos requested a review from chrisjsewell June 26, 2025 06:47
@danwos

danwos commented Jun 26, 2025

Copy link
Copy Markdown
Member

@chrisjsewell what do you think?
This PR is also some kind of a clean-up, as JS libs are reduced.

@danwos

danwos commented Jun 26, 2025

Copy link
Copy Markdown
Member

Another finding: Row style seems not to work

image
Source: https://sphinx-needs.readthedocs.io/en/latest/directives/needtable.html#style-row

image
Source: local build via DOCS_THEME=furo sphinx-build -a -E docs docs/_build/html

…ination limit selector, and enhance language settings
@chrisjsewell chrisjsewell added the pkg: sphinx-needs The sphinx-needs distribution (packages/sphinx-needs): its code, tests and docs label Sep 4, 2026
chrisjsewell added a commit that referenced this pull request Sep 8, 2026
…ancer (#1920)

A `needtable` with the default `:style: datatables` is still interactive
β€” sort a column, search the whole table, page through it, choose how
many rows a page holds, switch columns off, copy it or download it as
CSV β€” but it is now Sphinx-Needs' own script instead of a vendored copy
of DataTables 1.10.16 from 2018, and it only loads on pages that
actually have a table. The enhancement happens **in place**: every
`<tr>` and `<td>` Sphinx-Needs rendered is still the same element
afterwards, so `:style_row:` classes, the `needs_<column>` cell classes,
the links inside cells and the `<colgroup>` all survive sorting and
searching, and a reader with JavaScript off still gets the whole table.
The script is deferred, so it never blocks the parser.

**4.72 MB of vendored JavaScript in 34 files becomes 58.5 KB in three; a
page with a table loads 55.9 KB (14.8 KB gzipped) instead of 2.29 MB
(997 KB gzipped), and `search.html`, `genindex.html` and every page
without a table now load none of it** (closes #462 for the table
assets).

### Why not Grid.js, and why not DataTables 3

Grid.js β€” what #1464 proposed β€” reads `td.innerHTML` into a
`string[][]`, hides the source table and offers one class string for
*all* rows, so `:style_row:` cannot survive it by construction; it has
had no release since March 2024. DataTables 3 is jQuery-free, in-place
and MIT, and was the honest fallback at about 67 KB gzipped; it was not
chosen because it brings its own control DOM and CSS with fourteen
`!important` rules that every host theme has to fight, it is a
six-week-old major from a one-person project, and any third-party bundle
needs a build toolchain and a manifest in this repository that a
hand-written file does not. The full comparison β€” simple-datatables
(LGPL-3.0), Tabulator, List.js, tablesort, TanStack β€” is the decision
record at the top of
`packages/sphinx-needs/design/needstable-contract.md`.

The claims in this description were re-measured in review on two
independent corpora: 41.1 ms and 40.2 ms to initialise ten thousand rows
against our 41.5 ms, and 10 000 of 10 000 rows, 60 000 of 60 000 cells
and 6 of 6 `<col>` elements still the same DOM objects after five sort,
filter and destroy cycles.

### The markup is now a contract

`<table>` keeps `NEEDS_DATATABLES` as its hook and gains
`data-needstable-*` options; every `<th>` gains `scope="col"`, a
`data-col` column key and, where Sphinx-Needs knows it, a `data-type`;
every `<tr>` gains `data-need-id`, and a part row `data-parent`.
`:show_filters:` now writes its paragraph *after* the table instead of
between `</tbody>` and `</table>`, which was invalid HTML. The design
document specifies all of it β€” the markup, the script's API and
behaviours, the CSS custom properties a host answers, and the vendoring
rule β€” so that any producer of needtable markup can emit against it.
ubCode, useblocks' Rust-based tooling for needs projects, renders
needtables from the same sources and will ship this asset unchanged
against the same markup contract.

The script and its structural stylesheet,
`src/sphinx_needs/libs/html/needstable.{js,css}`, are plain ES2020
JavaScript type-checked through JSDoc (`// @ts-check`; the one-command
`tsc` recipe is in the design document) and host-agnostic CSS that asks
for every colour through a `--needstable-*` custom property.
Sphinx-Needs' own theme sheets answer those properties; a consumer
without them gets `currentColor` fallbacks.

### Two visible behaviour changes

The count under the table reads *Showing 1–10 of 42* and counts
**needs**, not rows: a need and its parts are one entry, so a page never
splits a need from its parts, and a table with `:show_parts:` reports a
smaller total than DataTables did. And a cell with no value now sorts
**last** whichever way the column points, where DataTables sorted blanks
first.

### Removed, and what replaces them

- **Excel and PDF export** β†’ the CSV download (UTF-8 with a BOM, so
Excel opens it directly) and the browser's own print command. They were
about 900 KB of the payload for two buttons.
- **Column reordering** β†’ nothing. It was switched on, undocumented, and
no issue ever mentioned it.
- **`.dataTables_*` / `.dt-button` CSS hooks** β†’ the `needstable-*`
classes. A project that styled the old chrome has to re-point at the new
one; a project that styled `.NEEDS_DATATABLES`, `.need`, `.need_part` or
`needs_<column>` is unaffected.
- **The `--sn-color-datatable-*` theme variables** β†’
`--sn-color-table-{label,btn-border}`.

Upgrading needs nothing else: this release changes four of the
extension's own asset files, and they are now re-copied over a build
directory that still holds the old ones. Sphinx would otherwise leave
the stale stylesheet in place with a `copy_overwrite` warning β€” and fail
a `-W` build β€” on the first incremental build after the upgrade.

### Gained

- a page-size control that includes *All* (a review finding on #1464),
configurable through `needs_table_page_size`, `needs_table_page_sizes`
and a `:page_size:` option;
- typed sorting that returns to the server's `:sort:` order on the third
click, so the documented "sorting only works with `:style: table`"
caveat is gone;
- part rows that travel with their need through sorting and filtering;
- `aria-sort` and keyboard-operable headers, an `aria-live` result
count, focus that stays inside the pager when you page with the
keyboard;
- a scroll frame of the widget's own, so a wide table scrolls inside it
instead of escaping the page, and a table that fits fills its column;
- the columns control is a plain button in every theme we render, rather
than whatever the theme does to a `<summary>`;
- twenty-one browser tests and a markup-contract test module where there
were none.

jQuery is still a dependency, because `sphinx_needs_collapse.js` (the
need meta box, not the table) is still jQuery β€” porting it is the
follow-up that lets `sphinxcontrib-jquery` go.

### Render pass

The docs were built with furo, sphinx_rtd_theme, pydata-sphinx-theme,
sphinx-immaterial and alabaster, in light and dark where the theme has
one, before and after, with a new `poe docs-needs-shots` instrument (its
own PR follows). The Read the Docs preview of this PR shows the furo
build live.

### Changelog draft

```rst
* :ref:`✨ Improvement <needtable>`: The ``datatables`` table style is now Sphinx-Needs' own,
  dependency-free enhancer instead of the bundled `DataTables <https://datatables.net>`__
  package. It sorts (typed, and back to ``:sort:`` order on the third click), searches, pages
  with a 10/25/50/All control, hides columns, copies and downloads CSV -- and it enhances the
  rendered table in place, so ``:style_row:``, the ``needs_<column>`` cell classes and the
  links inside cells survive every interaction. Need parts stay with their need. New:
  :ref:`needs_table_page_size`, :ref:`needs_table_page_sizes` and
  :ref:`page_size <needtable_page_size>`. (:pr:`1920`)
* :ref:`✨ Improvement <needtable>`: The table's script and stylesheet are added only to pages
  that contain a needtable; ``search.html`` and ``genindex.html`` no longer carry them.
  4.72 MB of vendored JavaScript becomes 58.5 KB. (:issue:`462`, :pr:`1920`)
* :ref:`πŸ‘Œ Improvement <needtable>`: The interactive table's count reads *Showing 1-10 of
  42* and counts **needs**: a need and its :ref:`parts <needtable_show_parts>` are one
  entry, so a page never splits a need from its parts. A table with ``:show_parts:``
  therefore reports a smaller total than the DataTables style did, which counted rows.
  Cells with no value now sort last whichever way the column points, where DataTables
  sorted them first. (:pr:`1920`)
* πŸ’₯ Removed: Excel and PDF export, and column reordering, from the ``datatables`` style. The
  CSV download opens in Excel and browsers print; column reordering was undocumented.
  Projects that styled ``.dataTables_*`` or ``.dt-button`` must re-point at the
  ``needstable-*`` classes, and ``--sn-color-datatable-*`` is now ``--sn-color-table-*``.
  (:pr:`1920`)
```

Closes #462.

### Related issues, and what this changes for each

- **#1464** β€” superseded by this PR; the size measurement, the deletion
of the dark-theme DataTables block and the 10/25/50/All requirement were
kept from it.
- **#408** (per-table DataTables configuration) β€” the requested shape, a
dict of raw DataTables options, no longer applies. Per-table settings
now travel as `data-needstable-*` attributes emitted from directive
options and config (page size and page sizes ship here); more knobs
follow the same route. Note that the side extension posted there in
August reconfigures DataTables and stops working with this change, and
PDF export is replaced by CSV plus the browser's print dialog.
- **#1425** (named table layouts) β€” orthogonal server-side presets;
nothing in the contract stands in their way. Follow-up.
- **#771** (Read the Docs theme) β€” the collision was between DataTables'
wrapper and the theme's own table wrapper; the DataTables wrapper is
gone and the widget's scroll frame is the container, with the theme's
wrapper nesting inside it. Measured correct on the docs build; a plain
project on that theme still wants confirmation, because these docs carry
a small RTD override of their own.
- **#1144** (order of appearance) β€” `:sort: lineno` already answers it,
but only worked with `:style: table` because DataTables re-sorted at
load. The interactive table now keeps the server's `:sort:` order as its
initial and "unsorted" state, so it works in the default style too.
Multiple `:sort:` keys remain a follow-up.

### In short: issues and follow-ups

| | after this PR |
|---|---|
| #462 | closed for the table assets; the shared host stylesheet is
still global β€” follow-up |
| #1464 | superseded; close with the measurement that Grid.js re-renders
from cell text |
| #1144 | `:sort: lineno` now works in the default style; close after
release, multi-key `:sort:` stays open |
| #771 | DataTables' wrapper gone, frame + theme wrapper nest cleanly on
the docs build; confirm on a plain RTD project, then close |
| #408 | answered by `data-needstable-*` attributes rather than a
DataTables option dict; the August side extension there stops working β€”
needs a heads-up |
| #1425 | orthogonal server-side presets; unaffected, follow-up |

Follow-up work, none blocking: port `sphinx_needs_collapse.js` off
jQuery and drop `sphinxcontrib-jquery`; gate the shared host stylesheet
per page; type-check the shared asset in CI; a sticky header now that
the frame is an explicit box; a caption-derived CSV file name; logical
instead of physical padding for the sort glyph under RTL; and, on
request, column reorder, multi-column sort, responsive collapse, URL
state or translation catalogues inside the script.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: sphinx-needs The sphinx-needs distribution (packages/sphinx-needs): its code, tests and docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants