diff --git a/.changeset/6178-detail-section-header-color.md b/.changeset/6178-detail-section-header-color.md new file mode 100644 index 0000000000..9a1a18887d --- /dev/null +++ b/.changeset/6178-detail-section-header-color.md @@ -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. diff --git a/packages/plugin-detail/src/DetailSection.tsx b/packages/plugin-detail/src/DetailSection.tsx index f6082e8e80..960ec26aa4 100644 --- a/packages/plugin-detail/src/DetailSection.tsx +++ b/packages/plugin-detail/src/DetailSection.tsx @@ -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, @@ -510,7 +511,7 @@ export const DetailSection: React.FC = ({ return ( {section.title && ( - +
{section.icon && } @@ -539,7 +540,7 @@ export const DetailSection: React.FC = ({
diff --git a/packages/plugin-detail/src/__tests__/DetailSection.headerColor.test.tsx b/packages/plugin-detail/src/__tests__/DetailSection.headerColor.test.tsx new file mode 100644 index 0000000000..e1d97f2f12 --- /dev/null +++ b/packages/plugin-detail/src/__tests__/DetailSection.headerColor.test.tsx @@ -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 => + ({ 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('.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( + , + ); + 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( + , + ); + 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( + , + ); + expect(classesOf(headerOf(container))).toContain('bg-primary/10'); + }); + + it('a value that is already a `bg-*` class passes through, not doubled', () => { + const { container } = render( + , + ); + 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( + , + ); + 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( + , + ); + const classes = classesOf(headerOf(container)); + expect(classes.filter((c) => c.startsWith('bg-'))).toEqual([]); + expect(classes.some((c) => c.includes('Object'))).toBe(false); + }); + }); +}); diff --git a/packages/plugin-detail/src/__tests__/headerColor.test.ts b/packages/plugin-detail/src/__tests__/headerColor.test.ts new file mode 100644 index 0000000000..1c6ee63886 --- /dev/null +++ b/packages/plugin-detail/src/__tests__/headerColor.test.ts @@ -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-` 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'); + } + }); +}); diff --git a/packages/plugin-detail/src/headerColor.ts b/packages/plugin-detail/src/headerColor.ts new file mode 100644 index 0000000000..dcd0dc2cea --- /dev/null +++ b/packages/plugin-detail/src/headerColor.ts @@ -0,0 +1,92 @@ +/** + * 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. + */ + +/** + * `DetailViewSection.headerColor` -> a Tailwind background class. + * + * ## Why a lookup and not a template literal (objectui#6178) + * + * Tailwind v4 has no runtime. It builds the stylesheet by scanning source + * TEXT for complete class tokens (the `@source` globs in each consuming app's + * CSS entry; this workspace ships no `bg-*` safelist). The previous read here + * was a template literal — `bg-` concatenated with the authored value — which + * is never a complete token in scanned text, so the call site contributed + * NOTHING to the compiled stylesheet. Measured, not assumed: compiling + * `apps/console/src/index.css` with the template literal deleted produced a + * byte-identical stylesheet (same sha256, 441302 bytes), and offering `bg-` + * to the design system as a candidate emits no rule. + * + * An authored value therefore styled the header only when some OTHER source + * file happened to use the identical class literally — `bg-muted` appears 691 + * times and `bg-primary/10` 63 times elsewhere in the workspace, which is why + * both documented examples appeared to work. That liveness was accidental and + * unversioned: it moved with unrelated edits in unrelated packages. + * + * Every class below is a COMPLETE literal, present verbatim in this file, and + * this file is inside every consuming app's scan (`packages/plugin-detail/ + * src/**` in `apps/console`, `examples/console-starter`, and + * `examples/byo-backend-console`). The key now works because this module + * declares it, not because a neighbour happens to. + * + * ## Shape + * + * The same one this repo already uses for the sibling problem — `useRowColor`'s + * `COLOR_TO_CLASS` in `@object-ui/plugin-grid`: a lookup of literal classes, a + * verbatim pass-through for a value that is already a `bg-*` class, and + * `undefined` for everything else. Never a fabricated class string. + * + * The AGENTS.md custom-property carve-out (`bg-[color:var(--os-…)]` fed by an + * inline custom property, as `getBadgeHexAppearance` does in `@object-ui/ + * fields`) is the right answer for a key whose value is a CSS COLOUR. It does + * not fit this one: `headerColor` is documented as a Tailwind class and its + * values are design-system tokens (`muted`, `primary/10`), which have no + * meaning as a CSS colour. + * + * ## Vocabulary + * + * Tints only. `CardHeader` sets no foreground colour, so a solid `bg-primary` + * or `bg-destructive` would leave the section title unreadable; those need a + * paired `text-*-foreground` and are left to the pass-through, where the + * pairing is the author's explicit choice. Both values the `@object-ui/types` + * mirror documents (`muted`, `primary/10`) are in the map, so nothing that + * worked before this module stops working. + */ +const HEADER_COLOR_CLASSES: Readonly> = Object.freeze({ + muted: 'bg-muted', + 'muted/50': 'bg-muted/50', + accent: 'bg-accent', + 'primary/10': 'bg-primary/10', + 'secondary/10': 'bg-secondary/10', + 'destructive/10': 'bg-destructive/10', +}); + +/** The declared vocabulary, for tests and for callers that enumerate it. */ +export const headerColorVocabulary = HEADER_COLOR_CLASSES; + +/** + * Resolve an authored `headerColor` to a class the stylesheet actually + * carries, or `undefined` when there is none — never a concatenated guess. + * + * A value that is already a complete `bg-*` class is handed through + * untouched, exactly as `useRowColor` does. It then renders on the same terms + * as any `className` a schema carries: the host app's Tailwind build has to + * generate it. + */ +export function headerColorClass(headerColor: string | undefined): string | undefined { + if (!headerColor) return undefined; + const value = headerColor.trim(); + if (!value) return undefined; + if (value.startsWith('bg-')) return value; + // `hasOwnProperty` rather than a bare index: a plain object literal inherits + // `constructor`, `toString` and friends, and indexing it with an authored + // string would hand one of those back as the "class". (`Object.hasOwn` is + // ES2022; this workspace compiles against the ES2020 lib.) + return Object.prototype.hasOwnProperty.call(HEADER_COLOR_CLASSES, value) + ? HEADER_COLOR_CLASSES[value] + : undefined; +}