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
41 changes: 41 additions & 0 deletions .changeset/6178-detail-section-header-color.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
---
'@object-ui/plugin-detail': patch
---

`DetailSection` now resolves `section.headerColor` through a lookup of complete
Tailwind class literals instead of building `bg-` + the authored value as a
template literal (objectui#6178).

Tailwind v4 has no runtime — it builds the stylesheet by scanning source text
for complete class tokens, and this workspace ships no `bg-*` safelist — so the
old expression contributed nothing to the compiled CSS. Measured, not assumed:
compiling `apps/console/src/index.css` with that expression deleted produced a
byte-identical stylesheet (same sha256). An authored value styled the header
only when some other source file happened to author the identical class
literally, which is why both documented examples appeared to work: `bg-muted`
occurs 691 times and `bg-primary/10` 63 times elsewhere in the workspace. That
liveness was accidental and moved with unrelated edits in unrelated packages.

The shape matches the sibling this repo already solved the same way —
`useRowColor`'s `COLOR_TO_CLASS` in `@object-ui/plugin-grid`:

- a lookup of literal, tint-only design-system classes: `muted`, `muted/50`,
`accent`, `primary/10`, `secondary/10`, `destructive/10`. Both values the
`@object-ui/types` mirror documents (`muted`, `primary/10`) are in it, so
nothing that rendered before renders differently now;
- a value that is already a complete `bg-*` class is passed through untouched.
This is new — `headerColor: 'bg-muted'` previously produced the meaningless
`bg-bg-muted`;
- anything else contributes no class at all, instead of a fabricated one.

Behaviour change to be aware of: an undocumented bare suffix outside the
vocabulary (say `headerColor: 'blue-100'`) no longer reaches the DOM as
`bg-blue-100`. It rendered before only where another file happened to author
that exact class; write it as the complete class (`headerColor: 'bg-blue-100'`)
to keep it, on the same terms as any `className` a schema carries. No value is
rejected and the declared type is unchanged.

`headerColor` remains undeclared on the strict `@objectstack/spec`
`record:details` section schema, which refuses it today on the strength of this
defect (objectstack#11661). Declaring it, and with which vocabulary, is a
separate spec decision.
5 changes: 3 additions & 2 deletions packages/plugin-detail/src/DetailSection.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -34,6 +34,7 @@ import { useSafeFieldLabel } from '@object-ui/react';
import { PermissionFacetLink } from './renderers/PermissionFacetLink';
import { NON_EDITABLE_SYSTEM_FIELDS } from './systemFields';
import { InlineFieldInput } from './InlineFieldInput';
import { headerColorClass } from './headerColor';
import {
enrichDetailField,
isComputedFieldType,
Expand DownExpand Up@@ -510,7 +511,7 @@ export const DetailSection: React.FC<DetailSectionProps> = ({
return (
<Card className={cn(section.showBorder === false ? 'border-none shadow-none' : '', className)}>
{section.title && (
<CardHeader className={cn('py-3 px-4 sm:py-4 sm:px-6', section.headerColor && `bg-${section.headerColor}`)}>
<CardHeader className={cn('py-3 px-4 sm:py-4 sm:px-6', headerColorClass(section.headerColor))}>
<CardTitle className="flex items-center justify-between text-base font-semibold tracking-tight">
<div className="flex items-center gap-2">
{section.icon && <SectionIcon name={section.icon} />}
Expand DownExpand Up@@ -539,7 +540,7 @@ export const DetailSection: React.FC<DetailSectionProps> = ({
<CollapsibleTrigger asChild>
<CardHeader className={cn(
"py-3 px-4 sm:py-4 sm:px-6 cursor-pointer hover:bg-muted/50 transition-colors",
section.headerColor && `bg-${section.headerColor}`
headerColorClass(section.headerColor)
)}>
<CardTitle className="flex items-center justify-between text-base font-semibold tracking-tight">
<div className="flex items-center gap-2">
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
/**
* 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.
*/

import { describe, it, expect } from 'vitest';
import { render } from '@testing-library/react';
import { DetailSection } from '../DetailSection';
import type { DetailViewSection } from '@object-ui/types';

/**
* objectui#6178 — `headerColor` reached the DOM as a template-literal Tailwind
* class, which the v4 source scan never sees as a complete token, so the class
* had no rule behind it unless another file happened to author the same class
* literally.
*
* WHAT THIS FILE CAN AND CANNOT SHOW. It renders the real `DetailSection` and
* inspects the class list the header receives. That proves which class string
* reaches the DOM; it proves NOTHING about whether Tailwind emitted a rule for
* it — asserting a `className` is exactly the blind instrument this defect
* hides behind. The CSS-generation half is `headerColor.test.ts`, which asks
* the Tailwind design system for the rule and reads this module's source text
* the way the scanner does. Neither file alone is the evidence.
*/

const TITLE = 'Billing';

const baseSection = (extra: Partial<DetailViewSection>): DetailViewSection =>
({ title: TITLE, fields: [{ name: 'amount', label: 'Amount' }], ...extra }) as DetailViewSection;

/**
* The header element, located by the two padding classes `DetailSection`
* passes to `CardHeader` on both render branches. `getBy`-style: it throws
* when the header did not render at all, so a negative assertion below cannot
* pass vacuously against a header that is not on screen.
*/
function headerOf(container: HTMLElement): HTMLElement {
const matches = Array.from(container.querySelectorAll<HTMLElement>('.py-3.px-4'));
expect(matches, 'exactly one section header should render').toHaveLength(1);
return matches[0];
}

const classesOf = (el: HTMLElement) => el.className.split(/\s+/).filter(Boolean);

describe('DetailSection headerColor -> a class the stylesheet can carry (objectui#6178)', () => {
// ---- the instrument, before anything is asserted with it ----------------
it('control: the header renders, carries its base classes, and shows the title', () => {
const { container, getByText } = render(
<DetailSection section={baseSection({})} data={{ amount: 1 }} />,
);
const header = headerOf(container);
expect(getByText(TITLE)).toBeTruthy();
expect(classesOf(header)).toEqual(expect.arrayContaining(['py-3', 'px-4', 'sm:px-6']));
// No headerColor authored -> no background utility at all.
expect(classesOf(header).filter((c) => c.startsWith('bg-'))).toEqual([]);
});

describe.each([
['non-collapsible (titled Card)', {}],
['collapsible (CollapsibleTrigger header)', { collapsible: true }],
])('%s', (_label, extra) => {
it('a mapped token renders its literal class', () => {
const { container, getByText } = render(
<DetailSection section={baseSection({ ...extra, headerColor: 'muted' })} data={{ amount: 1 }} />,
);
const header = headerOf(container);
expect(getByText(TITLE)).toBeTruthy(); // positive probe: the header is real
expect(classesOf(header)).toContain('bg-muted');
});

it('the second documented example (`primary/10`) renders its literal class', () => {
const { container } = render(
<DetailSection
section={baseSection({ ...extra, headerColor: 'primary/10' })}
data={{ amount: 1 }}
/>,
);
expect(classesOf(headerOf(container))).toContain('bg-primary/10');
});

it('a value that is already a `bg-*` class passes through, not doubled', () => {
const { container } = render(
<DetailSection
section={baseSection({ ...extra, headerColor: 'bg-accent' })}
data={{ amount: 1 }}
/>,
);
const classes = classesOf(headerOf(container));
expect(classes).toContain('bg-accent');
// The old concatenation produced `bg-bg-accent` for this input.
expect(classes).not.toContain('bg-bg-accent');
});

it('an unmapped value contributes no class at all — never a fabricated one', () => {
const { container, getByText } = render(
<DetailSection
section={baseSection({ ...extra, headerColor: 'not-a-token' })}
data={{ amount: 1 }}
/>,
);
const header = headerOf(container);
// Positive probe first: the header IS rendered, so the two negative
// assertions below are about a real element.
expect(getByText(TITLE)).toBeTruthy();
const classes = classesOf(header);
expect(classes).toContain('py-3');
expect(classes).not.toContain('bg-not-a-token');
expect(classes.filter((c) => c.startsWith('bg-'))).toEqual([]);
});

it('an inherited Object.prototype key is not a vocabulary entry', () => {
const { container } = render(
<DetailSection
section={baseSection({ ...extra, headerColor: 'constructor' })}
data={{ amount: 1 }}
/>,
);
const classes = classesOf(headerOf(container));
expect(classes.filter((c) => c.startsWith('bg-'))).toEqual([]);
expect(classes.some((c) => c.includes('Object'))).toBe(false);
});
});
});
164 changes: 164 additions & 0 deletions packages/plugin-detail/src/__tests__/headerColor.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
/**
* 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.
*/

import { describe, it, expect } from 'vitest';
import fs from 'node:fs';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { __unstable__loadDesignSystem } from 'tailwindcss';
import { headerColorClass, headerColorVocabulary } from '../headerColor';

/**
* objectui#6178 — the CSS-GENERATION half of the fix.
*
* `DetailSection.headerColor.test.tsx` proves which class string reaches the
* DOM. That is not the property this defect is about: the previous code put
* `bg-<value>` in the DOM too, and a rendering assertion was green the whole
* time it generated no CSS. Tailwind v4 emits a rule only when BOTH hold —
*
* 1. the class appears as a COMPLETE token in text the `@source` scan reads,
* 2. the class is a utility the design system can actually build.
*
* so both are asserted here, against the two artifacts that decide them: the
* module's own source text, and Tailwind's design system loaded on this
* workspace's real `@theme`.
*
* What this file still cannot show: it does not run the scanner (that lives in
* `@tailwindcss/oxide`, which this workspace does not declare at the root) and
* it does not verify any app's `@source` globs. It asserts the token property
* the scanner requires, on the file the globs cover.
*/

const here = path.dirname(fileURLToPath(import.meta.url));
const repoRoot = path.resolve(here, '../../../..');
const moduleSource = fs.readFileSync(path.join(here, '..', 'headerColor.ts'), 'utf8');
const callSiteSource = fs.readFileSync(path.join(here, '..', 'DetailSection.tsx'), 'utf8');

/** Every class the vocabulary can put in the DOM. */
const vocabularyClasses = Object.values(headerColorVocabulary);

/**
* The workspace theme, as shipped. `@object-ui/components`' `index.css` is the
* one `@theme` block every consuming app loads (see `skills/objectui/rules/
* styling.md`), so a token this vocabulary spends has to be defined there —
* `bg-accent` is not a stock Tailwind utility, it exists only because that
* block defines `--color-accent`.
*/
function themeBlock(): string {
const css = fs.readFileSync(path.join(repoRoot, 'packages/components/src/index.css'), 'utf8');
const start = css.indexOf('@theme {');
expect(start, 'components/src/index.css should declare a @theme block').toBeGreaterThan(-1);
let depth = 0;
for (let i = css.indexOf('{', start); i < css.length; i++) {
if (css[i] === '{') depth++;
else if (css[i] === '}' && --depth === 0) return css.slice(start, i + 1);
}
throw new Error('unterminated @theme block');
}

async function designSystem() {
const twEntry = path.join(repoRoot, 'node_modules/tailwindcss/index.css');
const twDir = path.dirname(fs.realpathSync(twEntry));
return __unstable__loadDesignSystem(`@import "tailwindcss";\n${themeBlock()}\n`, {
base: repoRoot,
loadStylesheet: async (id: string, base: string) => {
const file = id === 'tailwindcss'
? path.join(twDir, 'index.css')
: id.startsWith('tailwindcss/')
? path.join(twDir, id.slice('tailwindcss/'.length))
: path.resolve(base, id);
return { base: path.dirname(file), path: file, content: fs.readFileSync(file, 'utf8') };
},
});
}

describe('headerColor — the resolver', () => {
it('maps the two values the @object-ui/types mirror documents', () => {
// These are the examples on `DetailViewSection.headerColor`. Both worked
// before this module — by collision with other files' literal classes —
// so the fix has to keep them working, not merely stop lying.
expect(headerColorClass('muted')).toBe('bg-muted');
expect(headerColorClass('primary/10')).toBe('bg-primary/10');
});

it('passes a value that is already a `bg-*` class through untouched', () => {
expect(headerColorClass('bg-accent')).toBe('bg-accent');
expect(headerColorClass('bg-[color:var(--brand)]')).toBe('bg-[color:var(--brand)]');
});

it('returns undefined rather than fabricating a class', () => {
for (const input of [undefined, '', ' ', 'not-a-token', 'blue-100', 'muted-'])
expect(headerColorClass(input)).toBeUndefined();
});

it('does not hand back an inherited Object.prototype member', () => {
for (const input of ['constructor', 'toString', 'hasOwnProperty', '__proto__'])
expect(headerColorClass(input)).toBeUndefined();
});

it('trims surrounding whitespace before looking up', () => {
expect(headerColorClass(' muted ')).toBe('bg-muted');
});
});

describe('headerColor — (1) the scanner can extract every class it can emit', () => {
it('the vocabulary is non-empty and every entry is a complete `bg-` class', () => {
expect(vocabularyClasses.length).toBeGreaterThan(0);
for (const cls of vocabularyClasses) {
expect(cls.startsWith('bg-')).toBe(true);
// A complete token, not a fragment awaiting concatenation.
expect(cls).not.toMatch(/[${}`\s]/);
}
});

it('every class appears VERBATIM in the module source the @source glob reads', () => {
// This is the property the v4 extractor needs and the old code lacked.
for (const cls of vocabularyClasses) expect(moduleSource).toContain(`'${cls}'`);
});

it('neither the module nor the call sites build a class by interpolation', () => {
// The regression pin. `bg-` + an interpolation is never a complete token,
// so it contributes nothing to the stylesheet — measured on the console
// build, deleting the old expression left the compiled CSS byte-identical.
// Scoped to the colour-utility prefixes: an interpolated React `key` or
// DOM id is not this defect, and four of them live in sibling files here.
const interpolatedUtility =
/`(?:bg|text|border|ring|from|via|to|fill|stroke|shadow|outline|decoration|divide|placeholder)-\$\{/;
for (const [name, src] of [['headerColor.ts', moduleSource], ['DetailSection.tsx', callSiteSource]] as const) {
expect(src, `${name} must not interpolate a Tailwind class`).not.toMatch(interpolatedUtility);
}
// …and the call sites do go through the resolver, so the check above is
// not passing because `headerColor` stopped being read at all.
expect(callSiteSource.match(/headerColorClass\(section\.headerColor\)/g) ?? []).toHaveLength(2);
});
});

describe('headerColor — (2) Tailwind emits a rule for every class it can emit', () => {
it('the instrument answers NO for a non-utility (control)', async () => {
const ds = await designSystem();
// `bg-` is exactly what the extractor could take from the old template
// literal, and it builds nothing — the defect, at the compiler.
expect(ds.candidatesToCss(['bg-'])[0]).toBeNull();
expect(ds.candidatesToCss(['bg-not-a-token'])[0]).toBeNull();
expect(ds.candidatesToCss(['bg-mutedd'])[0]).toBeNull();
// …and YES for a class this workspace's theme defines, so a green result
// below means "emitted", not "instrument inert".
expect(ds.candidatesToCss(['bg-muted'])[0]).toContain('background-color');
});

it('every vocabulary class builds against the shipped @theme', async () => {
const ds = await designSystem();
const built = Object.fromEntries(
vocabularyClasses.map((cls) => [cls, ds.candidatesToCss([cls])[0]]),
);
for (const cls of vocabularyClasses) {
expect(built[cls], `${cls} produced no CSS rule`).toBeTruthy();
expect(built[cls]).toContain('background-color');
}
});
});
Loading
Loading