fix: improve sprite viewer mouse controls#215
Conversation
- Disable right-click drag to fix context menu conflicts - Add vertical drag support (up = top view, down = bottom view) - Fix rotation logic when dragging from vertical views - Maintain atomic drag gestures (either horizontal OR vertical) Each drag now properly handles transitions between horizontal and vertical views without breaking the rotation math. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Summary of ChangesHello @devonjones, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the user experience of the sprite viewer by making its mouse controls more robust and intuitive. It addresses several issues related to drag interactions, ensuring that users can seamlessly navigate between different sprite perspectives, including horizontal rotations and dedicated top/bottom views, without unexpected behavior or conflicts. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively improves the sprite viewer's mouse controls by adding vertical drag functionality and resolving a conflict with the right-click context menu. The implementation is logical, particularly the handling of rotation when transitioning from vertical to horizontal views. My main feedback is the lack of tests for this new functionality, which is critical for ensuring its correctness and preventing regressions. I've also provided a suggestion to improve the consistency of the drag detection logic. Overall, these are great enhancements that will improve the user experience once testing is added.
Added download functionality for sprite sheets to address the loss of right-click "save image" capability. Since sprite tiles are rendered as CSS background-images, browser context menus don't work normally. Implementation: - Added download button positioned in bottom-right of sprite viewer - Added custom right-click context menu with "Download sprite sheet" option - Created backend proxy endpoint `/api/images/proxy` to bypass CORS - Proxy validates URLs are from FILE_DOMAIN for security - Downloads full sprite sheet (extracting single tiles blocked by CORS) Technical details: - CORS prevents canvas manipulation of cross-origin images - CORS prevents download attribute from working on cross-origin links - Backend proxy fetches image server-side and adds Content-Disposition header - Uses existing `requests` library already in requirements.txt 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…comprehensive tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request significantly improves the sprite viewer's mouse controls and adds a much-needed sprite sheet download feature. The backend changes introduce a new proxy endpoint to handle downloads, but I've identified a critical security vulnerability (SSRF) in its implementation that needs to be addressed. On the frontend, the new drag logic is well-implemented, but the custom context menu uses direct DOM manipulation, which is not idiomatic for React and could lead to bugs and maintenance issues. I've provided detailed feedback and suggestions to resolve these issues.
- CRITICAL: Fixed SSRF vulnerability in proxy_image by validating FILE_DOMAIN is configured - Added query parameter stripping for clean filenames - Fixed download button title to match actual functionality - Added context menu cleanup on component unmount to prevent memory leak 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances the sprite viewer's mouse controls and adds a valuable sprite sheet download feature. The backend implementation includes a new proxy endpoint to handle downloads, which is a good approach to bypass CORS issues. However, I've identified a significant security vulnerability (SSRF) in the proxy's URL validation logic, as well as a less severe path traversal risk. On the frontend, the new mouse controls and associated tests are well-implemented. The custom context menu for downloading is functional, but its implementation relies on direct DOM manipulation, which is an anti-pattern in React; I've suggested refactoring it to a more idiomatic, state-driven approach for better maintainability. My review includes specific suggestions to address these points.
- Use urlparse to compare netloc instead of startswith check - Prevents SSRF bypass via subdomain attacks - Add secure_filename sanitization to prevent path traversal - Import urllib.parse for robust URL validation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces significant improvements to the sprite viewer's mouse controls and adds a new download functionality. The backend proxy for downloads is well-implemented, though I've suggested a minor improvement for filename extraction to make it more robust. The new mouse control logic and corresponding tests are comprehensive. However, on the frontend, I've identified a couple of areas for improvement. The new context menu is implemented using direct DOM manipulation, which is a React anti-pattern; I've recommended refactoring this into a declarative React component for better maintainability. Additionally, the logic for differentiating vertical and horizontal views relies on an implicit array ordering, which I've suggested making more explicit and robust. Overall, great work on enhancing the user experience of the sprite viewer.
- Add fallback for URLs without filenames (empty path case) - Use explicit topIndex/bottomIndex comparison instead of >= check - Makes angle detection independent of array ordering 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 participant
Summary
Mouse Control Changes
Download Functionality
/api/images/proxyto bypass CORS restrictionsTechnical Details
Testing
🤖 Generated with Claude Code