Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the A2UI Composer shell by modularizing several core features, including extracting payload parsing and syntax healing into a2ui-payload-parser, error formatting into ChatErrorFormatterService, file ingestion into FileIngestionService, and sharing capabilities into ShareService. It also introduces a base AbstractSelector for settings selectors, delegates startup state to StartupConfigStateService, and adds services for safe URL validation and screenshot capture. Feedback on these changes highlights several critical regressions and improvements: same-window messages should be allowed in preview-bridge.ts to avoid runtime regressions; the single JSON object fallback in a2ui-payload-parser.ts must be removed to comply with repository rules, while a nullish input guard should be added and the missing rules/mock* property filtering restored; the earlyMessageBuffer cleanup in host-communication.ts should be preserved to prevent memory leaks; and the gallery component should use @angular/cdk/clipboard for robust clipboard operations.
⚡ A2UI Composer PR PreviewYour automated preview is successfully live (commit
|
suyangw-g
left a comment
There was a problem hiding this comment.
Summary & Stack Bookkeeping
Stacked on #169. Note the clipboard API change + telemetry implications and the intended gallery-copy tracking semantics. Inherited blockers from #166-#169 (sanitize filter, cleanPayload, prompt content, empty-payload guard, iframe unregistration) remain open on this branch — fixing them downstack and rebasing is cleaner than patching at the top.
Key Points
- Share Service: Don't downgrade the share path to synchronous CDK copy; restore async
navigator.clipboardwith CDK fallback for KB-scale URLs. - Gallery Copy Telemetry: Restore success-conditioned copy telemetry rather than firing unconditionally on click via the directive.
- Change Detection: Restore
ChangeDetectionStrategy.OnPushonGalleryandOriginConfirmationDialog. - Preview Teardown (#169 fix): Restore
registerIframe(ref?.nativeElement || null)so detached iframes are unregistered. - Test Coverage: Backfill share tests lost during extraction, delegation tests, and directive error/param handling.
- Cleanups: Minor error message fix in
ShareServiceand documentation forTrackEventDirectivepre-click param evaluation.
98c928d to
0eddfbc
Compare
…ngestion - Implement TrackEventDirective applying declarative tracking boundaries natively. - Construct FileIngestionService strictly leveraging @angular/cdk/clipboard utilities. - Construct ShareService cleanly encapsulating clipboard interactions.
- Sweep all active standalone UI components throughout the app, converting default ChangeDetection behavior to ChangeDetectionStrategy.OnPush. - Strip FileReader native accesses from ChatPanel into the FileIngestionService. - Swap template tracking to the directive.
- Rip out the legacy shareDesign() bloated component implementation comprehensively. - Delete deprecated inner test asserts and old inline code.
- Clear earlyMessageBuffer when unregistering iframe in HostCommunication. - Inject CDK Clipboard service in Gallery for copying A2UI payloads. - Update unit test suites to verify buffer clearing and CDK Clipboard mocking.
- Track gallery copy usage conditionally on successful clipboard copy - Backfill comprehensive test suites for ShareService and TrackEventDirective - Add composer shell share button delegation test
- Replace Angular CDK Clipboard with navigator.clipboard.writeText - Update ShareService specs to mock native clipboard writeText
- Restore 'Shareable link copied to clipboard' format with KB size - Update unit test assertions in share service spec
Description
Fifth and final in a series of refactorings to clean up the A2UI Composer code.
UI Shell Encapsulation, CDK Share Service & Telemetry Directives
NOTE No e2e tests had to be changed.
Pre-launch Checklist