Skip to content

fix: update mcp-app height and improve documentation with source link…#997

Merged
sugoi-yuzuru merged 1 commit intomainfrom
a2ui_and_mcp_apps
Mar 26, 2026
Merged

fix: update mcp-app height and improve documentation with source link…#997
sugoi-yuzuru merged 1 commit intomainfrom
a2ui_and_mcp_apps

Conversation

@sugoi-yuzuru
Copy link
Copy Markdown
Collaborator

@sugoi-yuzuru sugoi-yuzuru commented Mar 26, 2026

…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.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

medium

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)

medium

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)

medium

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
  1. If there are code changes, code should have tests. (link)

@sugoi-yuzuru sugoi-yuzuru merged commit f9e9554 into main Mar 26, 2026
11 checks passed
@github-project-automation github-project-automation bot moved this from Todo to Done in A2UI Mar 26, 2026
@sugoi-yuzuru sugoi-yuzuru deleted the a2ui_and_mcp_apps branch March 26, 2026 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants