feat(ui): add @frontmcp/ui library for platform-aware HTMX components #76
Conversation
- Add comprehensive theme system with Tailwind v4 Browser CDN support - Add platform configurations (OpenAI, Claude, Gemini, Ngrok) with capability detection - Add base layout system with responsive sizing and alignment options - Add UI components: button, card, form, badge, alert, avatar, modal, table, list - Add page templates for consent and error flows - Add OpenAI App SDK resource widgets - Add HTMX support across all components for progressive enhancement - Add script caching for blocked-network platforms (Claude Artifacts) - Add comprehensive tests with 95%+ coverage (327 tests) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughAdds a new publishable UI library at libs/ui: components, Zod schemas, validation helpers, layouts/pages, theme/CDN/platform systems, widgets, tests, and project/tooling metadata (Nx project, tsconfigs, Jest, ESLint, SWC, package.json, README, docs). Changes
Sequence Diagram(s)sequenceDiagram
participant Consumer
participant Component
participant Validator
participant Theme
participant CDN
participant Renderer
Consumer->>Component: call component(options)
Component->>Validator: validateOptions(options, schema)
alt validation success
Validator-->>Component: typed data
Component->>Theme: merge/apply theme
Theme-->>Component: theme data
Component->>CDN: buildCdnScriptsFromTheme (optional)
CDN-->>Component: script/link tags
Component->>Renderer: assemble escaped HTML
Renderer-->>Component: HTML string
Component-->>Consumer: HTML string
else validation failure
Validator-->>Component: errorHtml
Component-->>Consumer: errorHtml
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Areas to focus review on:
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (33)
libs/ui/README.md (1)
1-11: Consider expanding documentation for a publishable library.This README is the default Nx boilerplate. For a publishable SDK library (
@frontmcp/ui), consider adding:
- Brief API overview and usage examples
- Security considerations (e.g., how XSS is mitigated via
escapeHtml)- Known limitations
- Link to full documentation
As per coding guidelines, documentation files should clearly document known limitations and security concerns.
libs/ui/src/components/list.test.ts (1)
12-265: Add tests for edge cases and error scenarios.The test coverage is comprehensive for happy paths, but per coding guidelines, error scenarios should also be tested. Consider adding:
- Empty array handling: What happens when
permissionList([]),featureList([]), etc. are called with empty arrays?- XSS prevention: Test that malicious input like
<script>alert('xss')</script>in names/descriptions is properly escaped.- Undefined/null fields: Test items with missing optional fields to ensure graceful handling.
Example for XSS testing:
it('should escape HTML in permission names', () => { const perms: PermissionItem[] = [{ scope: 'test', name: '<script>alert("xss")</script>' }]; const html = permissionList(perms); expect(html).not.toContain('<script>'); expect(html).toContain('<script>'); });libs/ui/src/components/modal.ts (1)
315-334: Consider addingcloseOnEscapeoption to DrawerOptions for consistency.The
ModalOptionsinterface includescloseOnEscape(line 33), butDrawerOptionsdoes not. For API consistency across overlay components, consider adding this option to drawers as well.libs/ui/src/theme/platforms.ts (1)
191-196: Shallow merge may not behave as expected for nestedoptions.The spread operator
{ ...preset, ...base }performs a shallow merge. Ifbaseincludes anoptionsobject, it will completely replace the preset'soptionsrather than merging them.If deep merging of
optionsis intended, consider:export function createPlatform( base: Partial<PlatformCapabilities> & { id: PlatformId; name: string }, ): PlatformCapabilities { const preset = PLATFORM_PRESETS[base.id] ?? CUSTOM_PLATFORM; - return { ...preset, ...base }; + return { + ...preset, + ...base, + options: { ...preset.options, ...base.options }, + }; }libs/ui/src/components/button.ts (1)
39-44: Consider documenting that icon parameters expect trusted HTML.The
iconBefore,iconAfter, andiconOnlyoptions accept raw HTML strings that are rendered without escaping. While this is necessary for SVG icons, it could be an XSS vector if misused with user input.Add a JSDoc note to clarify:
- /** Icon before text */ + /** Icon before text (trusted HTML - do not use with user input) */ iconBefore?: string; - /** Icon after text */ + /** Icon after text (trusted HTML - do not use with user input) */ iconAfter?: string;libs/ui/src/components/button.test.ts (1)
105-115: Consider testing button fallback whenhrefis provided butdisabledorloadingis true.The implementation (button.ts lines 212-218) renders a
<button>instead of<a>whendisabledorloadingis true, even ifhrefis provided. This behavior isn't covered by tests.Add tests to verify this edge case:
it('should render button instead of anchor when disabled with href', () => { const html = button('Link', { href: '/path', disabled: true }); expect(html).toContain('<button'); expect(html).not.toContain('<a'); }); it('should render button instead of anchor when loading with href', () => { const html = button('Link', { href: '/path', loading: true }); expect(html).toContain('<button'); expect(html).not.toContain('<a'); });libs/ui/src/components/form.ts (2)
96-99: Icon parameters accept raw HTML without escaping.Similar to the button component,
iconBeforeandiconAfterare rendered without escaping. This is intentional for SVG icons but should be documented to prevent misuse with user input.Add documentation:
- /** Icon before input */ + /** Icon before input (trusted HTML - do not use with user input) */ iconBefore?: string; - /** Icon after input */ + /** Icon after input (trusted HTML - do not use with user input) */ iconAfter?: string;Also applies to: 323-329
644-670: Duplicate HTMX attribute building logic.The
form()function builds HTMX attributes inline (lines 644-654) instead of reusingbuildFormHtmxAttrs(). However,FormOptions.htmxhas additional fields (put,delete,indicator,confirm) not inInputOptions.htmx, so a shared helper would need to be more generic.Consider extracting a shared generic HTMX builder or documenting why the inline approach is used here.
libs/ui/.spec.swcrc (1)
1-22: Confirm SWC module type and loose transforms match your Jest/runtime expectationsThis SWC config looks reasonable for TS + legacy decorators, but two knobs are worth a quick sanity check:
module.type: "es6": depending on how Jest is configured, you might prefer"commonjs"for transforms to avoid any subtle ESM interop issues."loose": true: fine for tests, but be aware it can diverge slightly from strict TypeScript emit semantics.If tests are all green, this is acceptable as‑is; otherwise, these are the first levers I’d tweak.
libs/ui/src/theme/theme.test.ts (1)
1-132: Theme tests give solid coverage; consider a few extra assertions for full branch coverageThe suite thoroughly exercises
DEFAULT_THEME,createTheme, andbuildThemeCss, including overrides andcustomVars, which is great for stability of the theming API.If you want to push coverage a bit further, you could optionally:
- Add a test that sets
colors.customand verifies those keys are emitted as--color-*CSS vars.- Assert a couple more typography/radius tokens (e.g.,
--font-mono,--radius-2xl) to catch accidental regressions there.Not blocking, just a potential future enhancement.
libs/ui/src/components/badge.test.ts (1)
1-238: Badge tests are comprehensive; a few assertions could be made more specificThe suite does a nice job covering variants, sizes, dots, HTMX removal, XSS escaping, groups, and status presets, which should give good confidence in the badge API.
As a small optional improvement, for things like size/gap checks you might assert the exact expected classes (e.g.,
gap-2/gap-4/gap-6, or concretepx-*values per size) instead of just matching'gap-'or'px-', to better guard against accidental class mapping changes.libs/ui/tsconfig.spec.json (1)
1-10: Spec tsconfig aligns with Jest; revisitmoduleResolution: "node10"for consistencyUsing CommonJS and adding
jest/nodetypes for the spec config is appropriate for Jest.The only thing I’d question is
moduleResolution: "node10", which is a bit dated compared to the"NodeNext"resolution used inlibs/ui/tsconfig.json. For consistency (and to avoid subtle path‑resolution differences between build and test type‑checking), consider aligning this to"NodeNext"or whatever matches your actual runtime target—unless you specifically neednode10here.libs/ui/tsconfig.lib.json (1)
1-10: Library tsconfig is reasonable; consider tightening environment assumptionsThis config is fine for emitting declarations and excluding tests.
Two optional refinements to consider:
- If the UI library itself doesn’t rely on Node globals or Node‑specific APIs, you might drop
"types": ["node"]to avoid pulling Node ambient types into consumers that use the published.d.ts.- If you plan to use TypeScript project references with
tsc --build, adding"composite": truehere can make that workflow smoother and silence TS’s build‑mode warnings.Neither is blocking; just worth a quick check against your intended publish/runtime story.
libs/ui/src/components/card.test.ts (1)
1-167: Card tests thoroughly exercise the API; only minor assertion hardening is optionalThese tests cover the important behavior: visual variants, sizing, headers/footers, ids, clickable/anchor modes, HTMX/data attributes, grouping, and XSS prevention on
title. That’s a solid safety net for the card builder.If you want to be extra strict, you could:
- Assert the exact gap classes for
cardGroup(gap-2/gap-4/gap-6) instead of just'gap-'.- Add a case that verifies HTMX/data attributes also appear on the
<a>variant whenhrefis provided (if that’s an expected use‑case).Purely optional; the current suite is already in good shape.
libs/ui/src/components/form.test.ts (3)
356-361: ExpandformRowtest coverage.The test only verifies the presence of
gridclass. Consider adding tests for:
- Gap options (
sm,md,lg)- Custom className propagation
- Correct column count in the class (
grid-cols-${fields.length})describe('formRow', () => { it('should render fields in a grid', () => { const html = formRow([input({ name: 'first' }), input({ name: 'last' })]); expect(html).toContain('grid'); + expect(html).toContain('grid-cols-2'); + }); + + it('should apply gap classes', () => { + const gaps = ['sm', 'md', 'lg'] as const; + for (const gap of gaps) { + const html = formRow([input({ name: 'a' }), input({ name: 'b' })], { gap }); + expect(html).toContain('gap-'); + } + }); + + it('should apply custom className', () => { + const html = formRow([input({ name: 'a' })], { className: 'my-row' }); + expect(html).toContain('my-row'); }); });
233-271: Add test for checkbox error message rendering.The
checkboxfunction accepts anerrorprop (per the implementation), but there's no test verifying error message rendering. As per coding guidelines, test all code paths including error scenarios.it('should render helper text', () => { const html = checkbox({ name: 'agree', label: 'I agree', helper: 'Please read terms first', }); expect(html).toContain('Please read terms first'); }); + + it('should render error message', () => { + const html = checkbox({ + name: 'agree', + label: 'I agree', + error: 'You must agree to continue', + }); + expect(html).toContain('You must agree to continue'); + expect(html).toContain('text-danger'); + }); });
130-142: Consider adding test for data attributes.The
inputfunction supports adataprop for custom data attributes, but this isn't tested. Consider adding coverage:it('should include data attributes', () => { const html = input({ name: 'email', data: { testid: 'email-input', custom: 'value' }, }); expect(html).toContain('data-testid="email-input"'); expect(html).toContain('data-custom="value"'); });libs/ui/jest.config.ts (1)
1-8: Consider narrowing the eslint-disable scope.The blanket
/* eslint-disable */disables all ESLint rules for the file. Consider narrowing to the specific rule(s) being violated, if any:-/* eslint-disable */ +/* eslint-disable @typescript-eslint/no-require-imports */ import { readFileSync } from 'fs';If no specific rule is being violated, the comment can be removed entirely.
libs/ui/src/theme/cdn.test.ts (2)
165-168: Missing cleanup ofglobal.fetchmock after tests.The
global.fetchmock is set in multiple tests but never restored. This can cause test pollution and flaky behavior in subsequent test files.Add an
afterEachhook to restore the original fetch:describe('Script Caching', () => { + const originalFetch = global.fetch; + beforeEach(() => { clearScriptCache(); }); + + afterEach(() => { + global.fetch = originalFetch; + jest.restoreAllMocks(); + });
259-300: Consider adding a test for partial fetch failure infetchAndCacheScripts.The
fetchAndCacheScriptsfunction usesPromise.allwhich will reject if any fetch fails, but this error propagation isn't tested. Per coding guidelines, error scenarios should be covered.it('should propagate errors when fetch fails', async () => { global.fetch = jest.fn() .mockResolvedValueOnce({ ok: true, text: () => Promise.resolve('content') }) .mockResolvedValueOnce({ ok: false, status: 500 }); await expect(fetchAndCacheScripts({ tailwind: true, htmx: true })) .rejects.toThrow('Failed to fetch script'); });libs/ui/src/theme/platforms.test.ts (2)
73-98: Consider testinggetPlatformwith an unknown platform ID.Per the implementation in
platforms.ts,getPlatformreturnsCUSTOM_PLATFORMas fallback for unknown IDs via the nullish coalescing operator. This fallback behavior isn't explicitly tested.it('should return Custom platform as fallback for unknown id', () => { // @ts-expect-error - testing runtime behavior with invalid id const platform = getPlatform('unknown-platform'); expect(platform).toEqual(CUSTOM_PLATFORM); });
1-13: Missing test forsupportsFullInteractivityutility function.Based on the AI summary,
supportsFullInteractivityis part of the public API inplatforms.ts, but it's not imported or tested in this file. Per coding guidelines, all code paths should be tested.Add import and test:
import { OPENAI_PLATFORM, CLAUDE_PLATFORM, GEMINI_PLATFORM, NGROK_PLATFORM, CUSTOM_PLATFORM, getPlatform, createPlatform, canUseCdn, needsInlineScripts, + supportsFullInteractivity, getFallbackMode, PlatformCapabilities, } from './platforms';Then add a test block:
describe('supportsFullInteractivity', () => { it('should return true for platforms with widgets, tailwind, and htmx', () => { expect(supportsFullInteractivity(OPENAI_PLATFORM)).toBe(true); }); it('should return false for platforms missing htmx', () => { expect(supportsFullInteractivity(CLAUDE_PLATFORM)).toBe(false); }); });libs/ui/src/layouts/base.test.ts (1)
183-190: Consider adding a test for platform that doesn't support HTMX.The test verifies HTMX inclusion for
OPENAI_PLATFORM, butCLAUDE_PLATFORMhassupportsHtmx: false. Testing this case would ensure proper behavior when HTMX should be excluded.it('should not include HTMX when platform does not support it', () => { const html = baseLayout('<div>Content</div>', { title: 'Test', platform: CLAUDE_PLATFORM, includeHtmx: true, // Request HTMX but platform doesn't support it }); // Verify HTMX is excluded or handled appropriately // Adjust expectation based on actual implementation behavior });libs/ui/src/layouts/base.ts (2)
187-199: Unusedthemeparameter.The
themeparameter is declared but never used in the function body. Either remove it or use it for the pattern background definition.-function getBackgroundClasses(background: BackgroundStyle, theme: ThemeConfig): string { +function getBackgroundClasses(background: BackgroundStyle): string { switch (background) { case 'gradient': return 'bg-gradient-to-br from-primary to-secondary'; case 'pattern': return 'bg-surface bg-[url("data:image/svg+xml,...")]'; // Pattern would be defined case 'solid': return 'bg-background'; case 'none': default: return ''; } }Also update the call site at line 323:
- const backgroundClasses = getBackgroundClasses(background, theme); + const backgroundClasses = getBackgroundClasses(background);
272-284: UsemergeThemesinstead of manual spread for consistency.The
mergeThemesfunction is imported but not used here. The manual merge misses nested properties liketypography,radius,shadows,components, andcustomVarsthatmergeThemeshandles. This creates inconsistency withcreateLayoutBuilderwhich correctly usesmergeThemes.- // Merge theme - const theme: ThemeConfig = { - ...DEFAULT_THEME, - ...themeOverrides, - colors: { - ...DEFAULT_THEME.colors, - ...themeOverrides?.colors, - semantic: { ...DEFAULT_THEME.colors.semantic, ...themeOverrides?.colors?.semantic }, - surface: { ...DEFAULT_THEME.colors.surface, ...themeOverrides?.colors?.surface }, - text: { ...DEFAULT_THEME.colors.text, ...themeOverrides?.colors?.text }, - border: { ...DEFAULT_THEME.colors.border, ...themeOverrides?.colors?.border }, - }, - }; + // Merge theme + const theme: ThemeConfig = themeOverrides + ? mergeThemes(DEFAULT_THEME, themeOverrides) + : DEFAULT_THEME;libs/ui/src/components/avatar.ts (1)
100-106: Edge case: empty string input.If
nameis an empty string,parts[0].charAt(0)will return an empty string, but the function handles this gracefully. However, consider adding a fallback for empty names.function generateInitials(name: string): string { const parts = name.trim().split(/\s+/); + if (parts.length === 0 || parts[0] === '') { + return '?'; + } if (parts.length === 1) { return parts[0].charAt(0).toUpperCase(); } return (parts[0].charAt(0) + parts[parts.length - 1].charAt(0)).toUpperCase(); }libs/ui/src/widgets/progress.ts (1)
101-126: Consider deduplicating inline styles for animated progress bars.When
animated=true, a<style>block is appended to each progress bar. If multiple animated progress bars are rendered on the same page, this creates duplicate CSS definitions.Consider either:
- Moving these styles to the theme/CDN system
- Using a unique identifier to inject the styles only once per page
- Documenting that consumers should include these styles globally if using multiple animated progress bars
libs/ui/src/pages/consent.ts (1)
8-8: Remove unused import.
dangerButtonis imported but never used in this file.-import { primaryButton, outlineButton, dangerButton } from '../components/button'; +import { primaryButton, outlineButton } from '../components/button';libs/ui/src/layouts/presets.ts (2)
316-329: Verify JavaScript string escaping in autoClose script.The
continueUrlis escaped withescapeHtml(), which converts'to'. While HTML entity encoding typically works within inline<script>blocks (the browser parses them as HTML first), a dedicated JavaScript string escaper would be more robust for this context.Consider adding a
escapeJsStringutility that escapes backslashes, quotes, and control characters for JavaScript string literals:+ function escapeJsString(str: string): string { + return str + .replace(/\\/g, '\\\\') + .replace(/'/g, "\\'") + .replace(/"/g, '\\"') + .replace(/\n/g, '\\n') + .replace(/\r/g, '\\r'); + }Then use it in the script:
- ${continueUrl ? `window.location.href = '${escapeHtml(continueUrl)}';` : 'window.close();'} + ${continueUrl ? `window.location.href = '${escapeJsString(continueUrl)}';` : 'window.close();'}
426-451: Consider adding missing layout builders for consistency.The module exports builders for
auth,consent, anderrorlayouts, but not forloading,success, orwidget. If this is intentional (these layouts are less commonly customized), consider adding a brief comment explaining the rationale. Otherwise, adding the missing builders would provide a more consistent API surface.libs/ui/src/widgets/resource.ts (2)
36-51: Consider constraining the index signature type.The
ResourceMetainterface uses[key: string]: unknownwhich allows any additional properties. While flexible, this may conflict with the explicitly defined properties due to TypeScript's index signature requirements.Per coding guidelines requiring strict TypeScript, consider using a discriminated approach:
export interface ResourceMeta { /** Creation timestamp */ createdAt?: string | Date; /** Last modified timestamp */ updatedAt?: string | Date; /** Author/creator */ author?: string; /** File size (bytes) */ size?: number; /** MIME type */ mimeType?: string; /** Tags */ tags?: string[]; - /** Custom metadata */ - [key: string]: unknown; } + +export type ResourceMetaWithCustom = ResourceMeta & Record<string, unknown>;
167-174: Consider parameterizing the locale for i18n support.The
formatDatefunction hardcodes'en-US'locale. For a UI library that may be used internationally, consider accepting an optional locale parameter:-function formatDate(date: string | Date): string { +function formatDate(date: string | Date, locale = 'en-US'): string { const d = typeof date === 'string' ? new Date(date) : date; - return d.toLocaleDateString('en-US', { + return d.toLocaleDateString(locale, { year: 'numeric', month: 'short', day: 'numeric', }); }libs/ui/src/components/table.ts (1)
189-193: Type assertion forrowKeydefault may cause silent failures.The default
rowKey = 'id' as keyof Tassumes all data types have anidproperty. If they don't,row[rowKey]will beundefined, falling back torowIndex(line 219), which works but the type assertion is misleading.Per coding guidelines for strict TypeScript, consider:
- rowKey = 'id' as keyof T, + rowKey,Then handle the undefined case explicitly:
- const rowId = String(row[rowKey] || rowIndex); + const rowId = rowKey && row[rowKey] !== undefined + ? String(row[rowKey]) + : String(rowIndex);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
libs/ui/.spec.swcrc(1 hunks)libs/ui/README.md(1 hunks)libs/ui/eslint.config.mjs(1 hunks)libs/ui/jest.config.ts(1 hunks)libs/ui/package.json(1 hunks)libs/ui/project.json(1 hunks)libs/ui/src/components/alert.test.ts(1 hunks)libs/ui/src/components/alert.ts(1 hunks)libs/ui/src/components/avatar.ts(1 hunks)libs/ui/src/components/badge.test.ts(1 hunks)libs/ui/src/components/badge.ts(1 hunks)libs/ui/src/components/button.test.ts(1 hunks)libs/ui/src/components/button.ts(1 hunks)libs/ui/src/components/card.test.ts(1 hunks)libs/ui/src/components/card.ts(1 hunks)libs/ui/src/components/form.test.ts(1 hunks)libs/ui/src/components/form.ts(1 hunks)libs/ui/src/components/index.ts(1 hunks)libs/ui/src/components/list.test.ts(1 hunks)libs/ui/src/components/list.ts(1 hunks)libs/ui/src/components/modal.ts(1 hunks)libs/ui/src/components/table.ts(1 hunks)libs/ui/src/index.ts(1 hunks)libs/ui/src/layouts/base.test.ts(1 hunks)libs/ui/src/layouts/base.ts(1 hunks)libs/ui/src/layouts/index.ts(1 hunks)libs/ui/src/layouts/presets.ts(1 hunks)libs/ui/src/pages/consent.ts(1 hunks)libs/ui/src/pages/error.ts(1 hunks)libs/ui/src/pages/index.ts(1 hunks)libs/ui/src/theme/cdn.test.ts(1 hunks)libs/ui/src/theme/cdn.ts(1 hunks)libs/ui/src/theme/index.test.ts(1 hunks)libs/ui/src/theme/index.ts(1 hunks)libs/ui/src/theme/platforms.test.ts(1 hunks)libs/ui/src/theme/platforms.ts(1 hunks)libs/ui/src/theme/theme.test.ts(1 hunks)libs/ui/src/theme/theme.ts(1 hunks)libs/ui/src/widgets/index.ts(1 hunks)libs/ui/src/widgets/progress.ts(1 hunks)libs/ui/src/widgets/resource.ts(1 hunks)libs/ui/tsconfig.json(1 hunks)libs/ui/tsconfig.lib.json(1 hunks)libs/ui/tsconfig.spec.json(1 hunks)tsconfig.base.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Document known limitations and security concerns clearly in documentation files
Files:
libs/ui/README.md
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/ui/README.mdlibs/ui/src/theme/theme.test.tslibs/ui/src/components/alert.test.tslibs/ui/package.jsonlibs/ui/src/theme/index.test.tslibs/ui/src/components/list.tslibs/ui/src/index.tslibs/ui/src/components/list.test.tslibs/ui/jest.config.tslibs/ui/src/components/button.test.tslibs/ui/src/components/avatar.tslibs/ui/src/layouts/base.test.tslibs/ui/tsconfig.spec.jsonlibs/ui/src/theme/cdn.test.tslibs/ui/src/widgets/index.tslibs/ui/src/components/form.test.tslibs/ui/src/layouts/base.tslibs/ui/src/components/badge.test.tslibs/ui/src/components/card.test.tslibs/ui/src/components/card.tslibs/ui/src/widgets/progress.tslibs/ui/eslint.config.mjslibs/ui/src/components/modal.tslibs/ui/src/pages/index.tslibs/ui/src/theme/platforms.tslibs/ui/src/pages/consent.tslibs/ui/src/theme/platforms.test.tslibs/ui/src/pages/error.tslibs/ui/tsconfig.jsonlibs/ui/src/layouts/presets.tslibs/ui/tsconfig.lib.jsonlibs/ui/src/components/alert.tslibs/ui/src/widgets/resource.tslibs/ui/src/theme/index.tslibs/ui/src/components/index.tslibs/ui/src/layouts/index.tslibs/ui/project.jsonlibs/ui/src/components/table.tslibs/ui/src/theme/cdn.tslibs/ui/src/components/badge.tslibs/ui/src/components/button.tslibs/ui/src/theme/theme.tslibs/ui/src/components/form.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript mode with no
anytypes without strong justification
Files:
libs/ui/src/theme/theme.test.tslibs/ui/src/components/alert.test.tslibs/ui/src/theme/index.test.tslibs/ui/src/components/list.tslibs/ui/src/index.tslibs/ui/src/components/list.test.tslibs/ui/jest.config.tslibs/ui/src/components/button.test.tslibs/ui/src/components/avatar.tslibs/ui/src/layouts/base.test.tslibs/ui/src/theme/cdn.test.tslibs/ui/src/widgets/index.tslibs/ui/src/components/form.test.tslibs/ui/src/layouts/base.tslibs/ui/src/components/badge.test.tslibs/ui/src/components/card.test.tslibs/ui/src/components/card.tslibs/ui/src/widgets/progress.tslibs/ui/src/components/modal.tslibs/ui/src/pages/index.tslibs/ui/src/theme/platforms.tslibs/ui/src/pages/consent.tslibs/ui/src/theme/platforms.test.tslibs/ui/src/pages/error.tslibs/ui/src/layouts/presets.tslibs/ui/src/components/alert.tslibs/ui/src/widgets/resource.tslibs/ui/src/theme/index.tslibs/ui/src/components/index.tslibs/ui/src/layouts/index.tslibs/ui/src/components/table.tslibs/ui/src/theme/cdn.tslibs/ui/src/components/badge.tslibs/ui/src/components/button.tslibs/ui/src/theme/theme.tslibs/ui/src/components/form.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{test,spec}.{ts,tsx}: In Jest test files, don't use prefixes like 'PT-001' in test names; use clean, descriptive names
Constructor validation tests must not be skipped and error classinstanceofchecks must be tested
Test all code paths including error scenarios
Files:
libs/ui/src/theme/theme.test.tslibs/ui/src/components/alert.test.tslibs/ui/src/theme/index.test.tslibs/ui/src/components/list.test.tslibs/ui/src/components/button.test.tslibs/ui/src/layouts/base.test.tslibs/ui/src/theme/cdn.test.tslibs/ui/src/components/form.test.tslibs/ui/src/components/badge.test.tslibs/ui/src/components/card.test.tslibs/ui/src/theme/platforms.test.ts
**/src/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/src/index.ts: Barrel exports (index.ts) must export everything users need without legacy exports or aliased names
Don't add backwards compatibility exports in new libraries; use direct imports instead
Files:
libs/ui/src/index.ts
🧠 Learnings (7)
📚 Learning: 2025-11-25T19:34:36.034Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:34:36.034Z
Learning: All tests must pass with 100% pass rate and builds must complete without TypeScript warnings
Applied to files:
libs/ui/src/theme/theme.test.tslibs/ui/src/components/form.test.ts
📚 Learning: 2025-11-25T19:34:36.034Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:34:36.034Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Test all code paths including error scenarios
Applied to files:
libs/ui/src/components/alert.test.tslibs/ui/src/components/list.test.tslibs/ui/src/components/button.test.tslibs/ui/src/layouts/base.test.tslibs/ui/tsconfig.spec.jsonlibs/ui/src/components/form.test.tslibs/ui/src/components/card.test.ts
📚 Learning: 2025-11-25T19:34:36.034Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:34:36.034Z
Learning: Applies to **/src/index.ts : Don't add backwards compatibility exports in new libraries; use direct imports instead
Applied to files:
libs/ui/src/theme/index.test.tslibs/ui/src/index.tslibs/ui/src/widgets/index.tslibs/ui/src/pages/index.tslibs/ui/tsconfig.jsonlibs/ui/tsconfig.lib.jsonlibs/ui/src/theme/index.tslibs/ui/src/components/index.tslibs/ui/src/layouts/index.ts
📚 Learning: 2025-11-25T19:34:36.034Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:34:36.034Z
Learning: Applies to **/src/index.ts : Barrel exports (index.ts) must export everything users need without legacy exports or aliased names
Applied to files:
libs/ui/src/theme/index.test.tslibs/ui/src/index.tslibs/ui/src/pages/index.tslibs/ui/src/theme/index.tslibs/ui/src/components/index.tslibs/ui/src/layouts/index.ts
📚 Learning: 2025-11-25T19:34:36.034Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:34:36.034Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : In Jest test files, don't use prefixes like 'PT-001' in test names; use clean, descriptive names
Applied to files:
libs/ui/jest.config.tslibs/ui/tsconfig.spec.jsonlibs/ui/src/components/form.test.ts
📚 Learning: 2025-11-25T19:34:36.034Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:34:36.034Z
Learning: Applies to **/*.{ts,tsx} : Use strict TypeScript mode with no `any` types without strong justification
Applied to files:
libs/ui/tsconfig.spec.jsonlibs/ui/.spec.swcrclibs/ui/tsconfig.jsonlibs/ui/tsconfig.lib.json
📚 Learning: 2025-11-25T19:34:36.034Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:34:36.034Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Constructor validation tests must not be skipped and error class `instanceof` checks must be tested
Applied to files:
libs/ui/src/components/form.test.ts
🧬 Code graph analysis (20)
libs/ui/src/theme/theme.test.ts (2)
libs/ui/src/theme/index.ts (3)
DEFAULT_THEME(61-61)createTheme(63-63)buildThemeCss(64-64)libs/ui/src/theme/theme.ts (3)
DEFAULT_THEME(311-407)createTheme(453-455)buildThemeCss(460-531)
libs/ui/src/components/list.ts (1)
libs/ui/src/layouts/base.ts (1)
escapeHtml(146-153)
libs/ui/src/components/button.test.ts (2)
libs/ui/src/components/button.ts (8)
button(149-223)buttonGroup(228-251)primaryButton(258-259)secondaryButton(262-263)outlineButton(266-267)ghostButton(270-271)dangerButton(274-275)linkButton(278-279)libs/ui/src/components/index.ts (8)
button(12-12)buttonGroup(13-13)primaryButton(14-14)secondaryButton(15-15)outlineButton(16-16)ghostButton(17-17)dangerButton(18-18)linkButton(19-19)
libs/ui/src/components/avatar.ts (2)
libs/ui/src/components/index.ts (9)
AvatarSize(85-85)AvatarShape(86-86)AvatarStatus(87-87)AvatarOptions(88-88)avatar(91-91)AvatarGroupOptions(89-89)avatarGroup(92-92)AvatarWithTextOptions(90-90)avatarWithText(93-93)libs/ui/src/layouts/base.ts (1)
escapeHtml(146-153)
libs/ui/src/theme/cdn.test.ts (1)
libs/ui/src/theme/cdn.ts (9)
CDN(18-62)buildCdnScripts(213-249)buildFontPreconnect(164-168)buildFontStylesheets(173-185)clearScriptCache(133-135)fetchScript(77-90)isScriptCached(126-128)getCachedScript(119-121)fetchAndCacheScripts(96-114)
libs/ui/src/components/form.test.ts (2)
libs/ui/src/components/form.ts (11)
input(245-345)select(350-422)textarea(427-501)checkbox(506-551)radioGroup(556-601)form(641-670)formRow(675-682)formSection(687-706)formActions(711-727)hiddenInput(736-738)csrfInput(743-745)libs/ui/src/components/index.ts (11)
input(37-37)select(38-38)textarea(39-39)checkbox(40-40)radioGroup(41-41)form(42-42)formRow(43-43)formSection(44-44)formActions(45-45)hiddenInput(46-46)csrfInput(47-47)
libs/ui/src/layouts/base.ts (4)
libs/ui/src/layouts/index.ts (5)
PageType(9-9)LayoutSize(10-10)BackgroundStyle(11-11)LayoutAlignment(12-12)BaseLayoutOptions(13-13)libs/ui/src/theme/index.ts (11)
PlatformCapabilities(26-26)ThemeConfig(60-60)OPENAI_PLATFORM(27-27)DEFAULT_THEME(61-61)canUseCdn(35-35)needsInlineScripts(36-36)buildFontPreconnect(16-16)buildFontStylesheets(17-17)buildCdnScripts(18-18)buildThemeCss(64-64)mergeThemes(62-62)libs/ui/src/theme/theme.ts (4)
ThemeConfig(272-302)DEFAULT_THEME(311-407)buildThemeCss(460-531)mergeThemes(416-448)libs/ui/src/theme/cdn.ts (3)
buildFontPreconnect(164-168)buildFontStylesheets(173-185)buildCdnScripts(213-249)
libs/ui/src/components/card.test.ts (2)
libs/ui/src/components/card.ts (2)
card(116-170)cardGroup(175-191)libs/ui/src/components/index.ts (2)
card(23-23)cardGroup(23-23)
libs/ui/src/components/card.ts (1)
libs/ui/src/layouts/base.ts (1)
escapeHtml(146-153)
libs/ui/src/widgets/progress.ts (1)
libs/ui/src/layouts/base.ts (1)
escapeHtml(146-153)
libs/ui/src/theme/platforms.ts (1)
libs/ui/src/theme/index.ts (16)
PlatformId(23-23)NetworkMode(24-24)ScriptStrategy(25-25)PlatformCapabilities(26-26)OPENAI_PLATFORM(27-27)CLAUDE_PLATFORM(28-28)GEMINI_PLATFORM(29-29)NGROK_PLATFORM(30-30)CUSTOM_PLATFORM(31-31)PLATFORM_PRESETS(32-32)getPlatform(33-33)createPlatform(34-34)canUseCdn(35-35)needsInlineScripts(36-36)supportsFullInteractivity(37-37)getFallbackMode(38-38)
libs/ui/src/pages/consent.ts (7)
libs/ui/src/pages/index.ts (8)
ClientInfo(10-10)UserInfo(11-11)ConsentPageOptions(12-12)consentPage(15-15)ConsentSuccessOptions(13-13)consentSuccessPage(16-16)ConsentDeniedOptions(14-14)consentDeniedPage(17-17)libs/ui/src/components/list.ts (2)
PermissionItem(16-31)permissionList(129-185)libs/ui/src/layouts/presets.ts (2)
ConsentLayoutOptions(77-88)consentLayout(93-140)libs/ui/src/components/alert.ts (1)
alert(104-163)libs/ui/src/layouts/base.ts (1)
escapeHtml(146-153)libs/ui/src/components/form.ts (2)
csrfInput(743-745)hiddenInput(736-738)libs/ui/src/components/button.ts (2)
outlineButton(266-267)primaryButton(258-259)
libs/ui/src/theme/platforms.test.ts (1)
libs/ui/src/theme/platforms.ts (11)
OPENAI_PLATFORM(85-97)CLAUDE_PLATFORM(103-117)GEMINI_PLATFORM(123-134)NGROK_PLATFORM(140-151)CUSTOM_PLATFORM(156-164)getPlatform(184-186)createPlatform(191-196)canUseCdn(201-203)needsInlineScripts(208-210)getFallbackMode(222-230)PlatformCapabilities(42-75)
libs/ui/src/layouts/presets.ts (1)
libs/ui/src/layouts/base.ts (4)
BaseLayoutOptions(81-137)baseLayout(254-368)escapeHtml(146-153)createLayoutBuilder(377-398)
libs/ui/src/components/alert.ts (1)
libs/ui/src/layouts/base.ts (1)
escapeHtml(146-153)
libs/ui/src/widgets/resource.ts (4)
libs/ui/src/components/badge.ts (2)
BadgeVariant(16-16)badge(95-161)libs/ui/src/components/card.ts (2)
CardOptions(26-57)card(116-170)libs/ui/src/layouts/base.ts (1)
escapeHtml(146-153)libs/ui/src/components/button.ts (1)
button(149-223)
libs/ui/src/components/table.ts (1)
libs/ui/src/layouts/base.ts (1)
escapeHtml(146-153)
libs/ui/src/components/badge.ts (1)
libs/ui/src/layouts/base.ts (1)
escapeHtml(146-153)
libs/ui/src/theme/theme.ts (1)
libs/ui/src/theme/index.ts (18)
ColorScale(43-43)SemanticColors(44-44)SurfaceColors(45-45)TextColors(46-46)BorderColors(47-47)ThemeColors(48-48)FontFamilies(49-49)FontSizes(50-50)FontWeights(51-51)ThemeTypography(52-52)ThemeSpacing(53-53)ThemeRadius(54-54)ThemeShadows(55-55)ButtonTokens(56-56)CardTokens(57-57)InputTokens(58-58)ComponentTokens(59-59)ThemeConfig(60-60)
libs/ui/src/components/form.ts (1)
libs/ui/src/layouts/base.ts (1)
escapeHtml(146-153)
🪛 Biome (2.1.2)
libs/ui/src/widgets/progress.ts
[error] 472-478: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 ESLint
libs/ui/src/widgets/progress.ts
[error] 472-478: Unexpected lexical declaration in case block.
(no-case-declarations)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
…th error handling
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
libs/ui/src/theme/theme.ts (3)
423-462: Missing deep merge forspacinganddarkproperties.The
mergeThemesfunction handles nested merging forcolors,typography,components, andcdn, butspacinganddarkare only shallow-merged via the top-level spread. If a user providesoverride.spacingoroverride.dark, it will completely replace the base values rather than merging nested properties.Consider adding explicit handling:
return { ...base, ...override, colors: { // ... existing color merging }, typography: { // ... existing typography merging }, + spacing: { ...base.spacing, ...override.spacing }, radius: { ...base.radius, ...override.radius }, shadows: { ...base.shadows, ...override.shadows }, components: { // ... existing component merging }, cdn: { // ... existing cdn merging }, + dark: override.dark !== undefined + ? mergeThemes(base, override.dark as Partial<ThemeConfig>) + : base.dark, customVars: { ...base.customVars, ...override.customVars }, customCss: [base.customCss, override.customCss].filter(Boolean).join('\n'), };
474-491:ColorScalevalues for semantic colors are silently ignored;neutralis not handled.The
SemanticColorsinterface allowsprimary,secondary,accent, andneutralto be eitherstringorColorScale, butbuildThemeCssonly handles string values. If a user provides aColorScaleobject, no CSS variables are generated. Additionally,neutralis never processed.Consider adding ColorScale support:
+function emitColorScale(lines: string[], name: string, scale: ColorScale): void { + for (const [shade, value] of Object.entries(scale)) { + if (value) lines.push(`--color-${name}-${shade}: ${value};`); + } +} + export function buildThemeCss(theme: ThemeConfig): string { const lines: string[] = []; // Colors - semantic const semantic = theme.colors.semantic; if (typeof semantic.primary === 'string') { lines.push(`--color-primary: ${semantic.primary};`); + } else if (semantic.primary) { + emitColorScale(lines, 'primary', semantic.primary); } + // Handle neutral + if (semantic.neutral) { + if (typeof semantic.neutral === 'string') { + lines.push(`--color-neutral: ${semantic.neutral};`); + } else { + emitColorScale(lines, 'neutral', semantic.neutral); + } + } // ... similar for secondary, accent
529-535:radius.noneandradius.fullare not emitted to CSS.The
ThemeRadiusinterface includesnoneandfullproperties, andGITHUB_OPENAI_THEMEdefines them, butbuildThemeCssdoesn't generate CSS variables for these values.// Radius const radius = theme.radius; + if (radius?.none) lines.push(`--radius-none: ${radius.none};`); if (radius?.sm) lines.push(`--radius-sm: ${radius.sm};`); if (radius?.md) lines.push(`--radius-md: ${radius.md};`); if (radius?.lg) lines.push(`--radius-lg: ${radius.lg};`); if (radius?.xl) lines.push(`--radius-xl: ${radius.xl};`); if (radius?.['2xl']) lines.push(`--radius-2xl: ${radius['2xl']};`); + if (radius?.full) lines.push(`--radius-full: ${radius.full};`);libs/ui/src/theme/cdn.ts (1)
76-83: Add integrity hash for Lucide icons to match security practices.The
iconsentry lacks a Subresource Integrity (SRI) hash, unlikehtmxandalpine. This inconsistency weakens the security posture for this resource.icons: { url: 'https://cdn.jsdelivr.net/npm/lucide@0.294.0/dist/umd/lucide.min.js', + integrity: 'sha384-43WP8IQ+5H0ncT+LNM4dZnu+hPINYmeOuNMhTvHfszzXdFjBEji77gkq7TyjQl/U', },
🧹 Nitpick comments (9)
libs/ui/src/theme/cdn.ts (2)
89-111: Module-level cache lacks test isolation mechanism.The
scriptCacheis a module-level singleton. WhileclearScriptCache()exists, tests running in parallel could interfere with each other. This is acceptable for now, but consider documenting this behavior.The implementation is correct for production use. For test isolation, the
clearScriptCache()function should be called in test setup/teardown.
359-361: Icons script tag missing integrity attribute when available.When building CDN script tags, if the icons config has an integrity hash (either from theme or after adding it to CDN.icons), it should be included in the script tag. Currently, the icons script tag is built without passing the integrity.
if (icons) { - scripts.push(buildScriptTag(iconsConfig.url)); + scripts.push(buildScriptTag(iconsConfig.url, iconsConfig.integrity)); }This applies to both
buildCdnScripts(line 265) andbuildCdnScriptsFromTheme(line 360).libs/ui/src/validation/wrapper.test.ts (1)
1-281: Validation wrapper tests are comprehensive; consider one extra edge caseThe tests do a good job covering successful validation, transformed data, various failure modes (required, type, unknown, nested, arrays, enums), and the
withValidationcontrol flow including “don’t call component on failure”.One small optional improvement: add a case that triggers a Zod error with an empty
path(e.g., a top-level refinement) so thegetFirstInvalidPathfallback to"options"is exercised as well. Not a blocker, but it would close the last defensive branch.libs/ui/src/validation/wrapper.ts (1)
1-145: Tighten generic typing so options type is tied to the Zod schemaRuntime logic looks good, but at the type level
Tisn’t currently connected toconfig.schema:
ValidationConfigusesschema: ZodSchemawithout a generic.validateOptions<T>’sTis unconstrained and inferred asunknownunless callers pass an explicit type.result.datais cast toTwithas T, so TypeScript can’t actually guarantee the match.You can make the public API more type-safe (and drop the cast) by parameterizing
ValidationConfigon the options type and usingZodSchema<TOptions>:-import { type ZodSchema } from 'zod'; +import { type ZodSchema } from 'zod'; -export interface ValidationConfig { +export interface ValidationConfig<TOptions> { /** Name of the component being validated */ componentName: string; /** Zod schema to validate against */ - schema: ZodSchema; + schema: ZodSchema<TOptions>; } -export type ValidationResult<T> = { success: true; data: T } | { success: false; error: string }; +export type ValidationResult<T> = { success: true; data: T } | { success: false; error: string }; -export function validateOptions<T>(options: unknown, config: ValidationConfig): ValidationResult<T> { - const result = config.schema.safeParse(options); +export function validateOptions<TOptions>( + options: unknown, + config: ValidationConfig<TOptions>, +): ValidationResult<TOptions> { + const result = config.schema.safeParse(options); if (result.success) { - return { success: true, data: result.data as T }; + return { success: true, data: result.data }; } @@ -export function withValidation<TInput, TOptions>( - componentFn: (input: TInput, options: TOptions) => string, - config: ValidationConfig, -): (input: TInput, options: unknown) => string { +export function withValidation<TInput, TOptions>( + componentFn: (input: TInput, options: TOptions) => string, + config: ValidationConfig<TOptions>, +): (input: TInput, options: unknown) => string { return (input: TInput, options: unknown) => { - const result = validateOptions<TOptions>(options, config); + const result = validateOptions<TOptions>(options, config);This preserves the runtime behavior while giving consumers fully typed
result.dataandcomponentFnoptions based on the schema they provide.libs/ui/CLAUDE.md (2)
17-32: Add language specifier to fenced code block.The architecture diagram code block lacks a language specifier. For plain text diagrams, use an empty identifier or
textto satisfy markdown linting.-``` +```text libs/ui/src/ ├── components/ # UI components (button, card, form, etc.)
246-250: Add language specifier to fenced code block.Same issue — the file naming conventions block lacks a language specifier.
-``` +```text component.ts # Component implementation component.schema.ts # Zod schema definitions component.test.ts # Jest tests (include validation tests)</blockquote></details> <details> <summary>libs/ui/src/components/card.schema.ts (1)</summary><blockquote> `51-60`: **Consider aligning HTMX schema with other components.** `CardHtmxSchema` lacks `put` and `delete` methods that are present in `ButtonHtmxSchema`. If cards can trigger state-changing HTMX actions, consider adding these for consistency. Otherwise, if this is intentional (cards typically don't perform destructive actions), this is fine as-is. </blockquote></details> <details> <summary>libs/ui/src/components/form.schema.ts (1)</summary><blockquote> `77-87`: **The `validate` property is correctly implemented but lacks documentation explaining its mapping to `hx-validate`.** The `validate` boolean in `FormHtmxSchema` (line 84) is properly converted to the HTMX attribute `hx-validate="true"` in the rendering logic (see `buildFormHtmxAttrs` in `form.ts` line 228). This design—using a boolean for type safety in the schema and converting it to a string attribute during rendering—is sound. However, add a JSDoc comment explaining this mapping to clarify for developers how the schema property translates to HTML output. </blockquote></details> <details> <summary>libs/ui/src/components/button.ts (1)</summary><blockquote> `212-217`: **Inconsistent escaping: `target` attribute is not escaped.** Other attributes (`id`, `name`, `value`, `href`) use `escapeHtml()`, but `target` does not. While `target` typically contains safe values like `_blank`, applying consistent escaping prevents edge cases where user input flows through. ```diff - const targetAttr = target ? `target="${target}"` : ''; + const targetAttr = target ? `target="${escapeHtml(target)}"` : '';
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
libs/ui/CLAUDE.md(1 hunks)libs/ui/README.md(1 hunks)libs/ui/package.json(1 hunks)libs/ui/src/components/alert.schema.ts(1 hunks)libs/ui/src/components/avatar.schema.ts(1 hunks)libs/ui/src/components/badge.schema.ts(1 hunks)libs/ui/src/components/button.schema.ts(1 hunks)libs/ui/src/components/button.test.ts(1 hunks)libs/ui/src/components/button.ts(1 hunks)libs/ui/src/components/card.schema.ts(1 hunks)libs/ui/src/components/form.schema.ts(1 hunks)libs/ui/src/components/index.ts(1 hunks)libs/ui/src/components/list.schema.ts(1 hunks)libs/ui/src/components/modal.schema.ts(1 hunks)libs/ui/src/components/table.schema.ts(1 hunks)libs/ui/src/index.ts(1 hunks)libs/ui/src/theme/cdn.ts(1 hunks)libs/ui/src/theme/index.ts(1 hunks)libs/ui/src/theme/presets/github-openai.ts(1 hunks)libs/ui/src/theme/presets/index.ts(1 hunks)libs/ui/src/theme/theme.test.ts(1 hunks)libs/ui/src/theme/theme.ts(1 hunks)libs/ui/src/validation/error-box.test.ts(1 hunks)libs/ui/src/validation/error-box.ts(1 hunks)libs/ui/src/validation/index.ts(1 hunks)libs/ui/src/validation/wrapper.test.ts(1 hunks)libs/ui/src/validation/wrapper.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- libs/ui/src/index.ts
- libs/ui/src/theme/theme.test.ts
- libs/ui/src/components/button.test.ts
- libs/ui/README.md
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript mode with no
anytypes without strong justification
Files:
libs/ui/src/validation/index.tslibs/ui/src/theme/presets/index.tslibs/ui/src/theme/presets/github-openai.tslibs/ui/src/components/button.schema.tslibs/ui/src/components/card.schema.tslibs/ui/src/components/badge.schema.tslibs/ui/src/components/list.schema.tslibs/ui/src/components/alert.schema.tslibs/ui/src/validation/error-box.tslibs/ui/src/validation/wrapper.test.tslibs/ui/src/validation/error-box.test.tslibs/ui/src/theme/theme.tslibs/ui/src/components/form.schema.tslibs/ui/src/theme/index.tslibs/ui/src/components/avatar.schema.tslibs/ui/src/components/index.tslibs/ui/src/validation/wrapper.tslibs/ui/src/components/modal.schema.tslibs/ui/src/theme/cdn.tslibs/ui/src/components/table.schema.tslibs/ui/src/components/button.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/ui/src/validation/index.tslibs/ui/src/theme/presets/index.tslibs/ui/src/theme/presets/github-openai.tslibs/ui/src/components/button.schema.tslibs/ui/src/components/card.schema.tslibs/ui/src/components/badge.schema.tslibs/ui/src/components/list.schema.tslibs/ui/src/components/alert.schema.tslibs/ui/src/validation/error-box.tslibs/ui/src/validation/wrapper.test.tslibs/ui/src/validation/error-box.test.tslibs/ui/src/theme/theme.tslibs/ui/src/components/form.schema.tslibs/ui/src/theme/index.tslibs/ui/src/components/avatar.schema.tslibs/ui/src/components/index.tslibs/ui/src/validation/wrapper.tslibs/ui/src/components/modal.schema.tslibs/ui/package.jsonlibs/ui/src/theme/cdn.tslibs/ui/src/components/table.schema.tslibs/ui/src/components/button.tslibs/ui/CLAUDE.md
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{test,spec}.{ts,tsx}: In Jest test files, don't use prefixes like 'PT-001' in test names; use clean, descriptive names
Constructor validation tests must not be skipped and error classinstanceofchecks must be tested
Test all code paths including error scenarios
Files:
libs/ui/src/validation/wrapper.test.tslibs/ui/src/validation/error-box.test.ts
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Document known limitations and security concerns clearly in documentation files
Files:
libs/ui/CLAUDE.md
🧠 Learnings (4)
📚 Learning: 2025-11-25T19:34:36.034Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:34:36.034Z
Learning: Applies to **/src/index.ts : Don't add backwards compatibility exports in new libraries; use direct imports instead
Applied to files:
libs/ui/src/validation/index.tslibs/ui/src/theme/index.tslibs/ui/src/components/index.ts
📚 Learning: 2025-11-25T19:34:36.034Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:34:36.034Z
Learning: Applies to **/src/index.ts : Barrel exports (index.ts) must export everything users need without legacy exports or aliased names
Applied to files:
libs/ui/src/validation/index.tslibs/ui/src/theme/index.tslibs/ui/src/components/index.ts
📚 Learning: 2025-11-25T19:34:36.034Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:34:36.034Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Test all code paths including error scenarios
Applied to files:
libs/ui/src/validation/wrapper.test.tslibs/ui/src/validation/error-box.test.ts
📚 Learning: 2025-11-25T19:34:36.034Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:34:36.034Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Constructor validation tests must not be skipped and error class `instanceof` checks must be tested
Applied to files:
libs/ui/src/validation/wrapper.test.tslibs/ui/src/validation/error-box.test.ts
🧬 Code graph analysis (11)
libs/ui/src/components/badge.schema.ts (1)
libs/ui/src/components/index.ts (3)
BadgeVariant(70-70)BadgeSize(71-71)BadgeOptions(72-72)
libs/ui/src/components/list.schema.ts (1)
libs/ui/src/components/index.ts (7)
PermissionItem(132-132)PermissionListOptions(135-135)FeatureItem(133-133)FeatureListOptions(136-136)DescriptionItem(134-134)DescriptionListOptions(137-137)ActionItem(138-138)
libs/ui/src/components/alert.schema.ts (1)
libs/ui/src/components/index.ts (3)
AlertVariant(89-89)AlertOptions(90-90)ToastOptions(91-91)
libs/ui/src/validation/wrapper.test.ts (2)
libs/ui/src/validation/index.ts (2)
validateOptions(16-16)withValidation(16-16)libs/ui/src/validation/wrapper.ts (2)
validateOptions(90-107)withValidation(132-145)
libs/ui/src/validation/error-box.test.ts (2)
libs/ui/src/validation/error-box.ts (1)
validationErrorBox(74-90)libs/ui/src/validation/index.ts (1)
validationErrorBox(13-13)
libs/ui/src/components/form.schema.ts (1)
libs/ui/src/components/index.ts (9)
InputType(45-45)InputSize(46-46)InputState(47-47)InputOptions(48-48)SelectOptions(50-50)TextareaOptions(51-51)CheckboxOptions(52-52)RadioGroupOptions(53-53)FormOptions(54-54)
libs/ui/src/components/avatar.schema.ts (1)
libs/ui/src/components/index.ts (6)
AvatarSize(103-103)AvatarShape(104-104)AvatarStatus(105-105)AvatarOptions(106-106)AvatarGroupOptions(107-107)AvatarWithTextOptions(108-108)
libs/ui/src/validation/wrapper.ts (1)
libs/ui/src/validation/error-box.ts (1)
validationErrorBox(74-90)
libs/ui/src/components/modal.schema.ts (1)
libs/ui/src/components/index.ts (5)
ModalSize(116-116)ModalOptions(117-117)ConfirmModalOptions(118-118)DrawerPosition(119-119)DrawerOptions(120-120)
libs/ui/src/theme/cdn.ts (1)
libs/ui/src/theme/theme.ts (1)
ThemeConfig(363-396)
libs/ui/src/components/table.schema.ts (1)
libs/ui/src/components/index.ts (3)
TableColumn(128-128)TableOptions(128-128)PaginationOptions(128-128)
🪛 markdownlint-cli2 (0.18.1)
libs/ui/CLAUDE.md
17-17: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
246-246: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (45)
libs/ui/src/theme/theme.ts (3)
1-40: LGTM! Well-documented module header with clear examples.The file header provides comprehensive documentation with usage examples and proper module annotations.
41-143: LGTM! Color type definitions are well-structured.The color system with
ColorScale,SemanticColors,SurfaceColors,TextColors,BorderColors, andThemeColorsprovides a comprehensive and flexible structure for theming.
547-560: LGTM!buildStyleBlockprovides clean Tailwind @theme integration.The function correctly wraps the theme CSS and custom CSS in a style block with proper Tailwind type annotation.
libs/ui/src/theme/presets/index.ts (1)
1-12: LGTM! Clean barrel export for theme presets.The module correctly re-exports the theme presets with proper documentation.
libs/ui/src/theme/presets/github-openai.ts (4)
1-46: LGTM! Well-documented theme preset with clear color palette documentation.The header clearly documents the design inspiration and provides usage examples.
47-96: LGTM! Comprehensive color configuration with proper semantic naming.The color palette follows GitHub/OpenAI design principles with clear semantic meaning for each token.
192-195: LGTM! Clean alias export for backwards compatibility.The
DEFAULT_THEMEalias provides a clear default for consumers.
164-189: CDN values in theme are duplicated from cdn.ts with no synchronization mechanism.The theme's CDN configuration (lines 164-189) contains values identical to those in
libs/ui/src/theme/cdn.ts(e.g., Lucide version 0.294.0, HTMX 2.0.7, Alpine 3.14.3). If cdn.ts is updated for security patches or library upgrades, this file must also be updated or the values will diverge and potentially conflict.However, the suggested import approach won't work directly due to structural differences:
CDN.iconsexports{ url: string }whileThemeCdnConfigexpects{ script?: { url: string } }. Consider one of these alternatives:
- Create a helper function in cdn.ts that builds a
ThemeCdnConfigfrom theCDNobject, then import and use it.- Document the dependency with a comment linking to the corresponding values in cdn.ts so maintainers know to keep them synchronized.
- Re-export as theme-compatible by adding a transformation layer in cdn.ts.
libs/ui/src/theme/index.ts (1)
1-93: LGTM! Comprehensive barrel export following best practices.The index correctly re-exports all public API surface from CDN, platforms, theme, and presets modules without legacy exports or aliased names. Based on learnings, this follows the project's convention for barrel exports.
libs/ui/src/theme/cdn.ts (3)
1-29: LGTM! Well-documented CDN module with clear usage examples.The header provides comprehensive documentation about the module's purpose and usage patterns.
98-111: LGTM!fetchScriptcorrectly handles caching and error states.The function properly checks cache first, throws on non-OK responses with descriptive errors, and caches successful responses.
374-399: LGTM! Theme-aware script caching follows the same pattern as non-theme variant.The function correctly resolves URLs from theme config with fallback to CDN defaults.
libs/ui/src/validation/index.ts (1)
1-16: Validation barrel is clean and aligned with public API goalsThe module-level docs and re-exports are cohesive, type-only exports are used correctly, and there are no legacy/aliased names. This gives consumers a clear, minimal validation surface.
libs/ui/src/validation/error-box.test.ts (1)
1-170: Thorough coverage of error-box rendering and XSS behaviorThe suite exercises the main output shape, attributes, nested paths, and escaping (including quotes) as well as styling and accessibility. This gives strong regression protection for the HTML contract of
validationErrorBox.libs/ui/src/validation/error-box.ts (1)
1-90: Validation error box component is well-structured and safeThe API surface is clear (
ValidationErrorBoxOptions+validationErrorBox), dynamic values are consistently escaped before use in attributes/text, and the generated markup includes good testing and accessibility hooks. Looks solid as a reusable validation primitive.libs/ui/CLAUDE.md (1)
1-309: Well-structured SDK development guidelines.The documentation thoroughly covers architecture, validation patterns, platform support, theme system, and anti-patterns. Security considerations (XSS escaping, error handling) are appropriately highlighted.
libs/ui/src/components/card.schema.ts (1)
1-129: LGTM!The schema definitions follow the documented patterns correctly: strict mode enabled, proper type inference via
z.infer, comprehensive JSDoc, and well-structured option groupings.libs/ui/src/components/button.schema.ts (1)
1-152: LGTM!Comprehensive button schema with proper strict validation, complete HTMX attribute coverage (including
confirmandindicator), and well-organized type exports. The separation betweenButtonOptionsSchemaandButtonGroupOptionsSchemais clean.libs/ui/src/components/badge.schema.ts (1)
1-127: LGTM!The badge schema correctly handles the removable badge pattern with HTMX support. The variant set is comprehensive and the
onRemoveschema provides appropriate HTMX hooks for dynamic badge removal.libs/ui/src/components/list.schema.ts (2)
181-189: Action item HTMX schema is intentionally minimal.The HTMX schema here only includes
get,post,target, andswap, whileButtonHtmxSchemaincludes additional attributes liketrigger,confirm, andindicator. This seems intentional for simpler action items, but verify this meets your use cases.
1-217: Well-designed list schemas for common UI patterns.The four list types (Permission, Feature, Description, Action) cover common UI patterns effectively. The
PermissionItemSchemawithrequired,checked, andsensitiveflags is particularly well-suited for OAuth consent flows.libs/ui/src/components/form.schema.ts (1)
1-507: Comprehensive form schema coverage.Excellent coverage of form elements with proper validation constraints (e.g.,
rows: z.number().min(1),cols: z.number().min(1).max(12)). The separation of HTMX schemas per element type appropriately reflects different interaction patterns.libs/ui/src/components/modal.schema.ts (2)
168-189: Good schema reuse pattern.The
DrawerOptionsSchemacorrectly reusesModalSizeSchemaandModalOnCloseSchema, following DRY principles while maintaining schema consistency across modal-like components.
1-219: LGTM!Well-structured modal schemas with appropriate accessibility considerations (
closeOnBackdrop,closeOnEscape). The confirm modal pattern with variants (info,warning,danger) and drawer positioning options are well-designed for common use cases.libs/ui/package.json (1)
42-45: Both specified package versions are valid and published on npm — no issues found.The Zod version
^3.25.76and TypeScript version^5.9.3both exist as published npm packages. Zod 3.25.76 was released as a transition version to Zod 4, and TypeScript 5.9.3 was published on September 30, 2025. Package installation will succeed without errors.libs/ui/src/components/alert.schema.ts (4)
1-33: LGTM! Well-structured variant schema with proper type inference.Clean Zod schema definition with
z.enumfor strict validation and proper TypeScript type derivation usingz.infer.
42-54: Redundant.optional()on AlertOnDismissSchema.The schema already has
.optional()at the end (line 49), so when used inAlertOptionsSchema(line 82), it doesn't need another.optional(). However, this is intentional design - making the schema itself optional allows reuse in contexts where optionality is already implied.
63-89: LGTM! Comprehensive alert options schema with strict validation.Good use of
.strict()to prevent extraneous properties. The schema properly captures all alert configuration options with appropriate types.
98-137: LGTM! Toast schemas are well-defined.The
ToastPositionSchemacovers all standard positioning options, andToastOptionsSchemaproperly validates duration withz.number().min(0)to prevent negative values.libs/ui/src/components/avatar.schema.ts (4)
27-55: LGTM! Enum schemas are well-defined.Size, shape, and status enums cover appropriate values. The
'2xl'size value is correctly defined as a string literal in the enum.
64-90: LGTM! Avatar options schema with requiredaltfor accessibility.Good design choice making
altrequired (line 69) - this ensures accessibility compliance by mandating alt text for all avatars.
104-120: LGTM! Avatar group options properly validated.The
maxfield correctly uses.min(1)to ensure at least one avatar is visible.
129-141: LGTM! Clean schema extension pattern.Using
AvatarOptionsSchema.extend({...}).strict()is the correct Zod pattern for extending schemas while maintaining strict validation.libs/ui/src/components/table.schema.ts (4)
42-68: Justified use ofz.any()for render function parameters.The comment on lines 40-41 and 56 properly justifies using
z.any()for the render function - Zod cannot validate runtime function implementations. This aligns with the coding guideline requiring justification foranytypes.
77-95: LGTM! HTMX schemas properly structured.Sort and select HTMX schemas correctly define required (
get/post) and optional (target,swap) fields with.strict().optional()pattern.
104-144: LGTM! Comprehensive table options schema.The schema covers all table features including HTMX integration, row selection, sorting, and UI states (loading, empty, compact, etc.).
165-197: I'll help you verify this pagination edge case concern. Let me search the codebase to understand how the pagination logic handles the scenario wherepage >= 1buttotalPagesis 0.
<function_calls>
#!/bin/bashFirst, locate table.ts and examine pagination-related logic
fd -t f 'table.ts' libs/ui/src/components/ 2>/dev/null | head -20
</function_calls>Now let me search for pagination-related functions and how the schema is used:
<function_calls>
#!/bin/bashSearch for pagination handling in table component files
rg -n 'pagination|PaginationOptions' libs/ui/src/components/ -A3 -B3 --type=ts --type=tsx | head -80
</function_calls>Let me also check the test files to see if there are any edge case tests for pagination:
<function_calls>
#!/bin/bashSearch for pagination-related tests
fd -t f '.test.|.spec.' libs/ui/src/components/ -x basename {} ; | grep -i table
</function_calls><function_calls>
#!/bin/bashCheck for test files related to table or pagination
find libs/ui/src -name 'table' -o -name 'pagination' | grep -E '.(test|spec).(ts|tsx|js)$'
</function_calls>Let me search more broadly for how PaginationOptions is actually consumed in the codebase:
<function_calls>
#!/bin/bashSearch for imports and usage of PaginationOptions
rg -n 'PaginationOptions' libs/ui/src/ -B2 -A5 --type=ts --type=tsx
</function_calls><function_calls>
#!/bin/bashLook at the table.schema.ts file to see context around the schema
cat -n libs/ui/src/components/table.schema.ts | sed -n '150,210p'
</function_calls>libs/ui/src/components/index.ts (2)
13-22: LGTM! Schema barrel exports follow best practices.Using
export *for schema modules provides clean access to all Zod schemas and derived types without aliasing.
24-143: LGTM! Well-organized component exports with no legacy aliases.The barrel file correctly exports all types and functions with direct imports, following the guideline to avoid backwards compatibility exports in new libraries. Consistent pattern used across all component modules.
Based on learnings, this properly exports everything users need without aliased names.
libs/ui/src/components/button.ts (6)
1-74: LGTM! Well-documented module with proper imports and type re-exports.Good JSDoc with usage examples. Clean import/export pattern that re-exports types from the schema module for consumer convenience.
83-119: LGTM! Type-safe variant and size class mappings.Using
Record<ButtonVariant, string>andRecord<ButtonSize, string>ensures exhaustive coverage of all variant/size values at compile time.
124-137: LGTM! HTMX attributes properly escaped.All HTMX attribute values are passed through
escapeHtml()to prevent XSS in user-provided URLs and targets.
154-239: LGTM! Robust button builder with validation and proper escaping.Good pattern: validates options with Zod schema upfront and returns error box on invalid input. HTML output properly escapes user content.
248-274: LGTM! Button group implementation with attached and gap options.Clean implementation handling both attached (merged borders) and separated button groups with configurable gap sizes and directions.
280-302: LGTM! Type-safe convenience functions.Using
Omit<ButtonOptions, 'variant'>prevents users from accidentally overriding the variant in shorthand functions.
…ling and caching warnings
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
libs/ui/src/theme/theme.ts (1)
423-463: RevisitmergeThemeshandling of thedarkvariant (base vsbase.dark, nesteddark).The current implementation:
dark: override.dark !== undefined ? mergeThemes(base, override.dark as Partial<ThemeConfig>) : base.dark,has a few problematic behaviors:
When both
base.darkandoverride.darkare defined,base.darkis effectively ignored: you mergeoverride.darkon top of the lightbasetheme instead of on top ofbase.dark, so any dark-specific defaults from the base theme are lost.The recursive call to
mergeThemesfor the dark variant will itself compute a.darkfield, sotheme.darkends up containing its own nested.darkproperty. Conceptually you usually wanttheme.darkto be a terminal variant, not a tree of nested dark themes.This diverges from the pattern used for other nested properties (
colors,typography,components,cdn), where you deep-mergebase.<prop>withoverride.<prop>rather than discarding the base’s specialized sub-config.Consider restructuring so that:
- You derive a
darkBasefrombase.darkwhen present (e.g. “base theme with its own dark overrides applied”), and- You merge
override.darkon top of that, while ensuring the recursive merge used fordarkdoes not itself reintroduce a.darkproperty into the dark variant.One way to get there is to factor out a helper that merges all fields except
dark, then explicitly computedarkon top ofbase.darkusing that helper, so you don’t end up with nested dark variants and you preservebase.darkwhen both sides provide dark config.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
libs/ui/CLAUDE.md(1 hunks)libs/ui/src/components/button.ts(1 hunks)libs/ui/src/theme/cdn.ts(1 hunks)libs/ui/src/theme/theme.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- libs/ui/CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/ui/src/theme/cdn.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript mode with no
anytypes without strong justification
Files:
libs/ui/src/theme/theme.tslibs/ui/src/components/button.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/ui/src/theme/theme.tslibs/ui/src/components/button.ts
🧬 Code graph analysis (1)
libs/ui/src/components/button.ts (3)
libs/ui/src/components/button.schema.ts (6)
ButtonVariant(37-37)ButtonSize(47-47)ButtonOptions(127-127)ButtonOptionsSchema(83-122)ButtonGroupOptions(152-152)ButtonGroupOptionsSchema(136-147)libs/ui/src/layouts/base.ts (1)
escapeHtml(146-153)libs/ui/src/validation/wrapper.ts (1)
validateOptions(90-107)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
libs/ui/src/components/button.ts (3)
62-74: LGTM: Imports and type exports are well-structured.The imports are clean and the type re-exports appropriately expose the public API surface.
83-137: LGTM: Helper functions are correctly implemented.The variant/size mapping is exhaustive, and
buildHtmxAttrsproperly escapes all HTMX attribute values to prevent injection.
280-302: LGTM: Convenience functions are well-typed.The shorthand functions correctly use
Omit<ButtonOptions, 'variant'>to prevent the variant from being overridden.libs/ui/src/theme/theme.ts (1)
474-520: Color scale + neutral + radius tokens in CSS output look solid.
emitColorScaleplus the updated semantic color handling now correctly emit variables forColorScale(includingneutral), and the radius block now includesnoneandfull. This closes the earlier gaps where semantic scale values and some radius tokens were silently ignored bybuildThemeCss.Also applies to: 558-566
…accessibility warnings
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
libs/ui/src/components/button.ts (1)
258-259: Consider consistent icon behavior during loading state.
iconBeforeis hidden whenloading: true(line 258), buticonAfterremains visible (line 259). This creates visual asymmetry during the loading state. Consider hiding both icons during loading for consistency, or explicitly document if the current behavior is intentional.If both icons should hide during loading, apply this diff:
const iconBeforeHtml = iconBefore && !loading ? `<span class="${iconOnly ? '' : 'mr-2'}">${iconBefore}</span>` : ''; - const iconAfterHtml = iconAfter ? `<span class="${iconOnly ? '' : 'ml-2'}">${iconAfter}</span>` : ''; + const iconAfterHtml = iconAfter && !loading ? `<span class="${iconOnly ? '' : 'ml-2'}">${iconAfter}</span>` : '';
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libs/ui/src/components/button.ts(1 hunks)libs/ui/src/theme/theme.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript mode with no
anytypes without strong justification
Files:
libs/ui/src/components/button.tslibs/ui/src/theme/theme.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/ui/src/components/button.tslibs/ui/src/theme/theme.ts
🧬 Code graph analysis (1)
libs/ui/src/theme/theme.ts (3)
libs/ui/src/theme/index.ts (29)
ColorScale(57-57)SemanticColors(58-58)SurfaceColors(59-59)TextColors(60-60)BorderColors(61-61)ThemeColors(62-62)FontFamilies(64-64)FontSizes(65-65)FontWeights(66-66)ThemeTypography(67-67)ThemeSpacing(69-69)ThemeRadius(70-70)ThemeShadows(71-71)ButtonTokens(73-73)CardTokens(74-74)InputTokens(75-75)ComponentTokens(76-76)CdnScriptResource(78-78)ThemeCdnFonts(79-79)ThemeCdnIcons(80-80)ThemeCdnScripts(81-81)ThemeCdnConfig(82-82)ThemeConfig(84-84)GITHUB_OPENAI_THEME(93-93)DEFAULT_THEME(93-93)mergeThemes(86-86)createTheme(87-87)buildThemeCss(88-88)buildStyleBlock(89-89)libs/ui/src/theme/presets/index.ts (2)
GITHUB_OPENAI_THEME(12-12)DEFAULT_THEME(12-12)libs/ui/src/theme/presets/github-openai.ts (2)
GITHUB_OPENAI_THEME(47-190)DEFAULT_THEME(195-195)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (9)
libs/ui/src/components/button.ts (1)
319-340: LGTM! Clean shorthand implementations.The convenience functions follow a consistent pattern with proper type safety using
Omit<ButtonOptions, 'variant'>to prevent variant conflicts. This provides an ergonomic API without sacrificing type correctness.libs/ui/src/theme/theme.ts (8)
1-39: LGTM: Comprehensive documentation and clean imports.The file header provides excellent documentation with clear usage examples. The import aliasing pattern allows for clean re-exports while avoiding naming conflicts.
41-396: LGTM: Well-structured and comprehensive type definitions.The type system is well-designed with clear separation of concerns, proper use of optional properties, and excellent documentation. The support for both simple strings and ColorScale objects for semantic colors provides good flexibility. No
anytypes detected, complying with strict TypeScript guidelines.
398-414: LGTM: Clean re-exports with appropriate documentation.The default theme constants are properly re-exported with clear documentation about their characteristics.
466-498: LGTM: Sophisticated dark variant handling with proper documentation.The function correctly handles dark mode variants while preventing infinite nesting of
.darkproperties. The three-case logic (override.dark provided, only base.dark exists, neither exists) is sound, and the destructuring approach cleanly strips nested.darkproperties. The JSDoc clearly documents the behavior.
500-505: LGTM: Simple and effective theme creation helper.Clean wrapper that provides a convenient API for extending the default theme.
507-514: LGTM: Clean color scale CSS variable emission.The helper correctly iterates over color scale entries and emits properly formatted CSS variables with appropriate guards for undefined values.
611-642: LGTM: Clear security documentation for customCss trust model.The extensive JSDoc (lines 614-628) clearly documents that
customCssis injected without sanitization and is intended for trusted, developer-defined themes only. This addresses security concerns through explicit documentation of the trust model rather than attempting complex CSS sanitization. The implementation correctly wraps theme CSS variables in a Tailwind@themedirective with optional custom CSS appended.
519-609: Document whytheme.spacing,theme.shadows, andtheme.typographydetails are not emitted to CSS variables.The
buildThemeCssfunction deliberately emits only colors, font families, and radii, while excluding:
theme.spacing(ThemeSpacing) — actively defined in presets and merged increateThemetheme.shadows(ThemeShadows) — actively defined in GITHUB_OPENAI_THEME presettheme.typography.sizes,theme.typography.weights,theme.typography.lineHeight— present in presets but only font families are emittedThis appears intentional (no test coverage for these properties, no sections in the function), but it's undocumented. Either:
- Tailwind CSS v4
@themedoesn't support these properties in the current syntax- They're intentionally excluded and should be applied via
customCssor Tailwind defaultsAdd a comment to the function's JSDoc explaining this design decision so users understand why configuring these properties in
ThemeConfigdoesn't automatically generate corresponding CSS variables.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
libs/ui/src/components/button.ts (1)
73-75: Ensure project-level docs for new public button API indocs/draft/docs/**This module adds/exports public APIs (
button,buttonGroup, and shorthand variants) from a publishable library underlibs/**. Per coding guidelines, such public surfaces should have matching documentation underdocs/draft/docs/**, not only inlibs/ui/README.md. Prior tooling already flagged the absence of component docs indocs/draft/docs/**.Please verify that a button/UI components page has been (or will be) added and wired into the draft docs navigation.
#!/bin/bash # Verify presence of draft docs for @frontmcp/ui button components echo "=== Looking for button/UI docs under docs/draft/docs ===" find docs/draft/docs -iname '*button*' -o -iname '*ui*' -o -iname '*components*' 2>/dev/null | sortAlso applies to: 283-321
🧹 Nitpick comments (2)
libs/ui/src/components/button.ts (2)
245-249: Sanitizedata-*attribute keys to avoid malformed attributes / potential injection
datavalues are safely escaped, but keys are interpolated directly into attribute names (data-${key}) without validation. If a consumer ever passes an untrusted key containing spaces or quotes, it could produce broken markup or additional attributes (and, in worst cases, an XSS vector if misused).Consider constraining/sanitizing keys before emitting:
+function sanitizeDataKey(key: string): string | null { + const sanitized = key.toLowerCase().replace(/[^a-z0-9_-]/g, '-').replace(/-+/g, '-').replace(/^-|-$/g, ''); + if (!sanitized) { + console.warn(`[frontmcp/ui] Dropping invalid data-* key: "${key}"`); + return null; + } + return sanitized; +} + const dataAttrs = data ? Object.entries(data) - .map(([key, val]) => `data-${key}="${escapeHtml(val)}"`) - .join(' ') + .map(([key, val]) => { + const safeKey = sanitizeDataKey(key); + return safeKey ? `data-${safeKey}="${escapeHtml(val)}"` : ''; + }) + .filter(Boolean) + .join(' ') : '';This keeps the API flexible while hardening against malformed attribute names.
148-161: Alignhrefdocs with allowlist behavior for clarity
isValidHrefProtocoluses an allowlist (http:,https:,/,#,mailto:,tel:), but the JSDoc remark only mentions blockingjavascript:,data:, andvbscript:. To avoid confusion for consumers (especially when a relative URL like"login"silently falls back to<button>), consider tightening the docs:
- Explicitly document which schemes/patterns are allowed.
- Note that bare relative URLs (e.g.
"login"or"./foo") are intentionally rejected and should instead use"/login"etc.This is just a doc/expectations tweak; the implementation itself looks good.
Also applies to: 170-176
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libs/ui/src/components/button.ts(1 hunks)libs/ui/src/theme/theme.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use strict TypeScript mode with noanytypes without strong justification; preferunknownfor generic type defaults
Avoid non-null assertions (!) - use proper error handling and throw specific error classes instead
Files:
libs/ui/src/components/button.tslibs/ui/src/theme/theme.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/ui/src/components/button.tslibs/ui/src/theme/theme.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Create shared base classes for common functionality (e.g., ExecutionContextBase) to reduce code duplication
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/ast-guard/SECURITY-AUDIT.md : Document all blocked attack vectors and security considerations in SECURITY-AUDIT.md
Applied to files:
libs/ui/src/components/button.tslibs/ui/src/theme/theme.ts
🧬 Code graph analysis (2)
libs/ui/src/components/button.ts (3)
libs/ui/src/components/button.schema.ts (6)
ButtonVariant(37-37)ButtonSize(47-47)ButtonOptions(127-127)ButtonOptionsSchema(83-122)ButtonGroupOptions(152-152)ButtonGroupOptionsSchema(136-147)libs/ui/src/layouts/base.ts (1)
escapeHtml(146-153)libs/ui/src/validation/wrapper.ts (1)
validateOptions(90-107)
libs/ui/src/theme/theme.ts (3)
libs/ui/src/theme/index.ts (4)
ColorScale(57-57)ThemeConfig(84-84)GITHUB_OPENAI_THEME(93-93)DEFAULT_THEME(93-93)libs/ui/src/theme/presets/index.ts (2)
GITHUB_OPENAI_THEME(12-12)DEFAULT_THEME(12-12)libs/ui/src/theme/presets/github-openai.ts (2)
GITHUB_OPENAI_THEME(47-190)DEFAULT_THEME(195-195)
🔇 Additional comments (7)
libs/ui/src/theme/theme.ts (6)
1-40: LGTM!The module documentation is comprehensive with a clear example demonstrating typical usage patterns. The import aliasing approach for re-exporting constants works correctly.
45-396: LGTM!The type definitions are well-structured with comprehensive interfaces for all theme tokens. Good use of optional properties for flexible partial configuration and clear JSDoc documentation for each interface.
423-485: LGTM!The deep merge implementation correctly handles nested structures. CDN arrays are concatenated rather than replaced (addressing the previous review concern), and script resources are deep-merged to preserve integrity hashes when only URL is overridden.
524-535: LGTM!The
createThemefunction provides a clean API for extending the default theme. TheemitColorScalehelper correctly iterates over color scale entries to emit CSS variables.
632-663: LGTM!The security/trust model for
customCssis now thoroughly documented. The JSDoc clearly explains that theme configurations should only come from trusted sources and warns against passing user input directly. This addresses the previous review concern appropriately for a developer-facing library.
605-627: These theme tokens should be emitted as CSS variables but currently aren't.The
buildThemeCssfunction is documented as generating CSS "from theme configuration" yet omits several defined tokens. TheGITHUB_OPENAI_THEMEpreset populatestypography.sizes,typography.weights,typography.lineHeight,shadows,spacing, andcomponents, but the function never converts them to CSS custom properties. Additionally, test coverage only verifies colors, font families, and radius—the missing tokens have no test assertions. This inconsistency (emitting font families but not sizes/weights, or radius but not shadows/spacing) suggests incomplete implementation rather than intentional exclusion.libs/ui/src/components/button.ts (1)
80-145: Solid, well-structured button API and implementationThe core implementation (variant/size helpers, HTMX attrs, loading spinner,
button,buttonGroup, and shorthand variants) is cohesive, type-safe, and consistent with the schema-driven validation pattern used elsewhere inlibs/ui. Good use ofescapeHtml, Zod validation, and accessibility-focused defaults (focus ring, disabled handling,aria-labelfor icon-only buttons).Also applies to: 182-281, 323-349
…nd accessibility improvements
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
libs/ui/src/components/button.ts (1)
59-75: Ensure matching docs/draft/docs entry for the new button public APIThis file defines and exports new public APIs (
button,buttonGroup, and the variant shorthand functions/types) from a publishable library underlibs/**. Per the coding guidelines, these should be documented indocs/draft/docs/**(not only inlibs/ui/README.md). Please confirm there is a draft docs page describing this API surface; if not, add one and wire it into the sidebar/nav.
🧹 Nitpick comments (3)
libs/ui/src/theme/theme.ts (3)
420-489: mergeThemesCore safely handles partial bases; consider deduping CDN font arrays.The deep‑merge logic for colors, typography, components, and CDN (including preserving
integrityon overrides) looks solid and fixes the earlier unsafe dark‑theme handling. One optional improvement: avoid duplicate entries when mergingfonts.preconnectandfonts.stylesheetsby deduping after concatenation.- fonts: { - // Concatenate arrays rather than replace (allows adding to preconnect/stylesheets) - preconnect: [...(base.cdn?.fonts?.preconnect ?? []), ...(override.cdn?.fonts?.preconnect ?? [])], - stylesheets: [...(base.cdn?.fonts?.stylesheets ?? []), ...(override.cdn?.fonts?.stylesheets ?? [])], - }, + fonts: { + // Concatenate then dedupe so base entries are preserved without duplicates + preconnect: Array.from( + new Set([...(base.cdn?.fonts?.preconnect ?? []), ...(override.cdn?.fonts?.preconnect ?? [])]), + ), + stylesheets: Array.from( + new Set([...(base.cdn?.fonts?.stylesheets ?? []), ...(override.cdn?.fonts?.stylesheets ?? [])]), + ), + },
526-531: createTheme wrapper is clear; ensure it’s treated as the documented entry point.
createThemesimply extendsDEFAULT_THEMEwith overrides, which is exactly what consumers will expect. Given this is a public SDK API, please double‑check that draft docs includecreateTheme(alongsidemergeThemes,DEFAULT_THEME, andGITHUB_OPENAI_THEME) as the primary way to construct themes.
533-635: buildThemeCss covers core tokens; revisit whether typography sizes/spacing should emit CSS vars.The CSS emitter correctly handles semantic (including
ColorScale), surface, text, border, custom colors, font families, radius (includingnone/full), andcustomVars. If typography sizes/line heights and the spacing scale are meant to be consumable via CSS variables in templates, consider also emitting vars for those here (or confirm they’re intentionally only used at the Tailwind config level).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libs/ui/src/components/button.ts(1 hunks)libs/ui/src/theme/theme.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use strict TypeScript mode with noanytypes without strong justification; preferunknownfor generic type defaults
Avoid non-null assertions (!) - use proper error handling and throw specific error classes instead
Files:
libs/ui/src/theme/theme.tslibs/ui/src/components/button.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/ui/src/theme/theme.tslibs/ui/src/components/button.ts
🧠 Learnings (1)
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/ast-guard/SECURITY-AUDIT.md : Document all blocked attack vectors and security considerations in SECURITY-AUDIT.md
Applied to files:
libs/ui/src/theme/theme.tslibs/ui/src/components/button.ts
🧬 Code graph analysis (1)
libs/ui/src/components/button.ts (3)
libs/ui/src/components/button.schema.ts (6)
ButtonVariant(37-37)ButtonSize(47-47)ButtonOptions(127-127)ButtonOptionsSchema(83-122)ButtonGroupOptions(152-152)ButtonGroupOptionsSchema(136-147)libs/ui/src/layouts/base.ts (1)
escapeHtml(146-153)libs/ui/src/validation/wrapper.ts (1)
validateOptions(90-107)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
libs/ui/src/theme/theme.ts (3)
1-415: Theme types and public surface look coherent and TS‑strict.The JSDoc header, imported presets, and the various interfaces (colors, typography, spacing, components, CDN, ThemeConfig) form a clear, strongly typed public API with no
anyor non‑null assertions, which is appropriate for a publishablelibs/SDK surface.
491-524: Dark variant merge behavior is well‑defined and avoids nested.dark.The
mergeThemesimplementation correctly:
- Uses
mergeThemesCorefor the base (light) theme.- Composes
override.darkon top ofbase.dark ?? base.- Strips any nested
.darkfrom both base and override to prevent recursion.This gives predictable dark variant composition without runtime surprises.
637-668: Style block builder and customCss trust model are explicit and appropriate.
buildStyleBlockis straightforward and the docstring clearly spells out thattheme.customCssis trusted, developer‑supplied CSS, not user input. This addresses the XSS concern by contract rather than ad‑hoc sanitization and is reasonable for this library layer.libs/ui/src/components/button.ts (3)
121-155: HTMX and data- attribute handling looks solid*The combination of
sanitizeDataKey,escapeHtmlfor HTMX values, and escapeddata-*values gives good defense against malformed attributes and injection viahtmx/dataoptions, while still allowing flexible usage. This is consistent with the shared validation/escaping approach elsewhere in the UI library.Also applies to: 278-287
209-260: Button builder logic, validation, and accessibility handling look well-structuredThe main
buttonfunction cleanly separates concerns: schema-based options validation, accessibility warnings, href protocol checks, class computation, HTMX/data attribute construction, and content assembly (including icon/loader behavior). The use ofeffectiveAriaLabelplus icon-only warnings gives good accessibility coverage, and invalid options correctly fall back to the shared validation error box.Also applies to: 295-307
329-360: Button group and shorthand helpers are consistent and ergonomicThe early guard for empty
buttons, schema-backed group options, and the attached vs. non-attached layouts look good. The variant shorthand functions (primaryButton, etc.) are thin, type-safe wrappers overbutton, avoiding duplication of behavior while making the API more ergonomic.Also applies to: 366-388
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
libs/ui/src/components/button.ts (1)
1-74: Documentation in docs/draft/docs/ is still pending.**Per coding guidelines, public API changes in publishable libraries require matching documentation updates in
docs/draft/docs/**. This was flagged in a previous review and remains unaddressed.libs/ui/src/theme/theme.ts (1)
641-672: customCss trust model is now clearly documented.The detailed remarks around
theme.customCssand trusted-only usage nicely address the earlier XSS concern about injecting raw CSS into a<style>block. Just ensure the same trust assumptions are echoed in external docs so consumers don’t mistakenly wire user input into this field.
🧹 Nitpick comments (3)
libs/ui/src/components/button.ts (1)
370-396: Consider addingsuccessButtonconvenience function for API consistency.The
getVariantClassesfunction (line 90) defines asuccessvariant, but there's no correspondingsuccessButtonshorthand like the other variants./** Link button shorthand */ export const linkButton = (text: string, opts?: Omit<ButtonOptions, 'variant'>) => button(text, { ...opts, variant: 'link' }); + +/** Success button shorthand */ +export const successButton = (text: string, opts?: Omit<ButtonOptions, 'variant'>) => + button(text, { ...opts, variant: 'success' });libs/ui/src/theme/theme.ts (2)
420-528: Dark variant merge semantics andbaseColorsfallback may be surprising for partial configs.Because
mergeThemesCorefalls back to{ semantic: { primary: '#24292f' } }whenbase.colorsis missing, any partialbase.darkthat omitscolorswill not inherit from the light theme’s colors but instead pick up this hardcoded default. Also, whenoverride.darkis undefined, global overrides (e.g., typography, colors, spacing) are applied only to the light theme; the existingbase.darkis preserved as-is, so dark mode can drift from the primary theme.If
darkis intended as a partial overlay on top of the main theme, consider:
- Seeding the dark merge with the light theme’s colors when
darkBase.colorsis absent, rather than a hardcoded fallback.- Optionally propagating non-dark overrides into the dark variant as well (e.g., merge
{ ...base, ...base.dark }with an override-without-.dark), so global tweaks apply consistently to both modes unless explicitly overridden indark.This is more about predictable behavior than correctness, but clarifying/adjusting now will avoid surprising results for consumers defining partial dark themes.
549-639: Some theme fields (spacing, font sizes/weights) are not emitted as CSS variables.
buildThemeCssemits semantic/surface/text/border colors, font families, radius, andcustomVars, but it doesn’t currently emitThemeSpacingor typography sizes/weights/line-heights as CSS variables. If components or layouts rely on those via CSS/Tailwind, you may want to extend this emitter (or explicitly document that those fields are used only at the config/TS level, not as CSS variables).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libs/ui/src/components/button.ts(1 hunks)libs/ui/src/theme/theme.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use strict TypeScript mode with noanytypes without strong justification; preferunknownfor generic type defaults
Avoid non-null assertions (!) - use proper error handling and throw specific error classes instead
Files:
libs/ui/src/components/button.tslibs/ui/src/theme/theme.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/ui/src/components/button.tslibs/ui/src/theme/theme.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Create shared base classes for common functionality (e.g., ExecutionContextBase) to reduce code duplication
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/ast-guard/SECURITY-AUDIT.md : Document all blocked attack vectors and security considerations in SECURITY-AUDIT.md
Applied to files:
libs/ui/src/components/button.tslibs/ui/src/theme/theme.ts
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/src/**/*.{ts,tsx} : Centralize record types in common/records directory and import from there rather than module-specific files
Applied to files:
libs/ui/src/theme/theme.ts
🧬 Code graph analysis (1)
libs/ui/src/theme/theme.ts (3)
libs/ui/src/theme/index.ts (27)
ColorScale(57-57)SemanticColors(58-58)SurfaceColors(59-59)TextColors(60-60)BorderColors(61-61)ThemeColors(62-62)FontFamilies(64-64)FontSizes(65-65)FontWeights(66-66)ThemeTypography(67-67)ThemeSpacing(69-69)ThemeRadius(70-70)ThemeShadows(71-71)ButtonTokens(73-73)CardTokens(74-74)InputTokens(75-75)ComponentTokens(76-76)CdnScriptResource(78-78)ThemeCdnFonts(79-79)ThemeCdnIcons(80-80)ThemeCdnScripts(81-81)ThemeCdnConfig(82-82)ThemeConfig(84-84)GITHUB_OPENAI_THEME(93-93)DEFAULT_THEME(93-93)mergeThemes(86-86)createTheme(87-87)libs/ui/src/theme/presets/github-openai.ts (2)
GITHUB_OPENAI_THEME(47-190)DEFAULT_THEME(195-195)libs/ui/src/theme/presets/index.ts (2)
GITHUB_OPENAI_THEME(12-12)DEFAULT_THEME(12-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
libs/ui/src/components/button.ts (1)
181-325: Well-implemented security and accessibility measures.The button function properly addresses key concerns:
classNameis escaped to prevent attribute injection (line 264)hrefuses an allowlist for safe protocols (lines 168-179)rel="noopener noreferrer"is added fortarget="_blank"(line 298)- Icon-only buttons get automatic
aria-labelfrom text (lines 300-303)- XSS risk for
iconBefore/iconAfteris documented in JSDoc (lines 189-192)libs/ui/src/theme/theme.ts (1)
45-396: Theme type surface looks coherent and strict‑mode friendly.The exported theme/CDN/component token types are well-structured, avoid
any, and should be stable as a public SDK surface. No issues from a typing/API-shape perspective.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.