♻️ Replace DataTables with an in-place, dependency-free needtable enhancer - #1920
Merged
Conversation
chrisjsewell
added
enhancement
pkg: sphinx-needs
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1920 +/- ##
==========================================
+ Coverage 91.84% 92.03% +0.19%
==========================================
Files 105 105
Lines 16231 16279 +48
==========================================
+ Hits 14907 14983 +76
+ Misses 1324 1296 -28
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
chrisjsewell
marked this pull request as ready for review
September 8, 2026 16:51
chrisjsewell
force-pushed
the
feat/needstable-enhancer
branch
from
September 8, 2026 17:09
68f9ca8 to
8a18bde
Compare
Sphinx-Needs and ubCode both render needtables, and the client-side table enhancer that is about to replace DataTables has to work against both. This commit adds the markup half of the shared contract to Sphinx-Needs. The `<table>` keeps `NEEDS_DATATABLES` as its selector and gains the per-table options `data-needstable-page-size` / `-page-sizes` (still literals here; the configuration that feeds them lands with the options). Each `<th>` gains `scope="col"`, a `data-col` column key and, where the field schema says so, a `data-type`. Each `<tr>` gains `data-need-id`, and a part row `data-parent`, so that a sorter can keep parts under the need they belong to. The visible header text, the cell classes, the links and the `<colgroup>` are unchanged. `:show_filters:` now writes its paragraph AFTER the table instead of between `</tbody>` and `</table>`, which was invalid HTML that browsers hoist out again and that any enhancer trips on. Order is table, filter paragraph, max-items note. docutils' HTML writer serialises only what a visitor hands to `starttag()`, so the three pieces of markup are node sub-classes with HTML visitors that delegate to the base ones with the extra attributes injected. No other builder gets a visitor, so `SphinxTranslator.dispatch_visit` walks the MRO and latex, text, texinfo, man and singlehtml render them as the `table`/`row`/`entry` they derive from -- asserted for latex and singlehtml. ENV_DATA_VERSION 7 -> 8: the table is built at `doctree-resolved`, so without a bump a rebuild over an existing `_build` writes no page whose source did not change and leaves the old markup on disk.
`libs/html/needstable.js` and `libs/html/needstable.css` are the pair that replaces DataTables. The script sorts, searches, pages, hides columns and exports, and it does all of it by MOVING the rows the server rendered, never by re-rendering from extracted cell values -- which is what keeps `:style_row:` classes, the per-cell `needs_*` classes, the links inside cells and the `<colgroup>` alive through every interaction. It has no dependencies, uses no `innerHTML` and no `eval`, fetches nothing, and runs from `file://` and under a strict CSP. Two behaviours are worth naming because neither incumbent had them: a need and its `need_part` rows are ONE unit, so a sort keeps parts under their need and a filter that matches a part shows its need; and there is no sort at initialisation, so the server's `:sort:` order IS the unsorted state and the third click on a header restores it exactly. Only the current page's rows are attached to the document, as DataTables also did -- that is what keeps a very large table usable. `needstable.css` is the STRUCTURAL half and is host-agnostic: it names no theme and no `--sn-*` token, and takes colour only through `--needstable-*` custom properties with `currentColor`/`inherit` fallbacks. sphinx-needs maps its own tokens onto them in a later commit. `design/needstable-contract.md` is the specification both files implement. sphinx-needs is the repository of record; ubCode will vendor the pair byte-identical behind a sha256 fence. Ten browser cases in `tests/test_needstable_js.py` drive it over `file://` in the same shape as the collapse-button tests, against a new test project with twelve needs, parts, a numeric field, an ISO-date field, links, `:colwidths:`, `:show_filters:` and a second `:style: table` table the script must not touch.
4.72 MB and 33 files go: DataTables 1.10.16, Buttons, ColReorder, FixedColumns,
FixedHeader, Responsive, Scroller, JSZip, a second copy of pdfmake, a Flash
export shim, and the 376-byte loader that drove them. All of it landed in one
commit in 2018 and was never updated -- half of it was never even loaded, and
no dependency ecosystem could see any of it. What replaces it is 38 KB of
hand-written source, 10 KB gzipped, that ships with the package and is versioned
with it.
The CSS follows the same boundary the pair does. `css/common/needstable.css`
keeps the theme-neutral rules (and the doubled-class specificity trick, with the
comment explaining it), adds the scroll behaviour for the wrapped table and maps
sphinx-needs' tokens onto the pair's `--needstable-*` properties. The ~228 lines
of DataTables-selector overrides go: `dark.css`'s 100-line block (which carried
a "TODO this should not be part of the theme" of its own), the `.dataTables_*`
and `.dt-button` rules in `sphinx_immaterial.css`, and the `div.dataTables_wrapper`
margin in `sphinx_rtd_theme.css`.
`--sn-color-datatable-{label,btn-border}` become `--sn-color-table-*`, in the
three themes and in the four docs theme sheets; `--sn-color-datatable-body-bg`
goes entirely -- it existed to undo a DataTables row background and was
`transparent` in every theme.
`sphinx_rtd_theme.css` is kept rather than deleted: its other block,
`div.wy-table-responsive { width: 100% }`, repairs the RTD theme's own
table-width collision (#771) and has nothing to do with the table library.
jQuery stays a dependency: `sphinx_needs_collapse.js`, which is about the need
meta box and not about needtables, is still jQuery. Its comment in
`pyproject.toml` now says so.
Closes #462 for the table assets. Since 2022 every page of every sphinx-needs project has carried the table's script and stylesheet, `search.html` and `genindex.html` included -- pages that have no doctree at all, and until the previous commit that meant 2.26 MB of DataTables on each of them. `process_needtables` marks the resolved doctree when it emits an interactive table, and an `html-page-context` handler registers the pair for that page. The doctree the handler is given is the very object the directive processing mutated moments earlier, in the same process, for serial and parallel writes alike -- so no set of document names has to be stored in the environment, purged when a document changes, or merged across parallel readers, and an incremental build cannot serve a page whose marker went stale. Registration goes through the BUILDER rather than `app.add_js_file`: the application's route also appends to the extension registry, once per page, and a builder re-initialised afterwards would pick every one of them up globally -- which is the behaviour this handler exists to remove. `handle_page` resets the builder's asset lists just before it emits the event, so the addition reaches that page and no other. `common_css/needstable.css`, the host half of the sheet, is still global: it comes from the deterministic `common_css/*` loop, and gating that loop is the rest of #462.
`needs_table_page_size` (default 10) and `needs_table_page_sizes` (default [10, 25, 50, 0], where 0 means "All") set what the interactive table's page-size control offers and which of them it starts on; `:page_size:` overrides the first for one table. They reach the browser as `data-needstable-*` attributes on the `<table>`, which is the extension point the contract reserves for exactly this -- and the shape #408 and #1425 want, without a DataTables-shaped configuration dictionary. Both are validated: a page size is a positive whole number (0 means "all rows", which is not a page size), and the offered sizes are a non-empty list of whole numbers that are not negative. A bad configuration value is reported once, at `config-inited`, with the default it fell back to; a bad `:page_size:` is reported at the directive that carries it. Neither stops a build. The docs say what the interactive table does and, as importantly, what it no longer does: Excel and PDF export and column reordering are gone, and the CSV download and the browser's print command are the answers. The note claiming that `:sort:` only works with `:style: table` was true of DataTables, which re-sorted by column one on load; it is replaced by what is now true -- `:sort:` is the order the reader first sees in both styles, and the one a third click on a header returns to. `tutorial.rst` said `:style: datatable`, which is not a value; it fell back to the configured default and rendered as intended by accident.
Two things a future reader of `needstable.js` will want and could not get from
the code.
**Why this file exists at all.** `design/needstable-contract.md` gains a decision
record as its first section: the problem as it was measured (2.26 MB and jQuery
on every page, a 2018 vendoring no ecosystem could watch, half the tree
unreachable, ~228 lines of override CSS, no behavioural test), the property that
decided it (enhance the server-rendered DOM; a `string[][]` data model cannot
carry a row class, a cell class, a link or a `<colgroup>`), every alternative
that was looked at with its one-line measured reason, why one hand-written file
rather than a dependency, and what was given up in exchange.
**What the types are.** The script stays plain ES2020 JavaScript -- one artefact,
no build step, readable in the tree and in `view-source`, and vendored verbatim
-- but it now starts with `// @ts-check` and carries JSDoc types for the public
API and every non-trivial internal. One command checks them, with no
`package.json` and no node in CI:
npx -y -p typescript tsc --allowJs --checkJs --noEmit --strict \
--target es2020 --lib dom,es2020 \
packages/sphinx-needs/src/sphinx_needs/libs/html/needstable.js
It exits 0. The recipe and the reason not to write TypeScript source are in the
design document.
Typing it turned the constructor-function into a class and `var` into
`const`/`let`; the behaviour is unchanged and the ten browser cases still pass.
Two negative amounts join the browser test project, because `Intl.Collator` with
`numeric: true` already orders plain positive integers correctly -- it is only
wrong from "-5" against "-20" onwards. Without them the typed number comparator
was not actually load-bearing in the test.
…pty pager
Two defects review reproduced, each one line and each with the assertion that
would have caught it.
**The script was not deferred.** `install_needtable_assets` passed
`loading_method="defer"` to the BUILDER's `add_js_file`, but only
`Sphinx.add_js_file` translates that keyword; the builder writes every keyword
straight into the tag. So every needtable page of every project shipped
`<script loading_method="defer" src="…">` -- an attribute HTML does not have,
on a render-blocking script in `<head>`, which is the opposite of what was
asked for. It is now `defer="defer"`, asserted twice: in
`test_html_head_files`, and in `test_assets_are_registered_per_page`, which
unlike the former is not skipped on Windows. Both read the element now, not
just its `src`, which is why nothing could see this before.
**The pager's `hidden` attribute did nothing.** `nav.needstable-pager` carries
an author `display: flex`, which beats the user agent's
`[hidden] { display: none }` -- and that rule is the whole of what the attribute
does. Nothing leaked visually, because the element is also emptied, but it stayed
in the accessibility tree: a screen reader met an empty "Pagination" landmark on
every single-page table. One rule fixes it, and the browser test now asserts the
computed `display` rather than a zero bounding box, which passed either way.
Two committed sentences that described the deferred tag are corrected in place,
in the design document and in the browser test's fixture comment, each saying
what was claimed and what review measured.
…width back Review measured that making the `<table>` itself the horizontal scroll container costs it up to 22 % of the space it is given, and that the rule doing it is dead code in the one theme where scrolling actually matters. A `<table>` set to `display: block` is a block box whose rows are re-wrapped in an anonymous table box, and that box shrink-to-fits: `width: 100%` reaches the outer block and stops. Measured on a real docs build, one widget rendered 555 px inside a 708 px column, another 667 px, and `:colwidths:` percentages resolved against the shrunken width rather than the column. Before this branch the same tables were `display: table; width: 100%`, so this was a regression, not a trade-off. Separately, a host theme whose own script wraps every `<table>` in a `<div>` displaces `div.needstable > table` entirely, so the rule never matched there at all -- and where it did match, the docs' own `display: table !important` beat it and `overflow` does not apply to a `display: table` box anyway, so the scroll never armed. The widget now has a third child, `div.needstable-scroll`, between the control bar and the footer; the table lives inside it and keeps `display: table; width: 100%`. The controls are siblings of the frame, so they still do not scroll with the data -- which was the reason the wrapper was rejected as the scroll container, and it does not apply to an inner box. `destroy()` needed no change: it moves the table out and removes the wrapper, frame and all. Measured after the change, sphinx-immaterial (the theme with the `!important`): a table that fits is 654 px in a 654 px frame, and the wide one reports `scrollWidth` 1301 against `clientWidth` 654 -- it scrolls inside the widget instead of escaping it. The `!important` no longer fights anything, because the table is `display: table` by design now. A new browser case pins both halves: a table that fits fills its frame to within a pixel, and a deliberately wide one (a new `wide.rst`, ten columns of unbreakable text) scrolls inside the frame while the control bar keeps the wrapper's width. The design document's DOM diagram gains the box, and the sentence that named the table as the scroll container is corrected in place.
Putting the contract attributes on the `<table>`, the `<tr>`s and the `<th>`s means wrapping the HTML translator's `starttag` for one call. The `finally` clause removed the instance attribute unconditionally -- so if anything else had put one there, it was gone from the first needtable onwards, for the rest of the document, including on tables that have nothing to do with needs. "Anything else" includes the very technique this code is using. Review constructed it: a second extension that patches `starttag` on the translator instance marks every start tag it writes; on a page with a needtable in the middle, everything before the table carried the marker and everything after it did not. The failure is silent and produces inconsistent output rather than an exception, because the needtable's own wrapper had already captured the patched callable and delegated to it. It now reads what the instance carried before -- with a sentinel, because "no attribute" and "an attribute whose value is None" are different -- and puts exactly that back. The test is the repro: a project whose own second extension installs the patch from a `visit_` handler, and an assertion that the paragraphs on BOTH sides of the needtable carry its attribute, and that the needtable's own attributes came through that extension's wrapper at the same time.
`paintPager` rebuilds every button, so the button the reader just activated is removed from the document while it holds focus. Measured: after "Next" the active element is `<body>`, and a keyboard reader has to Tab from the top of the document again on every page change. The widget is otherwise carefully keyboard-operable -- real buttons, `aria-sort`, an `aria-live` count -- which is what made this the one place the care ran out. The header buttons are not rebuilt, so the existing keyboard test could not see it. Where focus is now read BEFORE the pager is emptied, and given back afterwards to the equivalent control: the same page number if it is still offered, else the nearest one; a previous/next that has become disabled on the new page hands focus to the other one, which is what happens when you page to the end. The keyboard test now activates "Next" from the keyboard and asserts focus is still inside the pager -- and, on the last page, that it landed on "Previous".
…rk page `needstable.css` asks for six colours through custom properties; the sphinx-needs host sheet answered three. The two it did not answer are the columns disclosure's, and their structural fallbacks are the system colours `Canvas`/`CanvasText`, which follow the USER AGENT's colour scheme rather than the page's. Measured in furo with the site's own dark switch on: the page is `rgb(19, 20, 22)` and the popover is `rgb(255, 255, 255)` with black text. It is the only place this could show, because the popover is the only opaque surface the widget paints -- everything else is transparent and inherits the page, which is why the search box and the page-size select already look right in dark mode. It is also invisible to anyone testing with a dark OS, where the system colours happen to come out right. `--sn-color-table-popover-bg` / `-fg` join the three themes and are mapped in the host sheet; the docs' per-theme sheets, which already re-point the other table tokens, re-point these at each theme's own mode-aware background and text colours, so the sphinx-needs docs are right in both modes of all three. The browser test builds the project with `needs_css = "dark.css"` and asserts the open popover's computed colours are the theme's, not the user agent's. The design document's CSS contract now says a host MUST answer the pair, and why.
…e sort glyph A theme gallery across five themes, light and dark, found the widget inheriting things it should not and drawing one thing badly. **The `Columns` control was an admonition.** It is a `<details>`, and a host theme may style every `<summary>` on the page as a collapsible admonition: a pale fill, a coloured border, an accent bar, bold text, an injected pencil icon and a chevron. Measured at 141 px wide against 76-82 px in the four themes that leave `<summary>` alone -- the extra 60 px is the injected icons -- and it was the only coloured object in a toolbar of plain-bordered buttons. It now states its own `font`, `background`, `box-shadow` and `border`, and suppresses the marker and any injected `::before` / `::after`. **The search box and the page-size select ignored the host's font, then took it.** The buttons carry `font: inherit`; the two form controls did not, so a theme's compounding `font-size` cascade landed on them alone: 10.14 px against 14.08 px buttons in one theme, and 22 px against a 31 px input in another -- two controls on the same line, nine pixels apart in height. Both now take `font: inherit` as well. **The at-rest sort indicator read as an hourglass.** One element carrying both a `border-top` and a `border-bottom` triangle makes them meet at a point, which at 14 px is a solid shape that says "loading" rather than "sortable either way". It is now two absolutely positioned triangles with a 2 px gap -- still borders, still `currentColor`, still no glyph and no image -- with the up one above the down one, and the sorted states show one of the pair at full strength without moving the header text. The browser test gains a cheap guard that every control on the bar computes to one font size; the real verification for all of this is the gallery.
…ract The CSS contract now says the sheet has to fence the widget off from the host's bare element selectors, not only take colour from it, and that the at-rest sort indicator is two triangles with a gap. It also repairs a paragraph break the popover correction ran into.
Review's coverage map found seven behaviours the design document publishes as contract -- things another producer implements against -- with no exerciser anywhere in the tree, so a mutation that removed each of them left the whole suite green. Six of them are pinned here (the seventh, the deferred script tag, was pinned with its fix). - `<td data-sort>` overrides a cell's sort value, and `data-needstable-labels` overrides the English strings. sphinx-needs emits neither today, which is exactly why the prose was the only thing holding them up. Both are now set on a built page and the widget re-initialised over it. - The export honours column visibility for PART rows as for leads: the same index filter runs over every row of every matching group, not only the leads. - The info element is an `aria-live="polite"` region and the pager carries its accessible name, so the count is announced rather than merely displayed. - A page size the producer names that is not among the offered sizes is added to the control. The test project's small table asks for one, so the assertion is on the real thing. - The three things a truncated, filtered table emits come out in a fixed order -- the table, then what selected it, then what was left out -- in a project of its own, because the `max_items` warning is the point of it. And one decision made explicit rather than left as an accident: for `:style: table` the filter paragraph now follows the wrapper `div` rather than sitting inside the `<table>`, which is right (the paragraph is not table content) and is now asserted for both styles.
…ly do The design document is the deliverable, and review read it against the code. Where the two disagreed, the code was right and the prose was wrong; where the code was surprising, it now says so. Corrected in place: `data-col` is the option name **lower-cased**, not "as the author wrote it" -- a producer implementing the contract has to know the key is normalised, and that it goes into `needs_col_<key>` verbatim, so a key that is not a CSS identifier needs escaping in a selector. Newly stated, because none of it was: a full stop is always the decimal mark and a comma groups digits only in the `1,234,567` shape, so `1.000` sorts as one; a bare `YYYY-MM-DD` is read as UTC and any other spelling in the reader's timezone; the filter index is a snapshot taken at initialisation, and a page that mutates a cell afterwards has to re-initialise for the filter to see it; exactly one header row is read, the last of the `<thead>`; and a `:style: table` table carries the row and header attributes but none of the table-level options. Two sentences of the number and date rules go in `needtable.rst`, where a reader who is sorting a column will meet them. One real fix came out of the same reading: the filter index collapses runs of whitespace but the query only had its ends trimmed, so a cell rendered with two spaces was found by typing one and NOT by typing what is on the screen. Both are normalised the same way now, with a test. Two kindnesses while here. The last column still showing keeps its checkbox, disabled -- hiding every column produced an empty grid, an info line still counting rows and a CSV of empty records. And Escape closes the columns disclosure and hands focus back to the summary that opened it.
The two assertions the `defer` fix was supposed to arrive with never landed -- a scripted edit failed on its first file and the two that followed it were skipped, and nothing said so because the tag was correct. The mutation that reinstates `loading_method="defer"` found it: both tests stayed green against a page carrying an attribute HTML does not have. They are here now, one in `test_html_head_files` and one in `test_assets_are_registered_per_page`, which unlike the former is not skipped on Windows. Both read the element's attributes and fail loudly with them.
docutils' own node classes are lower case (`paragraph`, `table`, `row`), and the test extension follows them; ruff's CapWords rule does not know that.
`font: inherit` on the form controls was not enough on its own, for two reasons
a render pass across four themes turned up.
A host may set a `font-size` on bare `label`, and the control inside the label
then faithfully inherits it -- so the labels take the bar's size explicitly now.
The `font` shorthand also resets `line-height` to `normal`, which the control
inside inherits in turn, leaving it a couple of pixels shorter than the buttons
beside it; the labels state `line-height` too.
And one host sets `body, input { font-size: 16px !important }`, which nothing
weaker can answer. That is the single `!important` in this sheet and it is
documented where it stands, as the rule for this sheet requires.
Measured on real docs builds, the four controls' computed font sizes:
furo 14.08 / 14.08 / 14.08 / 14.08
sphinx_rtd_theme 14.08 / 14.08 / 14.08 / 14.08 (was 14.08 / - / 10.14 / 10.14)
sphinx_immaterial 14.08 / 14 / 14.08 / 14.08 (was - / - / 16 / 13.33)
with their heights within three pixels of each other, against nine before.
The scroll frame that fix round 1 introduced left the table's `width: 100%`
where it already was -- in `css/common/needstable.css`, sphinx-needs' own host
sheet. The shipped pair is `libs/html/needstable.{js,css}`, which the contract
says a consumer vendors byte-identical, and on its own it carried no width rule
at all: measured on a page loading only the pair, the table lays out at 378 px
in an 800 px frame, with the `<colgroup>` percentages resolving against the
shrunken width. That is the finding the scroll frame exists to fix, reproduced
in the artefact the fix was written for.
Every existing test passed either way, because sphinx-needs supplies the rule
from the host side. What can fence the structural half is a page that loads
nothing else, so there is one now: `tests/fixtures/needstable_pair.html` carries
the markup contract and links the two files straight out of the package tree, and
the browser test opens it over `file://` and asserts the table fills its frame and
the control bar the container. It needs no sphinx build.
The contract now states the boundary rather than implying it: the structural
sheet is sufficient for layout on its own, and a host sheet answers the colour
tokens and nothing more.
The contract and the docs both say "a table cannot be reduced to no columns at all", and `checkbox.disabled` was only ever assigned when the visibility changed -- which for two or more columns means the reader's first click engages the guard in time. With exactly one column that click is already the fatal one: the reader empties the table, the CSV button then hands over a file of blank records, and the checkbox stays enabled so nothing explains why. One line: apply the visibility once at the end of `buildControls()`. The browser project gains a one-column page, and the test asserts the flags at init on both -- nothing disabled where there are columns to spare, the single one disabled where there are not -- and that clicking the guarded box changes nothing.
`paintPager` reads where focus is before it empties the pager, but the `pageCount <= 1` branch returned before ever restoring it -- so the one case where the pager does not come back is the one case where focus was still lost. It is the failure the focus restore exists to prevent, in the branch the restore does not cover. Its practical reach is small: every control that can collapse the table to one page (the search box, the page-size select) takes focus itself when a human uses it, so a keyboard reader cannot be in the pager when it happens. A script can be, and so can any future control that re-filters without focusing itself. Focus goes to the search box -- the control that collapses the table in practice. The keyboard test drives the case programmatically and asserts both that the pager really went to `display: none` and where the reader ended up.
The gallery's remaining cell: in one theme the `Columns` control was drawn with a blue 1 px ring, because the fence named `details.needstable-columns > summary` and the host styles the `<details>` element itself -- a border, a background and a margin around the whole control, two pixels larger than the button inside it, while `Copy` and `CSV` beside it were plain. The fence now reaches both. Two things it turned up that the ring hid: - with the host's padding removed the disclosure shrank to 53 px around a 77 px summary, because it is a flex item on the control bar -- hence `flex: 0 0 auto` and `min-width: max-content`; - and the remaining 24 px of that gap was the host's **negative** horizontal margin on `summary` (`margin: 0 -.6rem`), which makes the control wider than the box around it. The summary states `margin: 0` now. Measured after, in the theme that had the ring: `<details>` and `<summary>` both 77 × 26 with no border and no background, beside a 55 × 26 `Copy` button.
…ctory Everything this extension copies into `_static/sphinx-needs/` is its own, never anything a project wrote -- so when the file on disk differs from the file in the package, the package is right. Sphinx has not agreed by default, and has not disagreed the same way for long: up to 7.4 `copy_asset`/`copy_asset_file` take a private `__overwrite_warning__` and copy anyway; from 8.1 they take `force` and, when the destination exists with different bytes, warn `misc.copy_overwrite` and **abort the copy**. The consequence lands on this branch harder than on any release before it, because this branch changes four shipped asset files. An upgrader's first incremental build -- or anyone's build after a `git checkout` between commits -- keeps the OLD stylesheet on disk while the source has moved, so the widget runs on its fallback colours, and under `-W` the build fails in four lines that explain none of that. The theme-gallery run met it and produced a set of screenshots of the previous commit's CSS; a reviewer met it on their first command. The fix asks the signature rather than the version number -- `force` if `copy_asset` takes it, nothing if it does not -- so the 7.4 floor is passed nothing it cannot understand and a release that spells it differently again resolves itself. It is splatted into all four call sites this module has. Measured: a `docs-needs` over a build directory whose two `needstable.css` files had been replaced with different bytes now exits 0, emits no `copy_overwrite`, and leaves both files matching their sources. The test does the same thing to the three call sites in miniature: build, overwrite the three built files, build again on the same outdir without `-E`, assert no warning and no stale byte.
…patch case Two claims nothing was holding up. **The default theme's popover tokens.** The popover test built with `needs_css = "dark.css"`, so deleting `--sn-color-table-popover-*` from `modern.css` -- the theme every project that never sets `needs_css` gets -- left the whole browser lane green while silently restoring the white-on-dark defect for the default. The test is parametrised over both themes now, each with its own expected pair. **The sentinel's stated reason.** Its docstring said it distinguishes "no instance attribute" from "an attribute whose value is `None`"; nothing set the latter, and the second half is not what the code is for. It now says what it does and what is tested: restoring means putting back exactly what was there -- another extension's patch, or nothing -- and those need different operations, so "nothing" needs a value that cannot be confused with an attribute that is present. The test extension gains a second patch installed further down the page, with a second needtable after it, so the either-order case is exercised: one extension already patching when the first table is written, two by the time the second is, and both markers on everything that follows.
The round that removed two prose "9.0.0" claims introduced a third, in the sentence explaining that the count is of needs rather than rows. It now says "than DataTables did", which is the wording that cannot go stale and the one the PR body already used. `git grep 9.0.0` over `docs/`, `design/` and `src/` now finds five hits and all five are `versionadded::` / `versionchanged::` directives, which stay by project precedent. And the contract records what review measured about the scroll frame in a host whose own script wraps every table: the theme's container lands inside the frame, takes the overflow, and the frame never scrolls -- harmless, and the property the frame was chosen for, since a box the host does not know about cannot be displaced by the host wrapping the table. The `overflow-y: auto` that `overflow-x: auto` implies is noted with it.
The rule that gives the table its width shipped as a child combinator, `div.needstable-scroll > table` -- the same shape whose fragility the paragraph three lines above it is about. A host theme's own script that wraps every table in a container of its own puts that container between the frame and the table, and the rule stops matching: measured on the pair-only fixture, 378 px in an 800 px frame, which is the number this section's previous correction quotes as the defect it fixed. sphinx-needs is unaffected -- its host sheet's rule names no parent -- so nothing a project renders changes, and no test could see it. What was wrong is the shipped artefact and the sentence beside it: the contract claimed the structural sheet is sufficient on its own, and separately that a box the host does not know about cannot be displaced by the host wrapping the table. The frame indeed cannot; the rule that sized the table could. It is a descendant selector qualified by the hook class now -- `div.needstable-scroll table.NEEDS_DATATABLES` -- qualified rather than bare so that a table nested inside a needtable's own cell is not caught. The pair-only fixture is measured in both shapes on one page load: as authored, and after a five-line script wraps the table the way a theme's would. 800 / 800 in both.
…iewport The branch that hands focus to the search box when the pager collapses is conditional on something having had focus in the pager, and nothing tested the condition. Review made it unconditional and the whole browser lane stayed green -- while the page under test focused its search box on load and scrolled the reader 2,820 px down the document to reach it. Any page whose second or third table happens to fit on one page would have done that to a reader. Two assertions in the case that already opens the page and touches nothing: after load the active element is `<body>` and `window.scrollY` is 0. The test project's second table is a single page, so the branch really does run there.
The block that fences the columns disclosure off from a host theme had nothing automated behind it: review deleted the whole of it and the browser lane stayed green, because the only theme the suite builds is a polite one. The gallery is a human read, and the measured claim -- that `<details>` and `<summary>` agree in width in every theme -- had nothing holding it. The browser project now loads a stylesheet that does what the real themes were measured doing: every `<details>` painted with a border, a background and a shadow; a glyph injected into every `<summary>` through `::before` and `::after`; the summary pulled out of its own box by a negative margin; a 22 px summary; and a 9 px font on form controls beside 14 px buttons. The test asserts the computed values inside the widget -- no border, no background, no box-shadow, no injected content, no margin, the two boxes equal within a pixel, and one font size across the button, the summary, the input and the select. It first paints a bare `<details>` on the same page and asserts it comes out red, so a passing run means the fence held rather than that the hostile sheet failed to load.
chrisjsewell
force-pushed
the
feat/needstable-enhancer
branch
from
September 8, 2026 17:17
8a18bde to
d367bc1
Compare
ubmarco
reviewed
Sep 8, 2026
ubmarco
left a comment
ubmarco
left a comment
Member
There was a problem hiding this comment.
Did you test the new solution with sphinx simplepdf?
Member
Author
Not before you asked — now yes, and it needs no change on either side. What I measured. sphinx-simplepdf's own
Why nothing changes. WeasyPrint runs no JavaScript, so DataTables never ran in a simplepdf build and the new script does not either — the PDF gets the server-rendered table in both cases. The things that reach WeasyPrint are:
The only markup differences are additive attributes ( If simplepdf ever wants to adapt — it does not have to — the one honest improvement would be to key its needtable rule on |
ubmarco
approved these changes
Sep 8, 2026
This was referenced Sep 8, 2026
chrisjsewell
added a commit
that referenced
this pull request
Sep 8, 2026
`needstable.js` (#1920) is plain JavaScript typed through `// @ts-check` and JSDoc, and `design/needstable-contract.md` §4 records the single command that checks those types. It passes at the tip, but nothing ran it, so any later edit could regress a type silently. This makes it a gate. - **A poe task, `typecheck-js-needs`**, holds the command with the typescript version **pinned** (`typescript@5.9.3`): an unpinned `npx -p typescript` resolves whatever compiler is newest that day, which would let a TypeScript release turn a pull request red with nothing here having changed. To bump it, edit that one literal and run the task. 5.9.3 is the newest 5.9.x; the file is also clean under 7.0.2, today's `latest`. - **One Lint step runs the task**, not a copy of the command, so the gate a developer runs locally and the gate that blocks a pull request cannot drift, the same arrangement as the prek hooks and `poe lint`. Lint rather than the `tests-js` lane: it needs no browser, no sphinx and no interpreter matrix, and Lint is a required status context in its own right. No `setup-node`: `ubuntu-latest` ships node and npm, and `npx -y -p` fetches the pinned compiler into the runner's npm cache for the run (about 2.5 s cold on an empty cache, 23 MB unpacked). - **Files checked**: `needstable.js` only. `sphinx_needs_collapse.js` is jQuery-era, carries no `// @ts-check` and does not pass `--strict`; #1922 ports it and is the pull request that adds the second line. The task lists one path per line, so that is a one-line change. - **Docs**: §4's "It is not a CI gate here" sentence now says the opposite, and the root `AGENTS.md` commands block gains the task. No changelog entry: nothing user-visible changed. Proof the fence bites: with `this.sortColumn = -1` (declared `@type {number}`) changed to a string, the task exits 2 with ``` src/sphinx_needs/libs/html/needstable.js(452,13): error TS2322: Type 'string' is not assignable to type 'number'. ``` Closes #1924
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A
needtablewith the default:style: datatablesis 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, theneeds_<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.htmland 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.innerHTMLinto astring[][], 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!importantrules 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 ofpackages/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>keepsNEEDS_DATATABLESas its hook and gainsdata-needstable-*options; every<th>gainsscope="col", adata-colcolumn key and, where Sphinx-Needs knows it, adata-type; every<tr>gainsdata-need-id, and a part rowdata-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-commandtscrecipe 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 getscurrentColorfallbacks.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
.dataTables_*/.dt-buttonCSS hooks → theneedstable-*classes. A project that styled the old chrome has to re-point at the new one; a project that styled.NEEDS_DATATABLES,.need,.need_partorneeds_<column>is unaffected.--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_overwritewarning — and fail a-Wbuild — on the first incremental build after the upgrade.Gained
needs_table_page_size,needs_table_page_sizesand a:page_size:option;:sort:order on the third click, so the documented "sorting only works with:style: table" caveat is gone;aria-sortand keyboard-operable headers, anaria-liveresult count, focus that stays inside the pager when you page with the keyboard;<summary>;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 letssphinxcontrib-jquerygo.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-shotsinstrument (its own PR follows). The Read the Docs preview of this PR shows the furo build live.Changelog draft
Closes #462.
Related issues, and what this changes for each
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.:sort: linenoalready answers it, but only worked with:style: tablebecause 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
:sort: linenonow works in the default style; close after release, multi-key:sort:stays opendata-needstable-*attributes rather than a DataTables option dict; the August side extension there stops working — needs a heads-upFollow-up work, none blocking: port
sphinx_needs_collapse.jsoff jQuery and dropsphinxcontrib-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.