Repository navigation
Handle footnotes, grouped rows, and multi-value cells in filterable tables - #4354
alexandra5000 wants to merge 4 commits into
Conversation
…ables
Three edge cases surfaced when the :filterable: option was tried on real
tables (the EDOT components list and the AWS Firehose data-type table):
- A footnote reference is part of a cell's text, so a footnoted "Core"
scraped as "Core 1" and split into its own dropdown option. Footnote
markers are now stripped before a cell's value is read.
- A grouped table uses rows whose first cell is a heading and whose other
cells are empty. Those rows inflated the count and vanished under a
filter. They are now excluded from the count and kept visible only while
their group has a matching row. If every row looks like a heading, none
is, so a late-rendering column does not misclassify the whole table.
- A cell listing several comma-separated values ("Logs, Metrics") never
matched a single dropdown option. Such a cell now contributes each value,
and a row matches a filter for any of them.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Docs preview (local build)Handbook preview: https://docs-v3-preview.elastic.dev/elastic/docs-builder/pull/4354/ |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes for a functional regression: multi-value columns can lose their facet under the current key-column heuristic.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
Tokenizing comma-separated cells made the key-column guard compare a token count against the row count, so a category column whose tokens are shared across rows (for example three rows of "Logs, Metrics" / "Metrics, Traces" / "Traces, Logs") was mistaken for a unique-key column and lost its dropdown. A column now counts as a key column only when every row holds exactly one value and those values are all unique. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Requesting changes: the new section-header heuristic currently misclassifies legitimate sparse data rows as structural headers, which can remove real rows from counts/facets and hide them during filtering.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
The first version classed any row with a filled first cell and empty rest as a section header, which also matched a sparse data row whose trailing cells are blank - removing it from the count and hiding it under a filter. A section heading is now recognised only when its sole filled cell is emphasized (`|***Receivers***|||||`), the convention these grouped tables use. A plain sparse data row stays data. The failure direction is safe: a non-emphasized heading is merely counted as data, never the reverse. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Requesting changes for a remaining correctness issue in footnote value extraction that can still produce mismatched facet values for valid nested-markup cells.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
cellText skipped only direct-child footnote-ref nodes, so a footnote written inside emphasis (`**Core[^1]**`, rendered as `<strong>Core<a class="footnote-ref">…</a></strong>`) left the digit in the value, giving "Core1". It now walks the cell and skips a footnote-ref subtree at any depth. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| // heading as data only miscounts by one, while the reverse would drop a | ||
| // real row from the count and hide it under a filter. | ||
| const emphasis = first.querySelector('strong, em, b, i') | ||
| return emphasis?.textContent?.trim() === heading |
There was a problem hiding this comment.
[LOW] Emphasized sparse data rows can still be treated as section headers
The current heuristic still classifies <tr><td><strong>alpha</strong></td><td></td></tr> as a section row (emphasis?.textContent?.trim() === heading), so that row is removed from this.rows and can disappear from both counts and search/facet results.
This is a real behavior change vs. pre-grouping logic: a sparse-but-data row with emphasized first-cell text becomes unfilterable because readRows() excludes it as structure and applyFilters() only toggles it as a heading.
Could we tighten this with a structural marker emitted by rendering (or another non-presentational discriminator), so emphasis alone does not decide row semantics?
There was a problem hiding this comment.
Yeah, so technically this is true, the filter is guessing which rows are section headings.
Markdown has no real way to mark a row as a heading, so the filter guesses from looks: first cell bold, other cells empty means it will be treated as a heading. That works today, but a real data row that happens to be bold-and-blank would be mistaken for a heading and vanish when someone filters, although that is rare and to my knowledge is not a real table case in the docs anywhere.
The proper fix would be of course to give authors a way to mark a section heading in the {table} directive and have the filter read that instead. But TBH I don't know if we really want to do that. Anyway that's a feature, not a bug fix. Noted on the main tracking issue, #4282, for any bots reading it in the future (if there is any).
The
:filterable:option now derives and matches dropdown values correctly on tables with footnotes, grouped section rows, or cells that list several values. A plain table is unaffected.Affects:
Site UI,AuthoringTrying the merged
:filterable:option on real tables — the EDOT components list and the AWS Firehose data-type table — turned up cases where the auto-generated filters misread the content.Why
The filter reads each cell's text to build its dropdowns and to match rows, and three kinds of real content break that. A footnote reference is part of the cell text, so a footnoted
Corereads asCore 1and becomes its own option, and selectingCoredrops the footnoted rows. A grouped table's section-header rows (first cell a heading, the rest empty) count as data rows, so the total is wrong, and they disappear the moment a filter runs. A cell that lists several comma-separated values never equals any single option, so a row that sends bothLogsandMetricsis dropped when the reader picksMetrics.What
Footnote markers no longer leak into values
A cell's value is now read with footnote references (
.footnote-ref) removed. A footnotedCorecontributesCore, so the component list offersCore,Deprecated, andExtendedrather than seven near-duplicates, and filtering toCorekeeps the footnoted components.Section-header rows are structure, not data
A row whose first cell is filled and whose other cells are empty is treated as a heading: excluded from the count, and shown only while its group still has a visible row. If every row looks like a heading — which happens when a lazily rendered column is still empty — none is treated as one, and the observer re-reads once the column fills.
A multi-value cell contributes each value
A cell is split on commas, so
Logs, MetricsoffersLogsandMetricsseparately, and a row matches a filter when any of its values matches. A service that sends both is no longer dropped by a single-value filter.Verify
Then serve a grouped table and confirm the count and the dropdown options:
Out of scope: the comma split treats every comma as a value separator, so an atomic low-cardinality value that contains a comma would be split wrongly. No such column exists in the tables seen so far; flagged in #4282 for a follow-up if one turns up.
🤖 Generated with Claude Code