Review 5310

Review 5310 — useTableSelection, opt out of the checked-row highlight

#5310 · ernestt · internal Head reviewed f2546e6bacbc94b932216a3da8cc75ae7fc5e83e Verdict request-changes, withdrawn same session, then approve · merged 4e5f148e A/B vibe test — both arms drafted; tied on findings, both wrong on the remedy. This review produced four rules: R7c · R16e · R3i, and confirmed R15e.

Problem

useTableSelection paints checked rows by writing el.style.backgroundColor onto each <tr> from a ref callback. An inline declaration outranks every class, so a product whose rows already mean something by colour — "open in the detail panel" — cannot reclaim the row background, and a theme cannot restyle the wash.

Solution

One optional config field, hasRowHighlight, read as config.hasRowHighlight ?? true, threaded into applyRowSelectionStyle. The background write moves out of the isSelected branch so it is written on every pass — which is what makes a runtime flip settle instead of stranding the colour. aria-selected untouched.

Impact

Invisible until someone opts in; default path verified byte-identical to main in Chromium. Reaches every future builder permanently as public config on a published package.

API

+ UseTableSelectionConfig.hasRowHighlight?: boolean = true   (public, @astryxdesign/core/Table)
~ applyRowSelectionStyle(el, isSelected, hasRowHighlight)     (module-private, 3rd param)

Theme targets

None new. themeProps('table-row') publishes no state, so selection is not reachable from a theme at all — no data-selected to key off.

Breaking

API no · Visual no (nothing grew) · Theme no.

Performance

No new effects, subscriptions or dependencies. The style-write count is unchanged — main's else branch already wrote '' on every pass.

Judgement — and the two corrections

Posted as request-changes, then withdrawn and approved within the session.

1. The opt-out still clears a background set via htmlProps.style — WRONG, withdrawn. main already writes '' on the unselected branch, so the behavior is pre-existing and not the author's (R13). Worse, the proposed remedy — skip the write when the flag is false — would strand the wash on an already-painted row when the flag flips, which the comment directly above that line says it is avoiding. → R3i.
2. Selection is not reachable from a theme — real, not blocking, follow-up.

Cindy's three rulings on the arms' shared remedy (both proposed keying the wash off [aria-selected="true"] in StyleX):

• StyleX is the ideal, not the ruling. A direct ref write may be deliberate, for render count. Ask why before proposing the layer.
• Never key styling off ARIA. It is an accessibility contract, not a styling hook. Key off theme-exposed state; if the state is not exposed, expose it.
• A new prop can be right. R1g-surface is a high bar, not a ban — and the ossification finding dissolved once its alternative fell.

→ R7c. And because the verdict came from the implementation, not the prop, → R16e: mark which finding blocks; a review with no [BLOCKS] line cannot be request-changes.

Evidence

Both arms captured full before/after/prototype matrices in real Chromium — default wash, flag off, consumer style vs xstyle, theme override attempt, sticky columns, and a working prototype of the class-keyed alternative.