fix(plugin-dashboard): the widget grid passes only whitelisted DOM props to its container (#4432)

.changeset/dashboard-grid-dom-prop-whitelist-4432.md

@@ -0,0 +1,17 @@
+---
+'@object-ui/plugin-dashboard': patch
+---
+
+`DashboardRenderer`'s widget grid now passes only whitelisted DOM props to its container (objectui#4432)
+
+`view:dashboard` resolves to this component, so `SchemaRenderer` handed it the dashboard node's own keys, the contents of the node's `props` container, the ARIA it resolved and the host's trailing props — and every key the component did not destructure was spread raw onto the grid container. React writes unknown lowercase attributes through in silence and stringifies object values, so the failure was invisible. Measured through the real SDUI path: **13 non-DOM attributes**, including `events="[object Object]"`, `props="[object Object]"` and a camelCase `arialabel` sitting next to the resolved `aria-label`, so the element carried each ARIA value twice under two spellings — one of them meaningless to assistive technology.
+
+The container is now consume-or-whitelist per objectui#4425 phase 2: only `toDomProps`' output reaches the element, and it is spread FIRST so the component's own computed attributes stay authoritative. The resolved `aria-label` / `aria-describedby`, `role`, `id`, `tabIndex`, `className` and the `data-*` family still arrive — dropping them would have been an accessibility regression dressed as a leak fix, so the new pin asserts the delivered set exactly, not just the absent one. Both layout branches are covered: the responsive desktop grid and the mobile stack spread the same props onto the same host element.
+
+Three behaviours move with the spread, all of them consequences of a trailing spread that used to override the component's own computed props:
+
+- **`onClick` now has one carrier.** It is a declared DOM pass-through key AND this container computes a design-mode background handler, and the old spread let the incoming handler replace the computed one — so a host that passed `onClick` silently lost background deselection. Both run now, container affordance first. An authored non-function `onClick` (SDUI spells click behaviour `events: { onClick }`, which is data and is dropped) is ignored instead of handed to React, which used to throw on it.
+- **An authored `style` no longer replaces the computed grid layout.** `style` is not in the SDUI pass-through set, and this container computes its own `gridTemplateColumns` / `gridAutoRows` / `gap`; an authored `style` used to overwrite all of it and collapse the grid.
+- **An authored `data-user-actions` no longer overrides the value computed from the `userActions` prop.** The `data-*` family still passes the whitelist; only this one collision with a computed attribute resolves the other way now.
+
+The injected `disabled` verdict is also dropped rather than forwarded. Nothing in this component ever read it: it only became a `disabled` attribute on a container element that has no such attribute, which is the leak, not a behaviour.

packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx

@@ -14,50 +14,66 @@
  *
  * ## What this is, and what it deliberately is NOT
  *
- * This is phase 1 of objectui#4425: **measure before converging.** It renders
- * every target through the real SDUI path with planted canaries and checks every
- * attribute of every rendered element against what HTML actually defines. It
- * changes **no widget contract and no widget source** — leaks it finds are
- * RECORDED in {@link LEAK_LEDGER} with a reason and an owning issue, never
- * silently tolerated and never fixed here.
- *
- * Phase 2 — promoting `toDomProps`' whitelist to the SDUI widget contract, i.e.
- * option 1 on objectui#4425 — is the maintainer's decision and is explicitly out
- * of scope. It is now to be decided on THIS GATE'S READING rather than on an
- * assumption about how wide the leak is. The reading, measured here:
+ * This file landed as phase 1 of objectui#4425: **measure before converging.**
+ * It renders every target through the real SDUI path with planted canaries and
+ * checks every attribute of every rendered element against what HTML actually
+ * defines. It changes **no widget contract and no widget source** — leaks it
+ * finds are RECORDED in {@link LEAK_LEDGER} with a reason and an owning issue,
+ * never silently tolerated and never fixed here.
+ *
+ * Phase 2 is now RULED (objectui#4425, comment 5270759246): option 1 —
+ * `toDomProps`' whitelist is promoted to the SDUI widget contract, and the
+ * migration runs as per-package cards that each delete their own ledger rows.
+ * So this gate's job changed without its mechanism changing: it was the
+ * measurement the ruling was waiting for, and it is now the ratchet that
+ * migration is graded against. The reading, kept at current truth as each card
+ * lands:
  *
  *   | package          | targets | targets leaking | leaked attributes |
  *   |------------------|---------|-----------------|-------------------|
  *   | plugin-charts    |       9 |               0 |                 0 |
  *   | plugin-calendar  |       3 |               0 |                 0 |
- *   | plugin-chatbot   |       3 |               2 |          14 each  |
- *   | plugin-dashboard |       8 |               3 |        7 / 9 / 13 |
- *
- * 5 of 23 targets leak. The two clean packages are clean for opposite reasons
- * worth keeping straight: `plugin-charts` never spreads the node onto its
- * container at all, while `plugin-calendar`'s components take a declared prop
+ *   | plugin-chatbot   |       3 |               0 |                 0 |
+ *   | plugin-dashboard |       8 |               2 |             7 / 9 |
+ *
+ * **2 of 23 targets leak**, and both are in {@link LEAK_LEDGER} below:
+ * `plugin-dashboard:metric` and `plugin-dashboard:metric-card`, the open tail
+ * objectui#4425 owns directly. Two migration steps have closed their rows since
+ * the phase-1 measurement:
+ *
+ *   - `plugin-chatbot:chatbot` / `chatbot-enhanced` — 14 attributes each,
+ *     objectui#4431 / PR #4485, which also lifted `toDomProps` to
+ *     `@object-ui/core` so later cards consume one executor.
+ *   - `view:dashboard` — `DashboardRenderer`'s widget-grid container, 13
+ *     attributes, objectui#4432 / this file's most recent edit.
+ *
+ * The three packages now reading 0 are NOT clean for the same reason, and the
+ * difference is worth keeping straight: `plugin-charts` never spreads the node
+ * onto its container at all; `plugin-calendar`'s components take a declared prop
  * list and drop what they do not name, so the node's keys never reach an
- * element.
+ * element; `plugin-chatbot` and `DashboardRenderer`'s grid reach zero by
+ * FILTERING — they still spread, through `toDomProps`.
  *
  * `calendar-view` was originally swept with the `events` canary WITHHELD, because
  * authoring it crashed the component outright (objectui#4433) — a worse failure
  * than the leak this gate was looking for, and one that would have read as a
  * clean pass. That is fixed, so the omission is gone and the target is swept
  * with the full canary set; section 5 below carries what is left of it.
  *
- * ## The divergence this repo is living with, recorded deliberately
+ * ## The divergence that decided phase 2, and what is left of it
  *
- * objectui#4425's option 3 was "record the divergence rather than converge", and
- * it happens here whichever way phase 2 goes, because both answers are live in
- * the tree at once and this gate observes both:
+ * objectui#4425's option 3 was "record the divergence rather than converge".
+ * Both answers were live in the tree at once and this gate observed both, which
+ * is how the ruling got its evidence:
  *
- *   - `packages/fields` uses a **whitelist** (`widgets/toDomProps.ts`, #3291):
- *     keep the declared DOM pass-through keys, drop everything else. Its own
- *     gate (`fields/src/__tests__/widget-dom-leak-e2e.test.tsx`) is untouched by
- *     this file and stays the reference implementation of the technique.
- *   - `packages/plugin-dashboard` uses a **deny-list** (`src/schemaHostProps.ts`,
- *     #4357/PR #4428): destructure seven measured non-DOM props out, spread the
- *     rest.
+ *   - a **whitelist** (`toDomProps`, #3291): keep the declared DOM pass-through
+ *     keys, drop everything else. Born in `packages/fields`, whose own gate
+ *     (`fields/src/__tests__/widget-dom-leak-e2e.test.tsx`) is untouched by this
+ *     file and stays the reference implementation of the technique; the
+ *     MECHANISM now lives in `@object-ui/core` (`utils/dom-props.ts`, #4431) so
+ *     every plugin package can reach it.
+ *   - a **deny-list** (`plugin-dashboard/src/schemaHostProps.ts`, #4357/PR
+ *     #4428): destructure seven measured non-DOM props out, spread the rest.
  *
  * The deny-list is correct for the seven props it enumerates — this sweep
  * confirms all seven are gone from `metric` and `metric-card`. What it cannot
@@ -67,8 +83,15 @@
  * `metric-card`, where `zzcanary` / `reference_to` / an authored
  * `props: { colorVariant }` all land as attributes while every one of the seven
  * named keys is correctly stripped. A deny-list bounded by enumeration cannot be
- * finished; a whitelist bounded by declaration can. That contrast is the
- * measurement phase 2 was waiting for.
+ * finished; a whitelist bounded by declaration can. That contrast IS the
+ * measurement phase 2 waited for, and the ruling went to the whitelist on it.
+ *
+ * So the divergence is no longer a standing state of the repo — it is a
+ * migration in progress, and the two rows still below are the last of it inside
+ * these four packages. `plugin-dashboard` is now MIXED by design: its
+ * `DashboardRenderer` grid container filters through `toDomProps` (#4432) while
+ * the two KPI components still run the deny-list, and the two surviving rows are
+ * precisely that difference, measured.
  *
  * ## Why this file lives in `packages/app-shell`
  *
@@ -542,23 +565,6 @@ const LEAK_LEDGER: Readonly<Record<string, LedgerEntry>> = {
       'the DOM as an attribute instead of rendering it.',
     issue: 'objectui#4425',
   },
-
-  /* ── plugin-dashboard: the container the KPI cards sit in ────────────────── */
-  //
-  // Same package as the two rows above, different element and NOT in #4357's
-  // scope: `DashboardRenderer`'s widget grid still spreads. `datasource` is
-  // absent here because this component consumes the adapter.
-  'view:dashboard': {
-    attributes: [
-      'ariadescribedby', 'arialabel', 'bind', 'colorvariant', 'events', 'name',
-      'props', 'reference_to', 'zzcanary', 'zzcanarycamel', 'zzcanarynum',
-      'zzcanaryobj', 'zzcanaryprop',
-    ],
-    reason:
-      "DashboardRenderer's widget-grid container ends in a bare `{...props}` " +
-      'spread, so the dashboard node\'s own keys land on it.',
-    issue: 'objectui#4432',
-  },
 };
 
 /* ════════════════════════════════════════════════════════════════════════════

packages/plugin-dashboard/src/DashboardRenderer.tsx

@@ -15,6 +15,7 @@ import {
   dashboardFilterVariableDefs,
   buildWidgetScopedFilter,
   mergeFilters,
+  toDomProps,
 } from '@object-ui/core';
 import { cn, Card, CardHeader, CardTitle, CardContent, Button, getLazyIcon } from '@object-ui/components';
 import { forwardRef, useState, useEffect, useCallback, useMemo, useRef, Fragment } from 'react';
@@ -401,6 +402,35 @@ const DashboardRendererInner = forwardRef<HTMLDivElement, DashboardRendererProps
       }
     }, [designMode, onWidgetClick]);
 
+    /**
+     * The click channel of the host element, with ONE carrier (objectui#4432).
+     *
+     * `onClick` is a declared DOM pass-through key of the SDUI widget contract,
+     * and this container also computes its own background handler — so both
+     * have a legitimate claim on the same prop. The old trailing `{...props}`
+     * spread resolved that by letting the incoming handler REPLACE
+     * `handleBackgroundClick` outright, which silently switched design-mode
+     * background deselection off for any host that passed one. Dropping the
+     * incoming handler instead would be the mirror failure: a whitelisted key
+     * that type-checks, reads as supported, and never arrives.
+     *
+     * Both run, container affordance first. A non-function `onClick` (an
+     * authored JSON string — SDUI spells click behaviour `events: { onClick }`,
+     * which is DATA and is dropped by the whitelist) is ignored rather than
+     * handed to React, which would throw on it.
+     *
+     * Deliberately NOT wrapped in `useCallback`: it closes over `props.onClick`
+     * read out of the rest object, which the React Compiler cannot prove stable,
+     * so a manual memo here is one it reports as unpreservable
+     * (`Compilation Skipped: Existing memoization could not be preserved`) and
+     * then declines to optimize the component around. It lands on a plain
+     * element with no memoized child, so identity churn costs nothing.
+     */
+    const handleHostClick = (e: React.MouseEvent) => {
+      handleBackgroundClick(e);
+      if (typeof props.onClick === 'function') props.onClick(e);
+    };
+
     // --- Drag-and-drop reordering (design mode only) ---------------------
     // Powered by @dnd-kit. Because each widget renders with
     // `gridColumn: span W` (no explicit x/y), array order *is* visual order,
@@ -865,6 +895,35 @@ const DashboardRendererInner = forwardRef<HTMLDivElement, DashboardRendererProps
 
     const userActionsAttr = userActions ? JSON.stringify(userActions) : undefined;
 
+    /**
+     * What may legitimately become a DOM attribute on this container
+     * (objectui#4432, migration step 2 of objectui#4425 phase 2).
+     *
+     * `view:dashboard` resolves to this component, so `SchemaRenderer` hands it
+     * the dashboard node's OWN keys, the contents of the node's `props`
+     * container, the ARIA it resolved, its evaluated `disabled` verdict and the
+     * host's trailing props. Everything this component does not destructure
+     * used to be spread raw onto the grid `<div>` below, and React writes
+     * unknown lowercase attributes through in silence while stringifying object
+     * values. Measured through the real SDUI path: **13 non-DOM attributes**,
+     * including `events="[object Object]"`, `props="[object Object]"` and a
+     * camelCase `arialabel` sitting next to the resolved `aria-label` — the same
+     * value twice, one spelling of it meaningless to assistive technology.
+     *
+     * Per objectui#4425 phase 2 the answer is the whitelist, not another
+     * deny-list: `toDomProps` keeps `id` / `className` / `role` / `tabIndex` /
+     * `aria-*` / `data-*` and drops the rest. It is spread FIRST so this
+     * component's own computed attributes (`className`, `style`,
+     * `data-user-actions`, the click channel above) stay authoritative — the
+     * old trailing spread let an incoming key overwrite each of them.
+     *
+     * `datasource` is absent from the 13 because this component destructures
+     * the adapter and hands it to its own `SchemaRenderer` calls; that is the
+     * one key objectui#4428 shipped a pass without, and it never reached this
+     * spread.
+     */
+    const hostDomProps = toDomProps(props);
+
     const refreshButton = onRefresh && (
       <div className={cn("flex items-center justify-end gap-3 mb-2", !isMobile && "col-span-full")}>
         {recordCountBadge}
@@ -902,7 +961,7 @@ const DashboardRendererInner = forwardRef<HTMLDivElement, DashboardRendererProps
       const otherWidgets = schema.widgets?.filter((w: DashboardWidgetSchema) => w.type !== 'metric') || [];
 
       const mobileBody = (
-        <div ref={ref} className={cn("flex flex-col gap-4 px-4", className)} data-user-actions={userActionsAttr} onClick={handleBackgroundClick} {...props}>
+        <div ref={ref} {...hostDomProps} className={cn("flex flex-col gap-4 px-4", className)} data-user-actions={userActionsAttr} onClick={handleHostClick}>
           {headerSection}
           {filterBar}
           {refreshButton}
@@ -937,6 +996,7 @@ const DashboardRendererInner = forwardRef<HTMLDivElement, DashboardRendererProps
     const desktopBody = (
       <div
         ref={ref}
+        {...hostDomProps}
         className={cn(
           "grid",
           // Content-sized rows only for the responsive flow layout. The
@@ -958,8 +1018,7 @@ const DashboardRendererInner = forwardRef<HTMLDivElement, DashboardRendererProps
             gap: `${gap * 0.25}rem`
         }}
         data-user-actions={userActionsAttr}
-        onClick={handleBackgroundClick}
-        {...props}
+        onClick={handleHostClick}
       >
         {headerSection}
         {filterBar}