Conversation
There was a problem hiding this comment.
Code Review
This pull request proposes removing the CODEOWNERS file to reduce notification noise. While the goal is understandable, completely removing this file could negatively impact the review process by making it harder to get the right experts to review changes. I've left a specific comment suggesting refining the CODEOWNERS rules as an alternative. Additionally, please note that the pull request description is missing the Pre-Review Checklist, which is required by the repository's contribution guidelines.
I am having trouble creating individual review comments. Click here to see my feedback.
.github/CODEOWNERS (1-27)
Removing the CODEOWNERS file entirely is a significant change to the repository's review process. While it may reduce notification noise, it also removes the safeguard that ensures relevant experts are looped into changes affecting their areas of ownership. This could lead to lower quality reviews or slower review times as authors manually search for appropriate reviewers.
Have you considered refining the CODEOWNERS rules instead of removing the file? For example, the wildcard rule on line 15 (* @dmandar ...) is very broad and could be the primary source of noise. Perhaps removing or narrowing that rule, while keeping the more specific directory-based rules, would be a better compromise to achieve the goal of reducing noise without losing the benefits of code ownership.
The rules were already removed, but having CODEOWNERS spams all owners for every PR, and with the rules turned off already, this is no longer valuable. |
* Add React renderer implementation * Improve React renderer with visual parity and tests - Refactor components to mirror Lit Shadow DOM structure for visual parity - Add CSS fixes for specificity and selector transformations - Add unit tests for all components (220 tests) - Add visual parity test infrastructure with Playwright - Update documentation with build and test instructions * Enable checkboxUnchecked visual parity test The Lit renderer bug has been fixed, so this fixture no longer needs to be skipped. * Update Icon component description to Material Symbols The Icon component uses Material Symbols Outlined font, not Lucide. * Update PARITY.md: add CheckBox and DateTimeInput to implemented components * Add unit tests for Tabs and Modal components - Add comprehensive Tabs tests (21 tests): rendering, tab selection, switching behavior, nested content, edge cases, accessibility - Add comprehensive Modal tests (21 tests): opening/closing, backdrop click, Escape key, nested content, portal rendering, accessibility - Theme tests document that empty theme classes are intentional (styling from structural CSS) * Add ThemeContext tests to verify theme switching - Add tests verifying components read theme from context, not hardcoded - Test custom theme classes applied to: Button, Card, Text, Column, Row, TextField, CheckBox, Tabs, Divider, Icon, Slider, MultipleChoice - Test theme isolation between different provider instances - Catches bugs where components import litTheme directly vs useTheme() * Fix path binding reactivity and add communication tests - useA2UIComponent now subscribes to state version via useA2UIState() ensuring components with path bindings re-render when data changes - Add comprehensive server-client communication tests (28 tests) covering message processing, multiple surfaces, dataModelUpdate, deleteSurface, path bindings, action dispatch, and error handling - Add createDataModelUpdate and createDeleteSurface test helpers - Update README with path binding reactivity documentation * Refactor test structure into unit and integration directories - Move component tests to tests/unit/components/ - Split communication.test.tsx into focused integration tests: - messages.test.tsx: basic message processing - data-binding.test.tsx: data model and path bindings - actions.test.tsx: action dispatch - components.test.tsx: component updates, nesting, errors - hooks.test.tsx: context hooks behavior - templates.test.tsx: template expansion tests - Move ThemeContext.test.tsx to tests/integration/ - Extract shared utilities to tests/utils/: - render.tsx: TestWrapper, TestRenderer - messages.ts: message factory functions - assertions.ts: type-safe getMockCallArg, getElement helpers - Update tsconfig.json to include tests directory and add types * Add ESLint and Prettier configuration - Add eslint.config.js with TypeScript, React, and React Hooks rules - Add .prettierrc for consistent code formatting - Add lint, lint:fix, format, and format:check scripts - Remove duplicate clsx and markdown-it from devDependencies - Add sideEffects field for proper tree-shaking - Add engines field requiring Node.js >= 18 * Update dependencies and clean up tsconfig - Update vitest to 4.x and jsdom to 28.x - Remove unused tsconfig options (declaration, declarationMap, sourceMap) - Update visual-parity dependencies: - vite 5.x → 7.x - @vitejs/plugin-react 4.x → 5.x - pixelmatch 5.x → 7.x - concurrently 8.x → 9.x - @playwright/test 1.57 → 1.58 * Fix ESLint errors and warnings - Fix ComponentNode: move useMemo before conditional return (Rules of Hooks) - Fix TextField: prefix unused isValid state with underscore - Auto-fix type imports in A2UIProvider and A2UIRenderer - Remove unused eslint-disable directive in Video * Add integration tests for message processing and surfaceUpdate behaviors * Add integration tests for server-driven updates across all components - Add property-updates.test.tsx: tests surfaceUpdate message handling for property changes (Text, Image, Icon, Button, TextField, CheckBox, Slider, Column, Row, List, Tabs) - Extend data-binding.test.tsx: tests dataModelUpdate message handling for all components with path bindings (Text, TextField, CheckBox, Slider, DateTimeInput, MultipleChoice, Image, Icon, Video, AudioPlayer) - Fix CheckBox and Slider to respond to server-driven literal value updates (literalBoolean/literalNumber), not just path bindings * Add React sample shell application for restaurant finder demo Mirrors the Lit shell demo with identical A2UI messages and theme. Includes mock restaurant data, demo configuration, and styling. * Fix React renderer style transformation to match Lit renderer - Transform primaryColor to --p-* color palette using color-mix - Transform font to --font-family and --font-family-flex variables - Add type assertion in ComponentNode to fix TypeScript build error - Add React deduplication in visual-parity vite config to fix hook errors * Remove Google copyright headers from React sample shell files * Fix additionalStyles applied to wrong element in DateTimeInput and Slider Move theme.additionalStyles from <section> container to <input> element to match Lit renderer behavior. TextField already did this correctly. * Use :where() for all component element selectors to match Lit specificity Wrap component-scoped element selectors (input, label, textarea, dialog, video, audio, img) in :where() so utility classes can override them, matching how bare element selectors work in Lit's Shadow DOM. * Initialize dark mode state from OS color scheme preference Read prefers-color-scheme media query on mount so the toggle button icon and hero image match the actual theme from first render. * Update PARITY.md to reflect :where() usage on all element selectors * Add test for action context resolving path bindings from data model * Add test for action context with mixed literal and path parameters * Fix Modal visual parity and add fixtures for Modal, Video, AudioPlayer Modal changes: - Render dialog in place (no portal) to stay inside .a2ui-surface - Match Lit's structure: closed shows section with entry, open shows dialog - Apply backdrop theme class to dialog, element class to inner section - Use #controls div with g-icon close button to match Lit - Add close event listener for native Escape key handling Visual parity fixtures: - Add modalBasic and modalWithCard fixtures - Add videoBasic and videoWithPathBinding fixtures - Add audioPlayerBasic and audioPlayerWithPathBinding fixtures - Document that Lit AudioPlayer does not implement description property Update PARITY.md to reflect implemented status for Modal, Video, AudioPlayer * Simplify MultipleChoice to select dropdown matching Lit renderer Replace radio/checkbox rendering with a <select> dropdown for visual parity. Update tests, fixtures, and documentation accordingly. * Exclude workspace packages from Vite optimizeDeps to fix stale cache * Add browser reset, style docs, and skip unknown components - Add @layer reset to restore browser defaults inside .a2ui-surface when host apps use CSS resets (e.g. Tailwind preflight) - Add style architecture README documenting the 7-layer priority system - Skip unknown components silently (return null) to match Lit behavior - Add markdown-it to Vite optimizeDeps exclude list * Use markdown-it renderer rules API for theme class injection in Text Replace regex-based HTML string patching in applyMarkdownTheme with markdown-it's renderer.rules API. Sets token.attrJoin() on _open tokens before render, then cleans up rules after — same approach as the Lit renderer's markdown directive. * Move CSS comments out of injected style output into JSDoc * fix(visual-parity): exclude transitive deps from Vite optimizeDeps in both renderers Exclude markdown-it, clsx, and signal-utils/* from pre-bundling in both the React and Lit dev servers to avoid stale cache / duplicate module instances. Also update cache-clearing instructions in PARITY.md. * Remove deprecated useA2UIStore and useA2UIStoreSelector * Simplify hash algorithm in A2UIViewer * fix(Text): make HintedStyles keys optional to match type guard semantics * fix(react): update test fixtures to match web_core Zod schema changes After merging main, web_core now validates messages with Zod schemas that enforce stricter requirements: - Text and Image components require usageHint (was optional) - dataModelUpdate contents use typed value keys (valueString, valueNumber, valueBoolean) instead of generic value - surfaceUpdate.components requires min 1 element Updated all test fixtures across 17 files to use valid protocol payloads. Updated empty surfaceUpdate test to verify both schema rejection and surface persistence after error. Converted nested valueMap template test to use JSON-encoded array format (the Zod schema does not support recursive valueMap in ValueMapItemSchema). * feat(react): add altText support to Image component Resolve the optional altText property from the schema and pass it to the img element, replacing the previously hardcoded empty string. * chore: add React renderer to CODEOWNERS Route renderers/react/ changes to @lukasmoschitz for review. * fix(react): use published @a2ui/lit package instead of local file path Replace "file:../lit" with "^0.8.1" so the package is publishable to NPM. * refactor(react): replace @a2ui/lit dependency with @a2ui/web_core Remove the heavyweight Lit renderer dependency in favor of the lightweight web_core package. This eliminates Lit, lit-html, and Lit signals from the React renderer's dependency tree, making it suitable for embedding in other packages without bundle bloat. - Replace all @a2ui/lit/0.8 imports with @a2ui/web_core subpath imports - Use plain A2uiMessageProcessor instead of Lit signal-based variant - Replace manual pixelmatch.d.ts with @types/pixelmatch package - Update empty surfaceUpdate test to match web_core@0.8.0 behavior * fix(visual-parity): inject markdown renderer for Lit test harness The Lit renderer no longer includes a default markdown renderer. Provide renderMarkdown from @a2ui/markdown-it via Lit context so Text widget parity tests continue to work. * replaced hero image with new version to keep parity to lit * fix(react): match Lit Icon component font-size (24px) web_core structural styles define .g-icon at 20px, but the Lit Icon component overrides this to 24px in its scoped styles. Add the same override in componentSpecificStyles to maintain visual parity. * feat(react): auto-initialize styles and catalog in A2UIProvider Move ensureInitialized() into A2UIProvider so users no longer need to manually call initializeDefaultCatalog() and injectStyles() before rendering. Both are idempotent so multiple providers are safe. * chore: update sample app package-lock after web_core migration * fix(visual-parity): force Vite dep re-optimization on startup Prevents stale 504 errors when file: dependencies are rebuilt by setting optimizeDeps.force in both vite configs. * Format main markdown file * foramt visual parity markdown file * Add GH workflow for the React renderer * add Google OSS copyright notices to all relevant files * Update formatter to match the Google TS style guide * Clarify gts import with a better comment * fix(lint): fix ESLint config and resolve lint errors - Replace require('gts') with ESM import in eslint.config.js - Disable prefer-arrow-callback to preserve React.memo display names - Exclude .d.ts files from linting - Fix floating promise in A2UIProvider dispatch * chore: remove CODEOWNERS, deleted upstream in #803 * chore: add Google OSS copyright notices to sample app --------- Co-authored-by: alan blount <alan@zeroasterisk.com> Co-authored-by: Ava Mattie <6314286+ava-cassiopeia@users.noreply.github.com>
Description
Removing the CODEOWNERS file because we've decided it's just too much noise for reviewers.