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
9 changes: 9 additions & 0 deletions .changeset/rotten-pugs-invent.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
---
'@object-ui/app-shell': patch
---

Page-block canvas: draw the real icon and colour tone for block types that render fine but are not offered in the palette.

The canvas read its per-node icon and tone from `BLOCK_TYPE_META`, which is the palette's *offer* list ("what may an author drag in") — while the canvas is answering a different question ("what may an author already have in this page"). The two diverge for an alias pair: `record:discussion` and `record:chatter` are one renderer under two names, so exactly one spelling is always unoffered, and a page carrying it showed a plain unknown-block box and neutral grey instead of the message icon and the blue record tone its twin gets.

`block-types.ts` now declares `BLOCK_RENDERER_ALIAS_GROUPS` — spellings that resolve to the same registered renderer — and the canvas resolves display chrome through `resolveBlockDisplayMeta()`, which falls back to an alias sibling's entry. Keyed on renderer identity, not on "is excluded": palette exclusions that genuinely are not page blocks (`element:text_input`, `element:record_picker`, `element:form`, `ai:chat_window`) keep the generic box, and nothing became draggable — the palette offer list is unchanged.
Original file line numberDiff line numberDiff line change
Expand Up@@ -31,6 +31,7 @@ import {
TYPES_BY_CATEGORY,
CATEGORY_LABEL_EN,
UnknownBlockIcon,
resolveBlockDisplayMeta,
resolveBlockTone,
type BlockTypeId,
} from './block-types.js';
Expand DownExpand Up@@ -619,7 +620,12 @@ function BlockRow({
onRenameLabel: (nextLabel: string) => void;
}) {
const typeStr = String(block.type ?? '');
const meta = BLOCK_TYPE_META[typeStr as BlockTypeId];
// DISPLAY meta, not the palette catalogue. `BLOCK_TYPE_META` answers "what may
// an author drag in"; a node already in the page may be a spelling the palette
// does not offer yet the app renders fine — the `record:discussion` /
// `record:chatter` alias pair. See BLOCK_RENDERER_ALIAS_GROUPS in
// block-types.ts; nothing here makes a type offerable.
const meta = resolveBlockDisplayMeta(typeStr);
const Icon = meta?.Icon ?? UnknownBlockIcon;
const tone = resolveBlockTone(typeStr);
const label = blockLabel(block);
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,259 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.

/**
* objectui#5837 — the page canvas draws chrome for what a page CAN CONTAIN,
* not for what the palette OFFERS.
*
* `BLOCK_TYPE_META` is the palette's offer list ("what may an author drag in").
* `PageBlockCanvas` was reading it to pick a node's icon and colour tone, which
* is a different question ("what may an author already have in this page").
* The two coincide for every palette exclusion whose reason is that the block
* is not page content at all — and diverge for an alias pair, where one renderer
* has two spellings and the palette deliberately advertises only one of them.
* The unadvertised spelling renders perfectly and got the unknown-block box.
*
* ## What this file pins, and how it avoids two easy ways of being wrong
*
* 1. RENDERABILITY, NOT EXCLUSION. `BLOCK_RENDERER_ALIAS_GROUPS` is only
* allowed to list spellings that the runtime registry resolves to the very
* same component. That is asserted here against the real `ComponentRegistry`
* (identity, not "both defined"), so the declaration cannot quietly grant
* friendly chrome to a type that has no renderer behind it. The counter-probe
* is `element:text_input`: excluded for an unrelated reason, no twin, and it
* must keep the generic box.
*
* 2. NO ORIENTATION IS HARD-CODED. Which member of the pair the palette
* advertises is a maintainer decision that already flipped once (#5495 moved
* it from the legacy alias to the canonical name). Every assertion below
* DERIVES the offered/unoffered split from `BLOCK_TYPE_META` instead of
* naming a side, and one test flips the catalogue outright to prove the
* resolver follows.
*
* The offer list itself must not move: "the icon appears" is also satisfiable by
* quietly making the type draggable, which is exactly the invariant #2943 built
* `PALETTE_EXCLUSIONS` to protect. That is the third block of tests.
*/

import * as React from 'react';
import { describe, it, expect, afterEach } from 'vitest';
import { render, screen, cleanup } from '@testing-library/react';
import { ComponentRegistry } from '@object-ui/core';
// Side-effect import: registers `record:chatter` and `record:discussion`. The
// app-shell test setup does not pull plugin-detail in, and relying on another
// file having imported it first would make this suite order-dependent.
import '@object-ui/plugin-detail';
import {
BLOCK_TYPE_META,
BLOCK_CATEGORY_TONE,
BLOCK_RENDERER_ALIAS_GROUPS,
PALETTE_EXCLUSIONS,
TYPES_BY_CATEGORY,
UnknownBlockIcon,
resolveBlockDisplayMeta,
resolveBlockTone,
} from '../block-types';
import { PageBlockCanvas } from '../PageBlockCanvas';

afterEach(cleanup);

const offered = BLOCK_TYPE_META as Record<string, { category: string; Icon: unknown } | undefined>;
const mutableMeta = BLOCK_TYPE_META as unknown as Record<string, unknown>;

/** The declared groups, split into what the palette advertises and what it does not. */
const groupSplits = BLOCK_RENDERER_ALIAS_GROUPS.map((group) => ({
group,
advertised: group.filter((t) => offered[t] !== undefined),
unadvertised: group.filter((t) => offered[t] === undefined),
}));

describe('BLOCK_RENDERER_ALIAS_GROUPS — the declaration is keyed on renderer identity (#5837)', () => {
const config = (type: string) =>
ComponentRegistry.getConfig(type) as { component?: React.ComponentType<unknown> } | undefined;

it('declares at least one group, each with at least two spellings', () => {
// Non-vacuity: every "for each group" assertion below passes over an empty
// list, and a one-member group has nothing to borrow from.
expect(BLOCK_RENDERER_ALIAS_GROUPS.length).toBeGreaterThan(0);
for (const group of BLOCK_RENDERER_ALIAS_GROUPS) {
expect(group.length, `${JSON.stringify(group)} is not a pair`).toBeGreaterThan(1);
}
});

it('every spelling in a group resolves to the SAME registered renderer', () => {
// This is how "renderable" is determined — not from the exclusion ledger,
// which records unrelated reasons too, but from the registry itself. Two
// independently registered renderers that happened to diverge would pass a
// defined/defined check; identity is what makes borrowing chrome honest.
for (const group of BLOCK_RENDERER_ALIAS_GROUPS) {
const configs = group.map((t) => ({ type: t, cfg: config(t) }));
for (const { type, cfg } of configs) {
expect(cfg, `\`${type}\` is declared an alias but nothing registers it`).toBeDefined();
expect(cfg!.component, `\`${type}\` registers no component`).toBeDefined();
}
const [first, ...rest] = configs;
for (const other of rest) {
expect(
other.cfg!.component,
`\`${other.type}\` and \`${first.type}\` are declared one renderer but resolve to two`,
).toBe(first.cfg!.component);
}
}
// Non-vacuity for the lookup: a `getConfig` that returned a truthy object
// for anything at all would make every assertion above meaningless.
expect(config('record:zzNotARegisteredType')).toBeUndefined();
});

it('each group has exactly one advertised spelling, and the rest carry an exclusion reason', () => {
for (const { group, advertised, unadvertised } of groupSplits) {
expect(
advertised.length,
`${JSON.stringify(group)} must have exactly one palette entry — one entry per renderer`,
).toBe(1);
for (const type of unadvertised) {
expect(
PALETTE_EXCLUSIONS[type],
`\`${type}\` is unoffered but absent from the exclusion ledger`,
).toBeTruthy();
}
}
});
});

describe('canvas chrome for a renderable-but-unoffered spelling (#5837)', () => {
it('the unadvertised spelling borrows the icon and tone of its twin', () => {
for (const { advertised, unadvertised } of groupSplits) {
const twin = offered[advertised[0]]!;
for (const type of unadvertised) {
const display = resolveBlockDisplayMeta(type);
expect(display, `\`${type}\` still resolves to no display meta`).toBeDefined();
expect(display!.Icon, `\`${type}\` draws a different icon than \`${advertised[0]}\``)
.toBe(twin.Icon);
expect(display!.Icon).not.toBe(UnknownBlockIcon);
expect(display!.category).toBe(twin.category);
expect(resolveBlockTone(type)).toBe(resolveBlockTone(advertised[0]));
expect(resolveBlockTone(type)).not.toBe(BLOCK_CATEGORY_TONE.misc);
}
}
});

it('an offered type still resolves to its own entry, unchanged', () => {
for (const [type, meta] of Object.entries(BLOCK_TYPE_META)) {
const display = resolveBlockDisplayMeta(type);
expect(display!.Icon, type).toBe(meta.Icon);
expect(display!.category, type).toBe(meta.category);
}
});

it('follows the pair when the palette flips which spelling it advertises', () => {
// #5495 moved the advertised spelling from the legacy alias to the canonical
// name; nothing stops a future ruling moving it back. A fix that named a
// direction would re-open this gap on that day, so the resolver is proved
// symmetric here rather than asserted in prose.
const { advertised, unadvertised } = groupSplits[0];
const wasOffered = advertised[0];
const wasUnoffered = unadvertised[0];
const saved = mutableMeta[wasOffered];
try {
delete mutableMeta[wasOffered];
mutableMeta[wasUnoffered] = saved;
const flipped = resolveBlockDisplayMeta(wasOffered);
expect(flipped, 'the formerly-offered spelling lost its chrome after the flip').toBeDefined();
expect(flipped!.Icon).toBe((saved as { Icon: unknown }).Icon);
expect(resolveBlockTone(wasOffered)).not.toBe(BLOCK_CATEGORY_TONE.misc);
} finally {
delete mutableMeta[wasUnoffered];
mutableMeta[wasOffered] = saved;
}
// The catalogue is back exactly as it was — a leaked mutation would make
// every later assertion in this process report on a palette nobody ships.
expect(offered[wasOffered]).toBe(saved);
expect(offered[wasUnoffered]).toBeUndefined();
});

it('an exclusion with no renderer twin keeps the generic box', () => {
// Counter-probe. `element:text_input` is unoffered for its own reason and is
// in no alias group; a fix keyed on "is excluded" would hand it a friendly
// icon it has not earned.
for (const type of ['element:text_input', 'element:record_picker', 'element:form', 'ai:chat_window']) {
expect(PALETTE_EXCLUSIONS[type], `${type} is not an exclusion any more`).toBeTruthy();
expect(resolveBlockDisplayMeta(type), `${type} borrowed display meta`).toBeUndefined();
expect(resolveBlockTone(type), `${type} borrowed a tone`).toBe(BLOCK_CATEGORY_TONE.misc);
}
// And an outright unknown type, which is what the fallback exists for.
expect(resolveBlockDisplayMeta('zz:not-a-block')).toBeUndefined();
expect(resolveBlockTone('zz:not-a-block')).toBe(BLOCK_CATEGORY_TONE.misc);
});
});

describe('the palette offer list is untouched (#2943 ledger invariant)', () => {
it('no alias-group member became offerable', () => {
for (const { unadvertised } of groupSplits) {
for (const type of unadvertised) {
expect(type in (BLOCK_TYPE_META as Record<string, unknown>)).toBe(false);
}
}
});

it('the drag-in list is exactly the palette catalogue, nothing more', () => {
// `TYPES_BY_CATEGORY` is what the Add-block picker iterates. Deriving the
// expectation from `BLOCK_TYPE_META` keeps this true across palette edits
// while still failing the moment a display-only type leaks into the picker.
const draggable = TYPES_BY_CATEGORY.flatMap((g) => g.types).sort();
expect(draggable).toEqual(Object.keys(BLOCK_TYPE_META).sort());
for (const excluded of Object.keys(PALETTE_EXCLUSIONS)) {
expect(draggable, `\`${excluded}\` became draggable`).not.toContain(excluded);
}
});
});

describe('PageBlockCanvas — the chrome reaches the DOM (#5837)', () => {
const pair = groupSplits[0];
const advertised = pair.advertised[0];
const unadvertised = pair.unadvertised[0];

const draftWith = (types: string[]) => ({
name: 'p',
type: 'record',
regions: [{ name: 'main', components: types.map((t) => ({ type: t })) }],
});

it('the unadvertised spelling gets the tone of its twin on the type badge', () => {
render(<PageBlockCanvas draft={draftWith([advertised, unadvertised, 'element:text_input'])} />);
const badgeClass = (type: string) => screen.getByText(type).getAttribute('class') ?? '';

const twinClass = badgeClass(advertised);
expect(badgeClass(unadvertised)).toBe(twinClass);
// Positive half: the shared class really is the record tone, not two nodes
// that both fell through to grey.
expect(twinClass).toContain(BLOCK_CATEGORY_TONE[offered[advertised]!.category as 'record'].badge);
// Counter-probe on the same render: the un-twinned exclusion stays grey.
expect(badgeClass('element:text_input')).toContain(BLOCK_CATEGORY_TONE.misc.badge);
expect(badgeClass('element:text_input')).not.toBe(twinClass);
});

it('the unadvertised spelling gets the icon of its twin', () => {
// The canvas draws a node's ICON in its container branch (a leaf node shows
// a live preview instead), so the probe uses container-shaped nodes — an
// empty `properties.children` is all `childGroups` needs.
const container = (type: string) => ({ type, properties: { children: [] } });
render(
<PageBlockCanvas
draft={{
name: 'p',
type: 'record',
regions: [{ name: 'main', components: [container(advertised), container(unadvertised), container('element:text_input')] }],
}}
/>,
);
const iconClass = (type: string) => {
// A container row prints the type twice — once as the fallback label
// (`blockLabel` falls back to the raw type) and once in the badge — and
// both live inside the same row button, so either match finds the row.
const row = screen.getAllByText(type)[0].closest('button');
return row?.querySelector('svg.lucide')?.getAttribute('class') ?? '';
};
expect(iconClass(advertised)).not.toBe('');
expect(iconClass(unadvertised)).toBe(iconClass(advertised));
expect(iconClass('element:text_input')).not.toBe(iconClass(advertised));
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -182,6 +182,41 @@ export const PALETTE_EXCLUSIONS: Record<string, string> = {
'record:chatter': 'compatibility alias — same renderer as the offered canonical `record:discussion`; still renders, just no longer advertised',
};

/**
* Block types that are ONE renderer under several spellings — the DISPLAY
* counterpart to {@link PALETTE_EXCLUSIONS}.
*
* {@link BLOCK_TYPE_META} answers "what may an author drag IN". The page canvas
* asks a different question — "what may an author already HAVE in this page" —
* and for most exclusions the two answers coincide: `ai:chat_window`,
* `element:form`, `element:record_picker` and `element:text_input` are not page
* blocks an author composes with, so a node bearing one of those names is
* something the canvas cannot draw meaningfully, and {@link UnknownBlockIcon}
* plus the neutral `misc` tone is the honest chrome for it.
*
* An alias pair is where the two questions diverge. `record:discussion` and
* `record:chatter` are registered against the SAME renderer function
* (`plugin-detail/src/index.tsx`), and the palette deliberately offers exactly
* one of them so there is one entry per renderer — which leaves the other
* spelling rendering perfectly while the canvas drew it as an unknown grey box.
*
* The key here is RENDERER IDENTITY, not "is excluded". A group may only list
* spellings that resolve, through `ComponentRegistry`, to the very same
* component; `canvas-display-meta.test.tsx` asserts exactly that against the
* real registry, so a group cannot claim a renderability it does not have. That
* is what keeps `element:text_input` — excluded for an unrelated reason, and
* with no twin — from borrowing an icon it has not earned.
*
* Groups are UNORDERED and orientation-agnostic on purpose. Which spelling the
* palette advertises is a maintainer decision that has already flipped once
* (objectui#5495 moved it from the alias to the canonical name), so the
* resolver borrows from whichever member is offered TODAY rather than
* hard-coding a direction; a future flip cannot re-open this gap.
*/
export const BLOCK_RENDERER_ALIAS_GROUPS: readonly (readonly string[])[] = [
['record:discussion', 'record:chatter'],
];

export const CATEGORY_LABEL_EN: Record<BlockCategory, string> = {
data: 'Data',
layout: 'Layout',
Expand DownExpand Up@@ -250,8 +285,41 @@ export const BLOCK_CATEGORY_TONE: Record<BlockCategory, BlockCategoryTone> = {
},
};

/**
* Icon + tone inputs for any block type the canvas can ENCOUNTER, as opposed to
* {@link BLOCK_TYPE_META}, which lists what the palette OFFERS.
*
* Deliberately narrower than {@link BlockTypeMeta}: no `label`. `blockLabel()`
* in `PageBlockCanvas` never consulted this catalogue — it falls back to the
* raw type string — and widening display resolution into labels would change
* author-visible naming, a separate decision from chrome.
*/
export interface BlockDisplayMeta {
Icon: LucideIcon;
category: BlockCategory;
}

/**
* Resolve the canvas chrome for a block `type`: its palette entry when it has
* one, otherwise an alias sibling's ({@link BLOCK_RENDERER_ALIAS_GROUPS}).
* `undefined` for everything else, so the caller draws
* {@link UnknownBlockIcon}. Nothing here makes a type offerable — the palette
* is built from {@link BLOCK_TYPE_META} alone ({@link TYPES_BY_CATEGORY}).
*/
export function resolveBlockDisplayMeta(type: string): BlockDisplayMeta | undefined {
const direct = BLOCK_TYPE_META[type as BlockTypeId];
if (direct) return { Icon: direct.Icon, category: direct.category };
for (const group of BLOCK_RENDERER_ALIAS_GROUPS) {
if (!group.includes(type)) continue;
for (const sibling of group) {
const meta = BLOCK_TYPE_META[sibling as BlockTypeId];
if (meta) return { Icon: meta.Icon, category: meta.category };
}
}
return undefined;
}

/** Resolve a category tone for any block `type` string (handles unknowns). */
export function resolveBlockTone(type: string): BlockCategoryTone {
const meta = BLOCK_TYPE_META[type as BlockTypeId];
return BLOCK_CATEGORY_TONE[meta?.category ?? 'misc'];
return BLOCK_CATEGORY_TONE[resolveBlockDisplayMeta(type)?.category ?? 'misc'];
}
Loading