fix(table): make the category filter work#2024
Merged
Merged
Conversation
The filter has never worked from Markdown: the shortcode does not forward its arguments, a string value is never split into a slice, its behaviour lives in an optional module, the DOM fallback breaks on wrapped tables, and its i18n keys do not exist. Makes the filter a data-table feature: filter implies a data table, so it works standalone, and the DOM fallback is deleted along with its bug.
Six tasks across hinode and mod-simple-datatables, each ending in an independently verifiable deliverable.
tableFilterLabel and tableFilterAll were called by the table partial but existed in no language file, so they were untranslatable and emitted i18n warnings. The partial only rendered because it passes a default to T. Also records in the table structure that filtering, like sorting, paging and searching, needs the simple-datatables module.
AddModule used complement to add a module to the page's dependencies scratch, but complement returns the elements of the LAST collection absent from the others - so it replaced the list with the single module being added, and wiped it entirely when re-adding one already present. Append and deduplicate instead, matching how head.html and scripts.html merge the same scratch.
The shortcode never forwarded filter or filter-col, so a filter declared in Markdown validated cleanly and then did nothing. Forward both, and split a comma-separated string into a slice: a shortcode parameter is always a string, InitArgs validates a kind without casting, and range cannot iterate a string. A filtered table is now always a data table, even with sorting, paging and search all off - filtering runs through simple-datatables' row model, so it composes with them, and searchable gates only the search input, not the search method. An author no longer has to enable sorting merely to get filtering. Drops data-filter-container, which only the DOM fallback ever read.
Replace the md5(page + now)-based filter id with a sequential counter held in page.Store. now() was evaluated per call, so a filtered table's data-filter-id changed on every build even when the page content was unchanged, defeating output diffing and CDN caching. The id only needs to be unique within a page, so a page-scoped ordinal (table-filter-1, table-filter-2, ...) is sufficient and stable.
A sortable, paginated, searchable or filtered table needs simple-datatables' JavaScript and its per-page stylesheet, and Hinode loads an optional module's assets only for pages listing it in frontmatter. Omitting the key produced a table that silently did nothing. The opt-in cannot be added at render time: head.html emits the stylesheet before the content renders, so a dependency declared by a shortcode would attach the script without the styles. Warn at build time instead.
AddModule.html ignored its `page` argument and always wrote dependencies to Hugo's bare global `page`. In a Bookshop-block render that global can resolve to the wrong page, so the dependency landed on the wrong page's Scratch. Now an explicitly-passed page is used, falling back to the global `page` when the caller omits it, matching how mod-blocks' list component already calls it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Read `filter-col` as-is in assets/table.html: mod-utils already defaults it to 1, and piping it through Hugo's `default` rewrote an explicit `0` (an empty value to `default`) back to 1, so column 0 could never be filtered - Move the missing-module warning from the partial to the shortcode: `.Page` is the content page only in a shortcode, while a Bookshop block resolves the partial's `page` argument to the rendering template's page - which warned about pages holding no table at all - Pass `details` as a slice, so LogMsg no longer appends a stray `%!s(<nil>)` line - Correct the rationale comment: a render-time dependency is unreliable because it is indeterminate (see assets/lightbox.html), not because head.html runs before the content Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Treat `filter` as present only if it has a non-comma, non-whitespace character, matching how assets/table.html normalizes it (split on commas, trim, drop blanks). A separator-only value like `filter=","` no longer triggers a spurious missing-module warning. - Correct the rationale comment for warning instead of auto-declaring the module dependency: state the actual reason (auto-declaring would tie the shortcode to head.html's eager evaluation of `.Page.Content`, an implementation detail, not a contract) instead of the disproven claim about Page.Store timing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
✅ Deploy Preview for gethinode-demo ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Four components in mod-blocks build a DOM id from md5(... now ...), so the id changes on every build and any page carrying one emits different HTML each time. The table shortcode had the same bug; its fix introduced a deterministic page.Store counter. Extract that into utilities/UniqueID.html and apply it to all four remaining sites. A spike confirmed page.Store is stable and unique in a Bookshop block render, which was the design's key unknown.
No gethinode module declares a dependency on Hinode, so a partial there is reachable only because the site merges Hinode's layouts - an undeclared inverted dependency that breaks each module's own exampleSite build and would block the .Ordinal follow-up, which must reach mod-lottie and mod-leaflet. Moves AddModule.html to mod-utils for the same reason: mod-blocks calls it without declaring Hinode. Callers are unaffected; Hugo resolves partials by path.
Hinode's exampleSite pins mod-blocks, so merging Hinode first would deploy a demo site still emitting dropdown-panel-<md5>. Sequence is mod-utils, then mod-blocks, then Hinode.
Six tasks across mod-utils, mod-blocks and hinode. Merge order is strictly sequential: mod-utils, then mod-blocks, then hinode.
The table carried its own page.Store counter, duplicating what mod-utils' UniqueID now provides. Same rendered id shape, one implementation. Also drops Hinode's copy of AddModule.html, which has moved to mod-utils: mod-blocks calls it without declaring a dependency on Hinode, so it belongs where every module can legitimately reach it.
Reflects deterministic ids for panel, faq, nav, preview, and testimonial-carousel once built against the unreleased mod-utils UniqueID helper and mod-blocks call-site migration. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Regenerate hugo_stats.json against the UID-shaped ids produced by the updated UniqueID.html helper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
Added: adopt the shared
|
mod-utils v6.5.0 provides the UniqueID helper the table now calls and the AddModule partial this branch deleted from Hinode, so the build resolves both from a release rather than a local override. mod-blocks v2.1.3 carries the four components' conversion to the helper, so the deployed exampleSite renders their ids in the deterministic -UID- form instead of a per-build hash.
…lter # Conflicts: # exampleSite/hugo_stats.json
Collaborator
Author
|
🎉 This PR is included in version 3.2.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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
1 participant
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.
The table shortcode's
filterargument — a button group that narrows a table's rows to a chosen category — has never worked from Markdown.data/structures/table.ymldeclaresfilterandfilter-col, so{{< table filter="widget,gadget" >}}validated cleanly and then did nothing at all.Fixing it turned up five separate defects, plus three more found along the way.
The filter
It was unreachable.
layouts/_shortcodes/table.htmlnever forwardedfilter/filter-colto the partial.Forwarding alone would not have worked.
filter's declared type is[string, slice], a shortcode named parameter is always a string, and mod-utils validates an argument's kind againstacceptswithout casting between kinds — so the partial would have received the unsplit string"widget,gadget"and reached{{ range $filter }}, which Hugo cannot iterate over a string. The partial now normalizes: a slice is used as-is, a string is split on commas with each value trimmed and blanks dropped.filternow implies a data table. Setting it alone adds thedata-tableclass even withsortable,paginateandsearchableall off. That is safe — in simple-datatables v10.2.0options.searchablegates only the search input and a wrapper class, not the publicsearch()method. Filtering therefore always runs through the library's row model, where it composes with sorting, paging and free-text search, and an author never has to switch on sorting they don't want just to get filtering.That makes the module's DOM-fallback branch unreachable, so gethinode/mod-simple-datatables#278 deletes it — along with the bug it carried on wrapped tables.
A build warning replaces a silent failure. A data table needs the module's JavaScript and its per-page stylesheet, and Hinode loads an optional module's assets only for pages listing it in frontmatter. Omit
modules: ["simple-datatables"]and the table silently did nothing — true ofsortable/paginate/searchableall along. The shortcode now says so at build time. It lives in the shortcode rather than the partial because only there is.Pagereliably the content page; a Bookshop block resolves it to the rendering template's page, which would warn about pages holding no table at all.The i18n keys did not exist.
tableFilterLabelandtableFilterAllwere absent from every language file — the partial only rendered because it passes a| defaulttoT. Added across all eight languages.Bugs found along the way
utilities/AddModule.htmldestroyed the dependency list it was meant to append to. It usedcomplement, which returns the elements of the last collection absent from the others, and then set the scratch to that result — replacing the list instead of appending, and wiping it entirely when re-adding an entry already present. It also ignored thepageargument callers hand it, using Hugo's bare globalpage; mod-blocks'listcomponent and mod-mermaid both pass one and were silently broken by this.data-filter-idchanged on every build. It wasmd5(delimit (slice . now) "-")— thenowmeant any page with a filtered table emitted different HTML each time, defeating output diffing and CDN caching. It is now a deterministic per-page counter inpage.Store. (page.Storewas empirically confirmed to survive an embeddedRenderString, which is what{{< example >}}does;.Ordinalwas confirmed not to.)filter-col="0"was silently coerced to1. A redundant| default 1met Hugo'sdefault, which treats0as empty — so column 0 could never be filtered.Verification
Driven in a real browser, with zero uncaught console exceptions: clicking a category filters the rows; the pager recounts to the filtered set (2 pages → 1), proving the filter reaches the row model rather than merely hiding DOM rows; sorting survives a filter; and on a wrapped table below the breakpoint the filtered-out records vanish entirely while every remaining record keeps both its data row and its description row — the old fallback bug is gone.
exampleSite/content/en/table-demo.mdcarries the fixture tables.Follow-up, not in this PR
.Ordinal-derived DOM ids collide inside{{< example >}}:example.htmlre-renders its inner content with.Page.RenderString, and.Ordinalrestarts at 0 in that sub-render. Reproduced — a page with two example blocks each holding an accordion emitsaccordion-0for both. It affects 12 shortcodes across hinode, mod-lottie and mod-leaflet, and hits the component docs hardest. Thepage.Storecounter landed here is the ready-made replacement.🤖 Generated with Claude Code