Make the comparison tables semantic and leave one main per page - #631
Conversation
Pricing/ComparisonTable and Landing/ComparisonTable both claimed .comparison-table and .comparison-title. Pricing declares its block is:global, so its table rules (width, border-collapse, table-layout, min-width) would apply to Landing's <section> as soon as both render on one page. Renames Pricing's 19 classes to pc-comparison-*, including the four selectors in pricing/index.astro and PlanFeatureItem that reach into them. No markup or style changes.
Emits caption, colgroup, thead with th scope="col", and a tbody slot; callers supply rows whose first cell is a th scope="row". Sets no box metrics at all, since the existing table skins agree on none of them. DataTableValue renders a cell's value as text in every case, with the icon marked decorative. That is what makes a cell readable both by a screen reader and by plain-text extraction, which currently sees the icon-only cells as empty. Ships its own visually-hidden class rather than reusing Starlight's: that one is layered, so any unlayered rule beats it, and it is absent from LegalLayout pages entirely. Both the hidden caption and the hidden header row zero their box instead of being positioned, because position: absolute would blockify them and strip the caption role and the columnheader mapping that scope="col" depends on.
These four already emit a real <table>; they were missing an accessible name, column and row header semantics, and a keyboard-reachable scroll region. Content is untouched. Two cells needed care rather than a straight td-to-th swap: UseCase/ComparisonTable styled th with the dark header row's white text, so a body row header would have rendered white on a light background. Scoped that rule to thead th and extended the td rule to tbody th. development-services had a 1279px media query keyed on td alone, so the converted row header would have lost its wider padding between 960 and 1279px. Extended that selector too. The hardware spec table gets a caption and scope="row" but no thead: it is a key/value table with no column headers to name. LegalLayout had the opposite defect from the rest of this work — no main landmark at all on its ten-plus pages, since they do not route through Starlight. Its container is now a main. Adds a visually-hidden mixin so the recipe has one home.
Replaces the hand-written table shell with the shared primitive, which brings a hidden caption, th scope="col" on every column including the first, and a keyboard-reachable scroll region. Cell values are now text in every case. The check icon becomes decorative with a hidden "Yes" beside it, and the empty span that stood for "not included" becomes a hidden "No" — the cell stays visually blank, which is the intended design. Plain-text extraction of this table goes from 45 empty cells to none. The feature column moves from td to th scope="row" and gains an explicit text-align, since th would otherwise centre it. The skin needed no changes: its block is already is:global, so the rules rooted at .pc-comparison-table still reach the thead and tbody now emitted by DataTable.
Removes the self-closing <td /> that produced the site's only W3C "Stray end tag td" error, on /products/paas/. It becomes a real th scope="col" carrying hidden text, which also gives the first column a header and the row a complete set of column headers. Value icons lose their aria-label — unreliable on a bare svg with no role="img", and invisible to text extraction — in favour of a decorative icon beside hidden text. Extraction of this table goes from 39 empty cells to none. The style block had to become is:global in the same commit as the markup move: Astro hashes every compound, so a rule such as "thead tr th" would have silently stopped matching once the shell came from DataTable. Every rule is nested under a new .comparison-matrix class so none of the bare element selectors — h2, table, thead tr, tbody tr — leak site-wide. The existing #matrix id is untouched, since it is an anchor target and is also used by the Edge matrix. The caption stays hidden and the table adds no height: the decorative column backgrounds are positioned from hardcoded row metrics, and the painted boxes still align to the columns exactly. Adds a row/column length check, which previously failed deep in the render with an opaque error.
Lifts the eleven feature rows out of the component into src/data/comparison/edge-vs-gateway.ts and takes them as an optional prop, matching how the CE-vs-PE table is parameterised. The empty td that opened the header row becomes a th scope="col" with hidden text, so the label column has a header and the row a complete set. It keeps the transparent background and zero padding the td had. Value icons become decorative beside hidden text rather than carrying an aria-label on a bare svg. Extraction of this table goes from 29 empty cells to none. Two things this skin depends on, both verified unchanged: Column widths are now emitted inline on the col elements as well as through the col classes. A fixed table whose col widths become unreachable falls back to equal quarters, which would have turned 40/20/20/20 into 25/25/25/25. The column boxes are painted positionally — td:nth-child(2..4) supply each column's colour and side borders, and tr:last-child adds the 3px base and the rounded bottom corners. nth-child counts every element child regardless of type, so promoting the first header cell from td to th does not shift the numbering. No row or cell may be inserted, reordered or appended here, and there must be no tfoot. The style block becomes is:global in the same commit as the markup move, nested under a new .edge-matrix class so the bare h2, table, thead and tbody selectors cannot leak. Both media queries move with it. The off-vocabulary values — Low, Medium to Low, High to Medium, Data Collection — are left alone: they are visible copy and already extract as text.
Starlight's Page component already wraps every one of these routes in a main, so the page-level one was a second, nested landmark. Each becomes a div keeping its class. Twenty pages plus the use-case and case-study layouts; verified by counting the rendered output, which now reports exactly one main on every affected route. Two couplings the change had to carry: All delegated pricing interaction — product tabs, sub-tabs, the FAQ scroll, the region and billing toggles — hangs off a single root looked up as "main.pricing-page". That selector loses its element and is now ".pricing-page"; the comments naming it are updated with it. Five product pages had no class on their main and styled it through a bare element selector in an is:global block, so the rule was reaching Starlight's main as well as their own. They gain a page-main class and the rule keeps main alongside it, so neither element changes width. The two remaining classless mains, on trainings and mqtt-broker, have no rule attached and become plain divs.
This table already had text in all 88 value cells, but it was the one table on the site not announced as a table at all: display:block, the GitHub-markdown overflow idiom, removes the table role in every engine. The horizontal scroll moves to a wrapper that is a labelled, focusable region, and the table goes back to display:table. The header row gains scope="col". The category and feature columns were both plain cells, so the two-level row header is now explicit: the category spanning a group is a th scope="rowgroup", and the feature is a th scope="row". Seven categories, twenty-seven features — note one category, Support, spans a single row and carries no rowspan, so it reads as a feature at a glance and is easy to mis-tag. Promoting those cells to th would have made them bold and centred, so the weight rule is now scoped to thead and the body headers are pinned back to normal weight and start alignment. Measured before and after: table 980x1085 at the same origin, and every cell type identical on weight, alignment, padding, width and position. Deliberately not routed through DataTable. This page's style block is scoped on purpose — the comment in it explains that .markdown-body needs that specificity to beat LegalLayout's global rules, and LegalLayout sets text-align: justify on the container at equal specificity but later in source order. Making the block global to satisfy the skinning contract would hand those rules the win. The table is authored here, so keeping it in place avoids the hashing problem entirely.
This section was a div grid, so nothing about it was a table to a screen reader or to text extraction. Each group card becomes its own table: the card is the table, the group title is its caption, and each row is a tr whose first cell is a th scope="row". Every table carries a zero-height header row of its own, and the visible label card on top is marked aria-hidden so those labels are announced once rather than per group. The row-wrapping anchor is gone. Its accessible name used to be the entire row including both values; the link now lives in the row header and is stretched over the row, so the name is just the feature label while the whole row stays clickable. Hover moves to tr:has(a[href]). Values are text in every cell: the mark becomes decorative with hidden "Yes" beside it, and the blank non-breaking space becomes hidden "No". 105 cells, none empty. Notes on the parts that were easy to get wrong: The white card is drawn as a 4px border split across the caption and the table rather than as padding, because a caption is a sibling of the table's grid box and the table's own padding, background and radius never reach it. The radius splits with it and the inner corners compute to zero, matching the square header corners. Column widths are plain percentages, recomputed per breakpoint. The formula the ticket specifies, calc(20px + (100% - 40px) / 3), does not work: a calc() containing a percentage is silently dropped on a col element — and on a header cell too — leaving the columns as equal thirds, which is the exact misalignment the formula was meant to avoid. The percentages fold the row's 20px side padding into the first and last columns so the row divider still spans the full card width. Padding, height and the divider all moved from the row onto the cells: a row in the separated border model ignores all three. The hover bar gets an explicit top, having previously been centred by the flex row. Verified against measurements of the previous render at 1440, 900, 500 and 375: the card, caption, divider span, feature text origin and every mark position match, and the marks land within 0.02px. One accepted deviation, on the TBMQ page only: column widths are table-wide, so the group mixing a text-link row with an icon row shifts that icon 15px at 375px.
All four were introduced by promoting a cell from td to th, or by the change of containing block that came with it. Each is measured. services/development-services: the Fast-delivery label column is styled by tbody td:first-child, which stopped matching once the labels became th. The column lost its blue background, its left alignment and its square corners, rendering identically to a value cell beneath a header that still painted blue — visible at any viewport above 960px, which is the only width where that table renders at all. tr:last-child td was orphaned the same way, leaving the last label cell with a border its neighbours drop. Both selectors now accept either cell type. Landing/ComparisonTable, hover target: tr:has(a[href]) also matches a row whose only link is a text link in a value cell, so three rows across two pages showed the full row-hover affordance — background, label growth and the left bar — while being inert. Previously those rows were divs, which [href] never matched. The selector now requires the link to be in the row header. Landing/ComparisonTable, hover bar: the bar was offset by a constant chosen for the row's resting height, but the same hover rule grows the label's line height, so every row grew and the bar sat 3px high on all 27 of them. It is centred now, which also holds when a label wraps. ce-vs-pe-diff: the new scroll wrapper is a block formatting context, so the table's own bottom margin can no longer collapse out of it and was added to the wrapper's. The gap to the following rule went from 24px to 40px. The table's margin is zeroed inside the wrapper. Also collapses a comment that had been left stacked on top of an earlier draft of itself.
Every table had a caption, which satisfied the rule, but most captions
said nothing on their own. A consumer that lifts one table out of the
page got "Additional features", "Feature comparison", "Specifications",
or — on the two product pages that render seven tables each — a bare
group name like "Platform Core", with no mention of the product or the
editions being compared. The cells were machine-readable; the table had
no subject.
Each caption now names it:
Additional features -> ThingsBoard Private Cloud: additional
features by plan
Feature Comparison Matrix -> ThingsBoard Community Edition,
Professional Edition and Cloud
Feature comparison -> Google IoT Core compared with
ThingsBoard Professional Edition
Feature Comparison -> ThingsBoard Community Edition compared
with Professional Edition
Specifications -> <partner name> hardware specifications
Fast delivery -> why each capability matters and what it
gives
<use case> overview -> high-performance compared with
traditional
The pricing and matrix components take an optional caption so the page,
which is where the product context lives, can supply it.
On the two comparison-grid pages the caption is the visible group header
bar, so its text could not change. The context is appended there as a
visually-hidden suffix instead: the bar still reads "Platform Core"
while the table's accessible name reads "Platform Core — ThingsBoard
Community Edition compared with Professional Edition". TBMQ passes its
own wording. Verified the visible text, the header bar's painted box and
the mark positions are all unchanged.
"Skin" and "skin-owned" appear nowhere else in this repo — they were invented in these comments and never defined, so a reader had to work out that they meant the calling component's own CSS. The comments now say that, and name the properties DataTable deliberately leaves alone rather than gesturing at "box metrics". Comments only; no rendered output changes.
Hovering a row grew its label's line-height from 24px to 28px, which took the row from 50px to 54px and shifted everything below it — the table moved under the pointer. The font still steps up a pixel, so the emphasis is unchanged, but the line box no longer grows. This was not introduced by the table conversion: the flex row it replaced had min-height: 50px and the same line-height growth, so it jumped by the same 4px. Measured across every row on both pages that render this component, at 1440px and 901px — the only widths where the hover rule applies: maximum height change 0px, and no label re-wraps at the larger font.
The Gantt-style timeline was the one in-scope table with no caption, no column scopes and no row headers. It now has all three: a hidden caption naming the table, scope="col" on the nine header cells, and the seven step labels promoted to th scope="row". Three rules needed widening first, or the promotion would have shown: The bare `th` rule was not scoped to thead, so a body header would have taken the 18px header font and an underline on every row. Two `td:first-child` rules carry the whole step-label column — its padding, size, colour and top alignment, plus the bold-on-row-hover — and would have stopped matching. A th also centres by default, where the td inherited start. The 49 empty week cells are unchanged. Emptiness is the data there: a step is simply not active that week, and each filled cell already states its own duration, so extraction loses nothing by their being blank.
The seven category headers carried scope="rowgroup", but the table has a single tbody, so each was asserting itself as the header for every remaining row in it — a Device Management row could resolve its group header as Platform Core. They now carry no scope at all, leaving the spec header-assignment algorithm to scope each one to exactly the rows its rowspan covers, which is what these cells mean. The feature column keeps scope="row" on all 27 rows and the header row keeps scope="col" on all four. The alternative — one tbody per category, which is what scope="rowgroup" is designed for — was rejected because the zebra striping is tr:nth-child(2n) and restarts inside each new tbody. Group four has an odd row count, so the parity flips from group five on and seven rows change shade. Verified after this change that all thirteen striped rows are exactly where they were.
Two ways this component could still produce the empty cell it exists to prevent. A string value fell through to be rendered verbatim, so an empty or whitespace-only string emitted a cell with no text and a green build. It now throws, naming the component and suggesting a real value \- an empty cell in a comparison table is an authoring mistake, not a state worth rendering. The hidden Yes/No text used a class declared only in DataTable, so the words rendered visible on any page using this component inside a hand-written table. It now declares the class itself, from the shared mixin. Both components declaring it is intentional \- each has to work alone, and the rule is identical either way. Verified on a throwaway page with no DataTable present: the spans are 1x1 and clipped, the checkmark still draws, a string value still renders as real text, and a blank value fails the build with the new message.
The id a caption carries, so its scroll region can name itself, was derived by the same unguarded slug expression in two components, and had three ways to produce an unusable value. Slugifying keeps Latin alphanumerics only, so a caption in any of the site's non-Latin locales — or one made of punctuation — reduced to nothing and emitted a bare prefix. Two such tables on a page collided. There is now a hash fallback, so an id is never bare. An override of the empty string was accepted, because ?? guards only null and undefined. A caller computing the override from data with a missing field would emit id="" and leave the region unnamed. Both components now treat a blank override as absent. SectionTable repeated the derivation and offered no override at all, so a collision there could not be worked around. It takes the same optional prop as DataTable now, and both call the shared helper. Verified against English, Chinese, Arabic, punctuation-only and a whitespace override: five distinct ids, none bare, no duplicate or empty id on the page, and every aria-labelledby resolving to its caption. The ids on the four live pages are unchanged. What this does not fix: two captions with identical text still derive the same id. A component cannot see its siblings, so that stays the caller's job — which is what the override is for, and why SectionTable needed one.
# Conflicts: # src/components/Landing/ComparisonTable.astro # src/pages/products/mqtt-broker/index.astro
vvlladd28
left a comment
vvlladd28
left a comment
There was a problem hiding this comment.
Review summary
Reviewed 39 changed files in Make the comparison tables semantic and leave one main per page. Left 20 comments inline.
The <main> removal itself checks out: BaseLayout renders through <StarlightPage>, which wraps the slot in Starlight's own <main id="_top">, so every page-level <main> really was a nested second landmark. The skip link targets #_top (Starlight's main), starlight-toc.ts's main [id] query still resolves against the outer main, and nothing in the tree reads main.pricing-page or main[data-slug] any more — the pricing delegation root and all four .comparison-tooltip-* → .pc-comparison-tooltip-* renames are consistent across CSS, TS and the querySelector calls. LegalLayout genuinely had no landmark and correctly gains one. What I'd push back on is not the decision but its execution: 20 pages were edited by hand into four different wrapper conventions, five pages keep a now-dead main, in the selector, one page is left with a classless <div> that does nothing, and the new "exactly one main per page" invariant has no guard — so the first page authored from an older template silently reintroduces the defect.
On "nothing changed visually": the cell-level arithmetic is genuinely careful — the 4px caption border + 20px cell padding reproduces the old 24px text origin exactly, the :is(td, th) sweeps through development-services catch every selector the td→th conversion would have orphaned, and I could not find a positional selector (tr:nth-child, :first-child) that the new <caption> displaces. Two things are not settled, though. The stretched row link depends on position: relative applying to a <tr>; if WebKit still computes that as static, the overlay re-anchors to .comparison-inner — the whole comparison section, all seven tables — and one row's link swallows clicks across the entire section. That is a hard blocker, not a cosmetic risk, and it needs the Safari check before merge. And the column widths are now 21 hand-measured four-decimal percentages with no recorded derivation, so "unchanged today" and "stays unchanged" are different claims.
On maintainability, the recurring theme is that the PR introduces a primitive and then works around it: the largest new table (seven tables in Landing/ComparisonTable) bypasses DataTable entirely and hand-rolls the caption, colgroup, zeroed thead and hidden Yes/No text; four of DataTable's ten props have no caller, including the one that would have served that very case; the new visually-hidden mixin gets four different class names plus one hand-inlined copy; and the zero-height-caption trick is written out nine times, comment and all. None of that is broken, but it means the next person adding a table has two competing patterns and no signal about which is intended.
Additional findings
These are about code outside the PR's diff — spotted while reading surrounding context.
- src/pages/blog/index.astro:330 — the comment
The inline script at the top of <main> may have added .is-entering...is now stale; that page's<main>became<div class="blog-page">in this PR. - src/pages/partners/affiliate.astro:564, src/components/LanguageSwitcher.astro:135, src/components/PromoBanner.astro:369 — three more hand-rolled visually-hidden blocks predating this PR. Now that
@mixin visually-hiddenexists they're candidates for a mechanical follow-up, which would also settle on one class name repo-wide.
This review was auto-generated. Findings may contain errors — please verify before applying changes.
| &::after { | ||
| content: ''; | ||
| position: absolute; | ||
| inset: 0; |
There was a problem hiding this comment.
The PR description calls the Safari check on this "worth one manual check before merge" — I'd frame it harder, because the failure isn't a small misalignment. The nearest positioned ancestor above <tr> is .comparison-inner (line 269, position: relative), which wraps all seven group tables. So if WebKit computes position on a table row as static, every row's ::after stretches over the entire feature-comparison section, not over its own row — and the last one in paint order takes clicks for the whole block. A visitor clicking "Rule Engine" would land on whatever the final linked row points at.
That argues for the deterministic option even at the cost of hit area: move the containing block to th.col-feature { position: relative }. Cells have been reliably positionable in every engine for years, and the worst case then is a smaller click target rather than a section-wide misdirect. If you'd rather keep the row-wide target, this needs an actual Safari run on /products/thingsboard-pe/ recorded in the PR before merge, not an open question.
| <td class="col-ce"> | ||
| {row.ceText ? ( | ||
| row.ceLinkHref ? ( | ||
| <a href={row.ceLinkHref} target="_blank" rel="noopener noreferrer" class="cell-link">{row.ceText}</a> |
There was a problem hiding this comment.
The full-row overlay and these cell links can't coexist. a::after { inset: 0 } in the row header is positioned, .cell-link is not, so the overlay paints above it and eats the click — any row that has both href and ceLinkHref/peLinkHref loses its cell links silently.
Today nothing hits it: Support Model is the only row with cell links and it has no href. But ComparisonRow advertises both as independent optionals, so this is a trap someone will fall into by adding one field to a data row, with no error to explain it. Either give .cell-link a position: relative; z-index: 1 so it stays above the overlay, or reject the combination at build time the way DataTableValue rejects a blank value — you clearly prefer the loud-failure approach elsewhere in this PR.
| <div class="legal-page"> | ||
| {/* <main>, not a div: these pages do not route through Starlight, so nothing | ||
| else supplies the landmark. */} | ||
| <main class="legal-page"> |
There was a problem hiding this comment.
This is where I'd most like to see something added rather than changed. The PR establishes two invariants that are invisible when violated — exactly one <main> per rendered page, and no icon-only table cells — and neither gets a guard, even though the repo already has the pattern for exactly this: scripts/lint-dual-render.ts, whose own header makes the same argument (builds clean, typechecks clean, only renders wrong).
A ~30-line scripts/lint-landmarks.ts counting <main occurrences per dist/**/*.html would pin the 22-file change down permanently. Without it, the invariant is recorded only in this diff, and the next page copied from an older template reintroduces the nested landmark with nothing to catch it — which is the same way the 20 pages got there in the first place. That's the difference between fixing this once and fixing it again next quarter.
|
|
||
| main { | ||
| main, | ||
| .page-main { |
There was a problem hiding this comment.
Five product pages keep a bare main in this selector for an element they no longer render. In mobile/ and mobile-pe/ the block is scoped, so main can never match and it's simply dead. Here, in paas/ and in thingsboard-edge/ the block is is:global, so width: 100% now applies only to Starlight's <main> — that was already true before the change so it isn't a regression, but it's precisely the accidental cross-boundary rule this PR is otherwise cleaning up. Dropping the main, half would make the point of the change legible.
The wrapper naming is also inconsistent across the 20 pages: five get a newly-invented .page-main (an odd name for the element that deliberately isn't main), development-services keeps .page-wrap, most keep their existing page class, and services/trainings ends up with a classless <div>. If the wrapper is just a styling hook now, something like .page-body applied uniformly would read better than three conventions.
| description="Practical ThingsBoard training led by experts — 15 two-hour sessions for teams up to 8 engineers, covering visualization, admin, SCADA, and more." | ||
| > | ||
| <main> | ||
| <div> |
There was a problem hiding this comment.
This one can go entirely — it's a classless <div> with no styles anywhere in the file and no siblings, so it wraps a slot's children for no reason. <main> → <div> was the mechanical edit, but here the element only existed to be the landmark.
| // Widths are emitted inline on <col>, which `table-layout: fixed` honours in every | ||
| // browser; the per-class width rules below are a redundant fallback. | ||
| const columns: DataTableColumn[] = [ | ||
| { label: '', srLabel: 'Feature', width: '30%', thClass: 'pc-comparison-feature-col' }, |
There was a problem hiding this comment.
All three DataTable callers open their column array with this same incantation — an empty label meaning "no visible header", plus an srLabel that all three set to the literal 'Feature', which is also DataTable's own ?? 'Feature' fallback. That's one convention encoded in four places.
A dedicated rowHeaderLabel?: string (defaulting to 'Feature'), with columns holding only the value columns, would make the shape self-evident and drop the empty-string sentinel — and it would make the two column arrays in ComparisonMatrix (columns from props, dtColumns derived with the feature column prepended) collapse into one, which is currently the only reason that component has to maintain both.
| * here — a component cannot see its siblings — so a caller rendering two tables with | ||
| * identical captions on one page must pass an explicit id for at least one of them. | ||
| */ | ||
| export function captionId(text: string, prefix: string): string { |
There was a problem hiding this comment.
There's a near-identical routine already in the tree: src/components/Pagination/PerPageSelector.astro:19 slugifies with the same .toLowerCase().replace(/[^a-z0-9]+/g, '-') pair to build a deterministic aria-labelledby target. Same purpose, same problem, slightly different edge handling — worth one slugId(text, prefix) util rather than a caption-specific second copy?
Two smaller notes: prefix is a magic string picked per call site ('dt-caption', 'ih-table') with nothing relating them, and the non-Latin fallback hash has no reachable caller — the marketing and landing pages that render these tables aren't localised, so that branch is speculative.
| columns: string[]; | ||
| rows: string[][]; | ||
| /** Overrides the derived id. Needed only when one page has two identical headings. */ | ||
| captionId?: string; |
There was a problem hiding this comment.
This escape hatch is copy-pasted from DataTable (same doc comment) and, like that one, has no caller — CategoryInfoSections passes only heading/intro/columns/rows. It's also where the contract is least reasonable to satisfy: headings come from data and several SectionTables render on one page, so honouring it requires the parent to notice a collision and thread an override down, and nothing prompts it to.
Since the parent is the only component that can see the sibling headings, deriving the id there (index-suffixed) would be simpler than exposing an override nobody sets.
| <h2 class="ih-table__title">{heading}</h2> | ||
| {intro && <p class="ih-table__intro">{intro}</p>} | ||
| <div class="ih-table__scroll"> | ||
| <div class="ih-table__scroll" role="region" aria-labelledby={id} tabindex="0"> |
There was a problem hiding this comment.
tabindex="0" here is unconditional, so every SectionTable becomes a keyboard tab stop announced as a region — including the ones that never overflow, which on wide viewports is most of them. The ARIA pattern this comes from wants the tab stop only when the region actually scrolls; applied unconditionally it adds a stop-and-announce to the tab order on every IoT Hub category page for no benefit.
Same applies to the new scrollable region in Pricing/ComparisonTable and the .table-scroll in ce-vs-pe-diff. If you want to keep it simple, a tiny shared script that sets tabindex only when scrollWidth > clientWidth (and on resize) would give the right behaviour in all three places.
| // The wrapper is a block formatting context, so the table's own bottom | ||
| // margin can no longer collapse out of it and would be added twice. | ||
| table { | ||
| margin-bottom: 0; |
There was a problem hiding this comment.
Is this rule doing anything? The margin-bottom: 16px it's guarding against was moved off table and onto .table-scroll in this same diff, so unless a margin arrives from outside this page's <style> block, the table has no bottom margin to zero — and the comment describes a double-margin that can't occur. Worth deleting both, or keeping the rule with a comment naming where the margin actually comes from.
Also on line 65: the new wrapper's contents weren't re-indented, so the ~120-line <table> and the closing </div> sit at the same depth as the opening <div> and read as siblings. Suggests pnpm format didn't run over this file.
WebKit does not reliably make a positioned <tr> a containing block; the overlay would re-anchor to .comparison-inner and one row's link would cover the whole comparison section. The label cell is the containing block now — positionable in every engine — at the cost of the click target shrinking from the row to the label column. Cell links get their own stacking level so a row carrying both an href and cell links keeps all of them clickable, and the hover selector keys off a data-row-link hook stamped where the row renders instead of encoding the DOM shape in :has().
The largest table work in the branch hand-rolled the exact shell the new primitive emits — caption, colgroup, zero-height announced header — so the codebase carried two competing ways to build the same thing, and the discoverable one was the non-shared one. Each group card is now a DataTable call: the category as a visible caption (captionClass), the announced headers as headerHidden columns, the column classes on the colgroup. Rows stay slot content, so the stretched row link and the value cells are untouched. This also gives captionVisible, captionClass and headerHidden their first callers. DataTable grows one prop for it: captionSr, visually hidden text appended to the caption, so a visible short caption can still carry the what-is-compared context in its accessible name. The Yes/No spans switch to the .dt-vh the primitive already ships, replacing the local copy. The style block follows the documented contract — is:global, nested under .comparison-table — and the caption class is group-caption now, since it is no longer a header row. Geometry measured before and after at 1440/900/500/375 px: zero differences, worst deviation 0.000 px.
The five-declaration trick that hides a caption while keeping its role was written out nine times, six of them carrying the same two-line comment. It is one mixin now, documented once next to visually-hidden, so a future change — or a switch to content-visibility — happens in one place. The header-row variant is the mixin plus border: 0. The four class names this branch coined for hidden value text — dt-vh, comparison-sr, uc-comparison__sr, fast-delivery-sr — converge on visually-hidden, matching the mixin. development-services stops hand-maintaining the mixin body: a namespaced @use gets the mixin without dumping global names into the page's stylesheet, which was the stated reason for inlining it. DataTable's own style block also records the real reason it is global: tbody rows are caller-authored slot content, so a scoped rule could never match a span a caller writes into its rows.
The three keyboard-scrollable table regions shipped tabindex="0" unconditionally, so every table was a stop-and-announce in the tab order even when nothing overflowed — which on wide viewports is most of them. A small shared init now removes the tabindex whenever scrollWidth fits clientWidth and restores it when it stops fitting, re-evaluated via ResizeObserver on the region and its content (a font swap can change scrollWidth without moving the region's own box). The markup still ships tabindex="0", so without JS the region stays scrollable by keyboard — the extra tab stop is the cheaper failure. Verified at load in both states: the ce-vs-pe region keeps its stop at 375px (648 > 329) and loses it at 1440px (980 = 980).
Four sharp edges, one commit: The row-header column was declared as label: '' plus srLabel: 'Feature' in every caller — an empty-string sentinel encoding one convention in four places. label is optional now: omit it and the header cell carries the visually hidden srLabel, which defaults to 'Feature' in exactly one place. Callers declare only what is theirs. DataTableValue's two branches had different shapes — flat yesIcon and yesClass against a string-or-object union for no. It is noIcon/noClass now, with no icon meaning a blank cell, which retires the 'blank' sentinel and its narrowing dance. The caption id helper was one of two copies of the same slug routine — PerPageSelector had the other — so it is a shared slugId(text, prefix) util now, and both call it. Pagination ids verified byte-identical. The hash fallback stays: the util is generic and docs pages come in fourteen languages, so a non-Latin text must not reduce to a bare colliding prefix. thStyle accepts an object, so ComparisonMatrix's hand-concatenated declaration string — four declarations, one ternary, nothing checking the semicolons — is a diffable object; its static declarations (text-align, the 40% feature width) move to the stylesheet that already declared them, and the --col-accent property nothing ever read is gone. CategoryInfoSections passes index-suffixed caption ids down to SectionTable, since only the parent can see that two headings collide.
ComparisonMatrix's decorative backgrounds are absolutely positioned off the table's metrics — header 152px, rows 64px, top margin 64px, the h2's 63px line — but those values lived as unrelated literals split between the frontmatter arithmetic and the stylesheet. Making the caption visible, or changing a row height in one place, silently detached the background columns from the rows. The frontmatter constants are the single source now: the stylesheet consumes them as custom properties set on the section, so both layers move together. The background divs' hand-concatenated style strings become objects while their lines were being rewritten anyway. The comparison card's 21 column percentages get their derivation written down: per-block content widths, the flex shares, and the formula they came from — plus the reason they stay literals, which is that they carry the browser's own flex rounding (within 0.002% of the formula, verified against all 21) and recomputing would shift the render the geometry gate holds at zero.
Five product pages kept a bare main in the width selector for an element they no longer render — in the scoped blocks it could never match, in the global ones it silently restyled Starlight's own main, which is exactly the cross-boundary accident this branch removes. The selector half is gone, and the wrapper settles on one name, .page-body, across all six pages that needed a styling hook (development-services' .page-wrap joins it). The trainings wrapper div is deleted outright: it was classless, styleless and only ever existed to be the landmark. The ce-vs-pe margin-zeroing comment now names where the margin actually comes from — the page's generic p/ol/table/pre rule, not the table's own margin, which this branch had already moved to the wrapper. And a blog comment that still said <main> points at .blog-page, the element that markup became.
Whitespace only (empty under diff -w): the wrapper was added around the table without re-indenting its contents, so the table and the closing div read as siblings of the opening div.
Both invariants this branch establishes are invisible when violated — the page builds, typechecks, lints and visually renders fine — and both got here through page templates copied from older pages. Without a check, the next copied template reintroduces the nested landmark and nothing notices until the next audit. lint:landmarks walks the built output and asserts exactly one <main> per page (redirect stubs exempt: they are bare meta-refresh documents the edge 301 supersedes) and, on the marketing pages that carry the comparison tables, that every table keeps a caption and no cell reads as empty — an icon-only cell strips to nothing, which is precisely the defect class. The development-services timeline keeps its ceiling of intentionally blank schedule cells. Modelled on lint-dual-render: needs a build, not wired into a pipeline yet, run by hand. Verified in both directions — green on the current build, and failing with exit 1 on an injected second landmark and on a cell whose icon lost its hidden text.
The page-level script was placed after </LegalLayout>, and anything after the layout close renders after </html> — the validator flags it as a stray start tag, exactly like the page's pre-existing anchor-js tags. Moved inside the layout slot, where hoisted scripts emit inside <body>.
vvlladd28
left a comment
vvlladd28
left a comment
There was a problem hiding this comment.
Re-review summary
Re-reviewed Make the comparison tables semantic and leave one main per page — verified 20 finding(s) from previous review.
| Status | Count |
|---|---|
| ✅ Resolved | 19 |
| 💬 Acknowledged | 0 |
| ❌ Unresolved | 1 |
Also found 5 new issue(s) in the fix commits, commented inline — one worth acting on, the rest small or optional.
The two blockers are genuinely closed. position: relative moved from <tr> to th.col-feature, so the stretched overlay's containing block is a cell in every engine — the WebKit re-anchoring scenario where one row's link covered all seven tables cannot happen, and it took the deterministic option rather than resting on a manual Safari run. The overlay/cell-link collision is closed twice over: .cell-link gets position: relative; z-index: 1, and the overlay no longer reaches the value columns at all.
The three comments resolved differently than suggested are all resolved better on the evidence given — the column percentages now carry the full per-breakpoint derivation the ask was actually after, making label optional beats a separate rowHeaderLabel because columns still maps 1:1 onto the colgroup, and the ce-vs-pe margin-bottom: 0 rule was one of the two options that comment offered, with the margin's source named.
The cleanup sweep is thorough: nine zero-height hiders down to one caption-hidden mixin, four visually-hidden class names down to one, .page-main/.page-wrap/the classless <div> down to a uniform .page-body, every dead main, selector gone, the inline style string in ComparisonMatrix now an object with six named geometry constants wired through custom properties, :has(> th.col-feature a[href]) replaced by a data-row-link hook, DataTableValue's no union replaced by symmetric noIcon/noClass, slugId shared with pagination, and SectionTable's caption id derived by the parent that can actually see its siblings. Three of the four unused DataTable props gained callers, and — the point of that comment — the largest table in the PR now goes through DataTable instead of routing around it.
On lint:landmarks: I checked whether its TABLE_PAGES list under-covers what this PR fixed, and it doesn't. The two omitted candidates are both structurally safe — partners/hardware/[slug] guards every row behind .length > 0, so an empty array drops the row rather than emitting a blank cell, and SectionTable has no kind: 'table' block anywhere in src/data/iot-hub/sections.ts, so it currently renders nowhere at all (adding iot-hub would make the check fail on no tables found). The script also isn't wired into a pipeline, but neither is lint:dualrender, ci.yml deliberately has no build step, and the linkcheck pipeline lives outside this repo — so that's a follow-up rather than something this PR can do. The one thing worth correcting is the PR description, which says the check guards the invariants "in every build"; it's a manual post-build command, same as lint:dualrender.
Finding details
- ✅ src/components/Landing/ComparisonTable.astro — stretched row link depended on
positionapplying to<tr>; WebKit would have re-anchored it to.comparison-innerand one row's link would swallow clicks across all seven tables — Fixed in code: the containing block is nowth.col-feature. - ✅ src/components/Landing/ComparisonTable.astro — the row overlay painted above
.cell-linkand would silently eat its clicks on any row with bothhrefandceLinkHref/peLinkHref— Fixed in code:.cell-linkisposition: relative; z-index: 1, and the overlay no longer covers the value cells. - ✅ src/layouts/LegalLayout.astro — the one-
mainand no-empty-cell invariants had no guard — Fixed in code:scripts/lint-landmarks.tsadded, and its page list checks out (see the summary above). - ✅ src/pages/products/thingsboard-pe/index.astro — five pages kept a dead
main,in a selector; wrapper naming split across three conventions — Fixed in code: everymain,half dropped,.page-bodyused uniformly. - ✅ src/pages/services/trainings/index.astro — classless
<div>wrapping a slot for no reason — Fixed in code: removed. - ❌ src/components/DataTable.astro — the
is:globalstyling contract is enforced only by a doc comment; a ninth table author with a scoped<style>gets a table that builds, typechecks and lints clean and renders unstyled — Still present. Theis:globalsub-point is resolved with a good reason (callers write.visually-hiddeninto slot content, which a scoped rule could never match), but the contract itself is unchanged andDataTablestill isn't in CLAUDE.md's Available Components list, even though this PR edits that file. See inline — the doc entry is the cheap half; I'm not asking for the custom-property refactor. - ✅ src/components/DataTable.astro — four props with no caller — Fixed in code:
captionVisible,captionClassandheaderHiddenall gained callers whenLanding/ComparisonTablemoved ontoDataTable. OnlycaptionIdis still speculative, which is a fair escape hatch to keep givenSectionTableneeded exactly that. - ✅ src/components/Landing/ComparisonTable.astro — the largest new table bypassed the primitive the same PR introduced — Fixed in code: the seven group cards render through
DataTable, and the class names now describe the elements they're on. - ✅ src/components/Landing/ComparisonTable.astro — 21 four-decimal percentages with no recorded derivation — Resolved differently, and correctly: the literals stay, but the comment now records content width per breakpoint, the flex shares, the formula, and that the values carry the browser's own flex rounding within 0.002%. That answers "has anything moved?" — which is what the comment was after — and recomputing in SCSS would have shifted a render the geometry gate holds at zero.
- ✅ src/styles/_variables.scss — the zero-height trick written out nine times — Fixed in code: one
caption-hiddenmixin, rationale documented once at the definition. - ✅ src/pages/services/development-services/index.astro — hand-copied mixin body justified by a reason that had a one-line fix — Fixed in code:
@use … as vplus@include v.visually-hidden, and the class renamed so all new call sites agree onvisually-hidden. - ✅ src/components/Landing/ComparisonMatrix.astro — four declarations hand-concatenated into an inline style string, nothing type-checking it — Fixed in code:
thStylenow accepts an object. - ✅ src/components/Landing/ComparisonMatrix.astro — six unnamed geometry literals mirroring values that live in the stylesheet — Fixed in code: named constants, emitted as custom properties the stylesheet reads back. One literal escaped; see inline.
- ✅ src/components/Landing/ComparisonTable.astro —
:has(> th.col-feature a[href])encoded too much markup shape — Fixed in code:data-row-linkon the<tr>. - ✅ src/components/DataTableValue.astro — asymmetric
yesIcon/yesClassvs a'blank' | {…}union — Fixed in code:noIcon/noClass, sentinel retired. - ✅ src/components/Pricing/ComparisonTable.astro — the empty-
label+srLabel: 'Feature'convention encoded in four places — Resolved differently:labelis optional and the'Feature'default lives in one place. Keepingcolumns1:1 with thecolgroupis the better trade than a separaterowHeaderLabel. - ✅ src/util/caption-id.ts — near-duplicate of
PerPageSelector's slugify — Fixed in code: oneslugId(text, prefix), used by both. - ✅ src/components/IotHub/SectionTable.astro — a
captionIdoverride only the parent could ever satisfy — Fixed in code:CategoryInfoSectionsderives it, index-suffixed. - ✅ src/components/IotHub/SectionTable.astro — unconditional
tabindex="0"on regions that never overflow — Fixed in code:initScrollRegionTabindexsyncs the tab stop to actual overflow, in all three places named. Therole="region"half is still unconditional; see inline, though the scale is small. - ✅ src/pages/ce-vs-pe-diff/index.astro — is the
margin-bottom: 0rule dead, and the wrapper contents weren't re-indented — Resolved on evidence: the rule is live (the page's genericp, ol, table, prerule is the source, now named in the comment) and the table is re-indented.
This re-review was auto-generated. Findings may contain errors — please verify before applying changes.
| const RULE_TOP = 108; // horizontal rule inside each background, from its top | ||
| const RULE_W = 2; | ||
|
|
||
| const numCols = columns.length; |
There was a problem hiding this comment.
The six constants above cover the vertical geometry, but the horizontal one didn't get the same treatment: 40 here is the feature column's width, and it's duplicated as a literal in .feature-col { width: 40%; } further down this same file. That's exactly the coupling the constants block was extracted to fix — change one and the value columns silently stop summing to 100%.
A const FEATURE_COL_PCT = 40 emitted as --matrix-feature-col-w alongside the other four, with the stylesheet reading it back, would put it on the same footing as the row heights.
| * size. The callers disagree on all of them — Pricing collapses its borders while the | ||
| * Edge matrix separates them by 16px — so appearance stays entirely with the caller. | ||
| * | ||
| * Styling contract: a caller moving its table shell in here must switch its <style> |
There was a problem hiding this comment.
Still unresolved: the is:global on this component's own block now has a real justification and it's the right one — callers do write .visually-hidden into slot content, and Landing/ComparisonTable relies on exactly that. But the contract itself is unchanged: a ninth table author who writes a normal scoped <style> still gets a table that builds, typechecks, lints clean and renders unstyled, with nothing at the call site to warn them.
I'm not pushing the custom-property refactor from last time — that's a large change for a risk this shape. The cheap half is the one that didn't happen: DataTable and DataTableValue never made it into CLAUDE.md's Available Components list, even though this PR edits that file. Two lines there naming the is:global requirement would make the contract discoverable at the moment someone goes looking for a table component, which is the only moment it matters.
| "lint:eslint": "eslint .", | ||
| "lint:steps": "node --experimental-transform-types ./scripts/lint-steps.ts", | ||
| "lint:dualrender": "node --experimental-transform-types ./scripts/lint-dual-render.ts", | ||
| "lint:landmarks": "node --experimental-transform-types ./scripts/lint-landmarks.ts", |
There was a problem hiding this comment.
Indentation: two tabs here while every sibling line uses four spaces, and CLAUDE.md's code style section says spaces for JSON. Looks like pnpm format didn't run over this file.
Unrelated to the line itself, but this is the natural place to mention it: the PR description says this check "now guards the one-main and no-empty-cell invariants in every build". It doesn't — it's a manual post-build command, exactly like lint:dualrender, which is a fine place for it to be (ci.yml has no build step and the linkcheck pipeline lives outside this repo), but the description overstates what landed. Worth a word change so nobody assumes the invariant is enforced when it isn't yet.
| // Chrome dev-tools' responsive mode (where the primary input | ||
| // stays "mouse" and the hover query still resolves to true). | ||
| @media (hover: hover) and (min-width: 901px) { | ||
| .comparison-group tbody tr[data-row-link]:hover { |
There was a problem hiding this comment.
Minor, and arguably an acceptable cost of the fix above it: the overlay moved onto the label cell, but the hover affordance didn't move with it. This rule still tints the whole row and grows the label font across it, while only th.col-feature — about 34% of the row width at desktop — actually navigates.
The cursor does already tell the truth (link cursor over the label, arrow over the value cells), so it's not silent, and shrinking the tint to one cell would be a real loss of affordance. Probably the smallest useful change is leaving the tint alone and being content that the cursor disambiguates — but since the two matched before this commit and don't now, it's worth being a deliberate choice rather than a leftover.
There was a problem hiding this comment.
Deliberate, and now recorded as a comment on the hover rule: the tint spans the row because it marks the row being read, while the cursor already marks where the link is — shrinking the tint to the label cell would trade a real reading affordance for consistency with the click target. The other four comments are addressed in 219209d: DataTable/DataTableValue are in CLAUDE.md's component list with the is:global contract named, the 40 joined the matrix geometry constants as --matrix-feature-col-w, role=region now leaves with the tab stop (verified in both states), and the package.json line is space-indented. The PR description no longer claims the guard runs in every build.
| if (el.scrollWidth > el.clientWidth) { | ||
| el.setAttribute('tabindex', '0'); | ||
| } else { | ||
| el.removeAttribute('tabindex'); |
There was a problem hiding this comment.
Small one. The tab stop is now honest, but only half the pattern is: role="region" and aria-labelledby stay on the wrapper unconditionally, and a region with an accessible name is a landmark — so a table that doesn't overflow still appears in the landmark list, named after its caption.
The scale is modest (one wrapper on /pricing/, one on /ce-vs-pe-diff/; SectionTable has no table data behind it today), so this isn't urgent. But sync already knows whether the region scrolls, so toggling role alongside tabindex is about three lines, and the no-JS fallback stays correct either way since the markup ships all three.
DataTable and DataTableValue join CLAUDE.md's component list, naming the is:global styling contract at the one moment it matters — when someone goes looking for a table component. The feature column's 40 joins the named matrix geometry as a constant read back by the stylesheet, instead of living once in the width math and once as a literal width rule. Scroll regions now drop role=region together with the tab stop when they do not overflow — a region that cannot scroll was still a named landmark — verified in both states at 1440/375. The lint:landmarks line in package.json matches its space-indented siblings, and the full-row hover tint is recorded as a deliberate choice: it marks the row being read while the cursor marks where the link is.
The two sibling comparison tables carried the same width coupling that ComparisonMatrix just shed: EdgeComparisonMatrix wrote 40% and three 20%s once in the columns array and again in the col-class rules, and Pricing/ComparisonTable did the same with 30% and 17.5%. The doubling is deliberate — inline width is immune to specificity drift, the class rule is the fallback a fixed table needs when the col width is unreachable — so both sides now read one local constant, the stylesheet through a custom property on the section. Eight literals become four, and the emitted values are unchanged. scroll-region-tabindex is renamed to scroll-regions, since it manages role alongside the tab stop and the old name sent anyone looking for what strips role=region to the wrong file. Its docblock now states that the role of a data-scroll-region element belongs to the function: it sets region and removes it again, so a consumer ships role="region" in the markup and never any other role. CLAUDE.md's DataTable entry said caption then colgroup, though colgroup is a prop and ComparisonMatrix passes false, and it left out scrollable — the one prop with a runtime partner, since it emits the role="region" wrapper initScrollRegions then toggles.
2 participants
Makes the comparison tables on the marketing pages readable as tables — by a screen reader and by anything that reads the HTML as text — and leaves exactly one
mainlandmark per page. No visual change.Ref: PROD-8679
The problem
Values were carried by icons alone, so a text extraction of a row returned nothing:
What changed
A shared primitive.
src/components/DataTable.astroemits caption → colgroup → thead (th scope="col") → tbody slot; callers pass rows whose first cell is ath scope="row". It sets no box metrics at all — the callers disagree on every one of them, so appearance stays with the caller.DataTableValue.astrorenders a cell's value as text in every case, with the icon marked decorative.Eight tables migrated or repaired. Pricing, the Cloud and Edge matrices, the Google IoT Core comparison, the CE-vs-PE reference table, the hardware spec table, the use-case comparison, and both tables on the development-services page.
The feature comparison grid became real tables. It was a div grid, so nothing about it was a table to begin with. Each group card is now its own table with its title as the caption. The row-wrapping anchor is gone — its accessible name was the entire row including both values; the link now lives in the row header, stretched over the label cell. The cell — not the row — is the containing block: WebKit does not reliably honour
positionon a table row, and the fallback ancestor would have put one row's overlay over the whole section.One
mainper page. Starlight already emits one, so the page-level element was a second, nested landmark. 20 pages plus two layouts.LegalLayouthad the opposite defect — no landmark at all on its pages — and gained one.Results
/pricing/empty cells/products/paas//products/thingsboard-edge//products/thingsboard-pe//ce-vs-pe-diff/display: blockstripped the table rolemainStray end tag "td")Every caption also names its subject, so a table lifted out of the page still says what it compares —
Platform CorebecamePlatform Core — ThingsBoard Community Edition compared with Professional Edition.Verification
Re-run in full after the review-response commits:
pnpm build:fastclean (4,359 pages) ·lint:linkcheck·lint:dualrender·lint:slugcheck·lint:steps·eslint·astro checkall pass — plus the newpnpm lint:landmarks— a manual post-build check likelint:dualrender, verified to fail on an injected second landmark and on a cell whose icon loses its hidden text; wiring it into a pipeline is a follow-up, since ci.yml has no build step. W3C Nu on the 20 affected pages: 0 table-structure errors. The remaining Nu errors are pre-existing classes —<source>withoutsrc,<button>inside<a>, and the cookie banner'saria-label, which the ticket lists as out of scope.Geometry was measured before and after at 1440 / 900 / 500 / 375 px on the pages that changed most: card boxes, caption painted regions, divider spans, text origins and every icon position match, with icons within 0.02px.
Merge notes
Main moved 10 commits while this was in progress, two of which overlapped:
products/mqtt-broker/index.astrowas deleted (TBMQ related info migration to new website #585, refactor(tbmq): drop TBMQ legal pages superseded by tbmq.io #630). Its edits here are dropped, andLanding/ComparisonTablenow has a single consumer. This also removes the one visual deviation this PR previously carried: TBMQ had a group mixing a text-link row with an icon row, and since column widths are table-wide, that icon shifted 15px at 375px. With the page gone, there is no deviation left.externalLinkAttrs()(TBMQ related info migration to new website #585) on the same lines this PR replaced. Adopted for the row link, where the inline test and the helper agree.One thing deliberately not folded in: the two cell-links still set
target/relexplicitly. The CE one is external, but the PE one is a local path, and both currently open in a new tab — so switching them to the helper would change behaviour, since its policy gives local paths neither attribute. That looks like a pre-existing policy violation worth its own change rather than a silent rider here.One thing reviewers should know
The formula in the ticket does not work.
calc(20px + (100% - 40px) / 3)is silently dropped on acolelement — and on a header cell — leaving the columns as equal thirds, which is the exact misalignment it was written to prevent. Column widths are plain per-breakpoint percentages instead, derived from the measured render.Review response
All 20 review comments are addressed in the last 10 commits, most-severe first: the row overlay anchors to the label cell (kills the Safari re-anchoring scenario outright), cell links sit above the overlay and the hover keys off a
data-row-linkhook;lint:landmarkspins the invariants; scroll regions keep their tab stop only while they actually overflow; the seven group cards render throughDataTable— geometry re-measured at 1440/900/500/375, zero differences — which also givescaptionVisible/captionClass/headerHiddentheir callers; the nine zero-height hiders are onecaption-hiddenmixin and the sr classes agree onvisually-hidden; the matrix geometry is named constants consumed by both the frontmatter and the stylesheet;DataTableValueis symmetric (noIcon/noClass); the caption-id helper is a sharedslugIdalso used by pagination (ids byte-identical); wrapper naming, dead selectors and stale comments are swept.Three comments were resolved differently than suggested, on evidence:
labeloptional (srLabel defaults to'Feature'in one place) rather than a separaterowHeaderLabelprop, socolumnsstill maps 1:1 onto thecolgroup.margin-bottom: 0rule is live, not dead — the page's genericp, ol, table, prerule feeds the table a 16px margin the BFC wrapper would double. The comment now names that source.One defect found during re-verification and fixed: the page-level script this branch added to
/ce-vs-pe-diff/was emitted after</html>(anything placed after the closing layout tag is); it now renders inside the document. The same pre-existing pattern on 21 other files is a separate follow-up branch.Left out, deliberately
Medium to LowandSQL + NoSQLis untouched — normalising it is a content decision./docs/*, outside this ticket's scope. They share the same missing-scope defect and are a mechanical follow-up now that the primitive exists.