feat: add raw HTML component support and unify sandbox iframe impleme…#985
feat: add raw HTML component support and unify sandbox iframe impleme…#985sugoi-yuzuru merged 3 commits intomainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new Pong game application, integrating it into the agent and client. The review highlights several issues: invalid HTML in pong_app.html due to duplicate closing tags, an inline style attribute that should be moved to CSS, a magic number in the CPU AI logic that needs to be a named constant, and the use of setInterval instead of requestAnimationFrame for the game loop. Additionally, a redundant import os statement was found in tools.py, and new tests are required for the get_pong_app tool as per the style guide.
| </div> | ||
| </div> | ||
|
|
||
| <div style="position: relative; width: 100%; display: flex; justify-content: center; flex: 1; min-height: 0;"> |
There was a problem hiding this comment.
This div uses an inline style attribute. To improve maintainability and separate structure from presentation, it's better to move these styles to a dedicated CSS class in the <style> block.
You can then add the following class to your CSS:
.canvas-container {
position: relative;
width: 100%;
display: flex;
justify-content: center;
flex: 1;
min-height: 0;
} <div class="canvas-container">|
|
||
| // CPU AI | ||
| let targetY = ball.y - cpu.height / 2; | ||
| cpu.y += (targetY - cpu.y) * 0.1; |
There was a problem hiding this comment.
The value 0.1 used for the CPU AI's movement speed is a "magic number". Using hardcoded values like this makes the code harder to understand and maintain. Please define it as a named constant at the top of the <script> block (e.g., const CPU_AI_SPEED = 0.1;) and use the constant here.
cpu.y += (targetY - cpu.y) * CPU_AI_SPEED;| const framePerSecond = 50; | ||
| setInterval(game, 1000 / framePerSecond); |
There was a problem hiding this comment.
The game loop uses setInterval, which is not ideal for animations. requestAnimationFrame is more efficient, provides smoother rendering by syncing with the browser's repaint cycle, and pauses automatically when the tab is not visible, saving resources.
I'd suggest refactoring the game loop to use it.
function gameLoop() {
game();
requestAnimationFrame(gameLoop);
}
requestAnimationFrame(gameLoop);| return f"Error connecting to MCP server: {e}" | ||
|
|
||
|
|
||
| async def get_pong_app(tool_context: ToolContext): |
There was a problem hiding this comment.
The repository style guide states that 'If there are code changes, code should have tests.' This pull request introduces a new get_pong_app tool and the corresponding Pong game, but no new tests have been added to verify this functionality. Please add tests for the new tool.
References
- The style guide requires that new code changes are accompanied by tests. (link)
|
|
||
| async def get_pong_app(tool_context: ToolContext): | ||
| """Fetches the Pong game app.""" | ||
| import os |
google#985) * feat: add raw HTML component support and unify sandbox iframe implementation * refactor: rename get_pong_app to get_pong_app_a2ui_json for clarity * chore: add license header to pong_app.html and clean up agent code formatting
…ntation
Description
This PR introduces a responsive "Neon Pong" game and implements the pattern for serving self-contained HTML applications via the ADK agent. It leverages the McpApp component with literalString content for robust, server-less application delivery.
Key Changes:
Added get_pong_app tool to mcp_app_proxy to serve HTML content.
Updated agent.py to register the new tool and defined the "Open Pong" skill for the agent.
Pre-launch Checklist
If you need help, consider asking for advice on the discussion board.