fix: update mcp-app height and improve documentation with source link…#997
fix: update mcp-app height and improve documentation with source link…#997sugoi-yuzuru merged 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request enhances the MCP Apps documentation by adding direct links to relevant source files and samples, while also updating the Mermaid diagram syntax. Additionally, it modifies the Angular mcp-app component's CSS to use a fixed height. Reviewers suggested restoring context to the diagram labels, implementing dynamic resizing for the component instead of a hardcoded height, and adding tests for the component changes to comply with the repository's style guide.
I am having trouble creating individual review comments. Click here to see my feedback.
docs/guides/mcp-apps-in-a2ui-surface.md (32-40)
The updated subgraph titles are simpler, but they've lost some useful context that was present before (e.g., (A2UI), (Same-Origin), (Cross-Origin/Isolated)). This information is valuable for understanding the architecture at a glance from the diagram. Consider re-adding this context to the titles while keeping the valid quoted syntax.
subgraph "Host Application (A2UI)"
A[A2UI Page] --> B["Host Component e.g., McpApp"]
end
subgraph "Sandbox Proxy (Same-Origin)"
B -->|Message Relay| C[iframe sandbox.html]
end
subgraph "Embedded App (Cross-Origin/Isolated)"
C -->|Dynamic Injection| D[inner iframe untrusted content]
end
samples/client/angular/projects/mcp_calculator/src/a2ui-catalog/mcp-app.ts (51-52)
While replacing this with a fixed height of 500px is a valid fix, it misses an opportunity to implement dynamic resizing as suggested by the TODO on line 189. Implementing the onsizechange handler would allow the component to adapt to its content, providing a better user experience and avoiding hardcoded heights.
samples/client/angular/projects/mcp_calculator/src/a2ui-catalog/mcp-app.ts (51)
Per the repository style guide, code changes should be accompanied by tests. Since you've modified the component's styling and behavior, please add corresponding tests to ensure the change is working as expected and prevent future regressions.
References
- If there are code changes, code should have tests. (link)
…s and diagrams
Description
Fix graph and links on guide
Pre-launch Checklist
If you need help, consider asking for advice on the discussion board.