Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions .changeset/9529-detail-section-declares-icon.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
---
'@object-ui/plugin-detail': minor
---

An authored `detail-section` node now takes `icon`, and draws it.

`DetailSection` draws `section.icon` before the title in both of its header
branches, but the `detail-section` registration did not declare `icon` among
its `inputs`, and the node adapter folds only declared names into the section
it hands the component. So an author who wrote `icon` on the node was warned
`unknown-prop "icon"` by the JSX-page compiler's validator — which judges an
authored page against these same `inputs` — and the header drew no icon.

The registration now declares `{ name: 'icon', type: 'string' }` (a Lucide icon
name such as `map-pin`), and the adapter folds it like the other declared
inputs. A non-string `icon` now draws `type-mismatch` instead of `unknown-prop`.

**Clause-②: yes** — the authoring surface of `detail-section` widens by one
key, `icon`, which the renderer already honoured. No other key's verdict moves:
`name`, which the block neither declares nor reads, still draws `unknown-prop`.
No exported symbol is added, removed, renamed or retyped — the package entry
re-exports neither `DetailSectionNode` nor `DETAIL_SECTION_NODE_INPUTS`. The
tag is outside the public block tier, so the generated `sdui.manifest.json` and
`sdui-intrinsics.d.ts` never carried it and do not change.
17 changes: 11 additions & 6 deletions packages/plugin-detail/src/DetailSectionNode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@

/**
* The component the `detail-section` TAG is registered against — the seam that
* makes the block's eight declared `inputs` true (objectui#8626).
* makes the block's declared `inputs` true (objectui#8626).
*
* ## The defect this closes
*
* `ComponentRegistry.register('detail-section', …)` declares eight FLAT inputs
* `ComponentRegistry.register('detail-section', …)` declared eight FLAT inputs
* — `title`, `description`, `fields`, `collapsible`, `defaultCollapsed`,
* `columns`, `showBorder`, `headerColor`. `DetailSection` declares no such
* props: it takes a single `section` object and reads `section.title`,
Expand All @@ -35,11 +35,11 @@
* The two repairs objectui#8626 names are not symmetric, and the measurement
* decides between them rather than taste:
*
* - The flat eight are the PUBLISHED surface, and the platform enforces them.
* - The flat inputs are the PUBLISHED surface, and the platform enforces them.
* `packages/components/src/renderers/layout/page.tsx` builds the JSX-page
* compiler's manifest from `getKnownTypes()` plus these `inputs`, and
* `sdui-parser`'s `validateTree` judges an authored page against it.
* Measured against that live manifest: the flat eight draw ZERO
* Measured against that live manifest: the flat eight drew ZERO
* diagnostics, while `section` draws `unknown-prop` AND
* `missing-required-prop "fields"`. The validator does not merely permit
* the flat shape — it REFUSES the nested one.
Expand Down Expand Up @@ -90,6 +90,10 @@ import { DetailSection, type DetailSectionProps } from './DetailSection';
export const DETAIL_SECTION_NODE_INPUTS = [
'title',
'description',
// objectui#9529: `DetailSection` draws `section.icon` in both header
// branches, so it is declared and folded like the rest. `name` and `visible`
// stay out — `DetailSection` reads neither.
'icon',
'fields',
'collapsible',
'defaultCollapsed',
Expand All @@ -101,8 +105,9 @@ export const DETAIL_SECTION_NODE_INPUTS = [
type DetailSectionNodeInput = (typeof DETAIL_SECTION_NODE_INPUTS)[number];

/**
* What an author may write on a `detail-section` node: the eight declared
* inputs, flat, plus the render-context props a host supplies.
* What an author may write on a `detail-section` node: the declared inputs
* (`DETAIL_SECTION_NODE_INPUTS`), flat, plus the render-context props a host
* supplies.
*/
export type DetailSectionNodeProps = Omit<DetailSectionProps, 'section'> &
Partial<Pick<DetailViewSection, DetailSectionNodeInput>>;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#9529 — an authored `icon` on a `detail-section` node VALIDATES and
* RENDERS.
*
* ## The gap
*
* `DetailSection` renders `section.icon` in both of its header branches, but
* the `detail-section` registration did not declare `icon` among its `inputs`,
* and `DetailSectionNode` folds only the declared names into the `section` it
* hands the component. So an author who wrote `icon` on the node met both
* halves of one gap:
*
* - the JSX-page compiler's validator (a manifest built from
* `getKnownTypes()` plus these `inputs`, the way
* `packages/components/src/renderers/layout/page.tsx` builds it) warned
* `unknown-prop "icon"` — steering the author away from a key the renderer
* honours; and
* - the fold left `icon` out of `section`, so the header rendered no icon.
*
* The ruling on the card (director seat, maintainer-agreed) declares the key:
* `{ name: 'icon', type: 'string' }` on the registration and in
* `DETAIL_SECTION_NODE_INPUTS`.
*
* ## What each row reads, and why it is a verdict
*
* - VALIDATES — the node with an authored `icon` draws ZERO diagnostics from
* the live manifest. The declared TYPE is read through the same judge: a
* non-string `icon` draws `type-mismatch` naming the key, which only a
* declared `string` input can produce (an undeclared key draws
* `unknown-prop` instead).
* - THE CONTROL IS LIT — `name`, a `DetailViewSection` member this block
* neither declares nor reads, still draws `unknown-prop` from the same
* manifest in the same run. A silenced validator would pass the rows above
* vacuously; it cannot pass this one. `visible` cannot serve here: it is one
* of the validator's base node keys (`BASE_PROPS` in `sdui-parser`'s
* `validate.ts`), which every node may carry and which are never judged
* against `inputs`, so it draws nothing before or after this change.
* - RENDERS — measured in the DOM through the real `SchemaRenderer` and the
* real registry, in BOTH header branches `DetailSection` reads the key in:
* the authored Lucide name is drawn as that icon's glyph beside the
* authored title, and the same node without `icon` draws nothing there.
*/

import React from 'react';
import { describe, it, expect, afterEach } from 'vitest';
import { render, screen, cleanup, waitFor } from '@testing-library/react';
import { ComponentRegistry } from '@object-ui/core';
import { SchemaRenderer } from '@object-ui/react';
import { manifestFromConfigs, validateTree } from '@object-ui/sdui-parser';
// The reference glyph the render rows compare against, imported at module
// scope rather than in a hook (see the test-discipline section of AGENTS.md):
// `LazyIcon` draws `map-pin` through lucide's `DynamicIcon`, which fetches the
// icon module with a lazy `import()`, and lucide's ESM entry re-exports
// `MapPin` from that same icon module.
import { MapPin } from 'lucide-react';

// Module scope, not a hook (same section of AGENTS.md): importing the package index
// executes its registration side-effects, so the entry under test is the very
// one production resolves.
import '../index';

const TAG = 'detail-section';
const TITLE = 'Billing Address';

/** A minimal authored node: the one required input plus a title to anchor on. */
const authoredNode = (overrides: Record<string, unknown> = {}) =>
({
type: TAG,
title: TITLE,
fields: [{ name: 'street', label: 'Street' }],
...overrides,
}) as never;

const RECORD = { street: '1 Market St' };

/** Built the way `page.tsx` builds the JSX-page compiler's manifest. */
const liveManifest = () =>
manifestFromConfigs(
ComponentRegistry.getKnownTypes().map((type) => {
const meta = ComponentRegistry.getMeta(type);
return {
type,
namespace: meta?.namespace,
isContainer: meta?.isContainer,
inputs: meta?.inputs,
};
}) as unknown as Parameters<typeof manifestFromConfigs>[0],
);

const diagnose = (overrides: Record<string, unknown>) =>
validateTree(authoredNode(overrides), liveManifest()).diagnostics.map((d) => ({
code: d.code,
message: d.message,
}));

/** The row the header's title sits in — where `DetailSection` puts the icon. */
const titleRow = () => {
const row = screen.getByText(TITLE).parentElement;
expect(row).not.toBeNull();
return row as HTMLElement;
};

afterEach(() => {
cleanup();
});

describe('objectui#9529 — an authored detail-section icon validates', () => {
it('draws no diagnostic for an authored string icon', () => {
expect(diagnose({ icon: 'map-pin' })).toEqual([]);
});

it('judges the authored icon as a declared STRING input', () => {
const found = diagnose({ icon: 42 });
expect(found.map((d) => d.code)).toEqual(['type-mismatch']);
expect(found[0].message).toContain('"icon"');
});

it('CONTROL: still warns on name, which the block neither declares nor reads', () => {
const found = diagnose({ name: 'billing' });
expect(found.map((d) => d.code)).toEqual(['unknown-prop']);
expect(found[0].message).toContain('"name"');
});
});

describe('objectui#9529 — an authored detail-section icon renders', () => {
// Both header branches read `section.icon`, so both are measured. The
// verdict is the GLYPH: the drawn svg's markup must equal `MapPin`'s, which
// the `Database` glyph `LazyIcon` draws while loading (and for an unknown
// name) can never satisfy.
for (const collapsible of [false, true]) {
it(`draws the authored Lucide icon before the title (collapsible: ${collapsible})`, async () => {
const expected = render(<MapPin />).container.querySelector('svg')?.innerHTML ?? '';
expect(expected.length).toBeGreaterThan(0);
cleanup();

render(
<SchemaRenderer schema={authoredNode({ collapsible, icon: 'map-pin' })} data={RECORD} />,
);
await waitFor(() => {
expect(titleRow().querySelector('svg')?.innerHTML).toBe(expected);
});

// Control: the same node without `icon` draws nothing beside its title.
cleanup();
render(<SchemaRenderer schema={authoredNode({ collapsible })} data={RECORD} />);
expect(titleRow().querySelector('svg')).toBeNull();
expect(titleRow().textContent).toBe(TITLE);
});
}
});
9 changes: 7 additions & 2 deletions packages/plugin-detail/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -330,10 +330,10 @@ ComponentRegistry.register('detail-view', DetailViewRenderer, {
// directly, `section` arrived `undefined` and the very first
// `section.defaultCollapsed` read THREW — measured end to end, the author's page
// showed `SchemaErrorBoundary`'s orange "failed to render" banner in place of the
// block. `DetailSectionNode` folds the eight declared inputs into the `section`
// block. `DetailSectionNode` folds the declared inputs into the `section`
// object the component reads; see that file for why the fold sits at this seam
// rather than in `DetailSection` (which every in-repo caller uses directly), and
// why re-declaring these eight as a nested `section` input was the repair NOT
// why re-declaring these as a nested `section` input was the repair NOT
// taken.
ComponentRegistry.register('detail-section', DetailSectionNode, {
namespace: 'plugin-detail',
Expand All @@ -342,6 +342,11 @@ ComponentRegistry.register('detail-section', DetailSectionNode, {
inputs: [
{ name: 'title', type: 'string' },
{ name: 'description', type: 'string' },
{
name: 'icon',
type: 'string',
description: 'Section header icon, drawn before the title: a Lucide icon name such as `map-pin`.',
},
{ name: 'fields', type: 'array', required: true },
{ name: 'collapsible', type: 'boolean' },
{ name: 'defaultCollapsed', type: 'boolean' },
Expand Down
Loading