Enhance CombatScreen and CombatControls for mobile responsiveness #168
Conversation
…nsiveness and layout adjustments
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the CombatScreen and CombatControls components for better mobile responsiveness by implementing significant layout restructuring and UI optimizations. The changes focus on improving usability on smaller screens through responsive sizing, better component positioning, and cleaner visual design.
Key Changes:
- Introduced PixiJS Layout system integration with proper responsive containers and positioning
- Significantly reduced button sizes, text sizes, and component dimensions for mobile devices
- Restructured CombatScreen layout using flexbox-style layout containers for better organization
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| CombatHUD.tsx | Reduced sizes for mobile elements, simplified visual decorations, and removed bilingual text displays |
| CombatControls.tsx | Implemented smaller button dimensions, consolidated text displays, and improved mobile detection |
| CombatScreen.tsx | Major layout restructuring with PixiJS Layout system, better component positioning and responsive design |
| @@ -1,6 +1,11 @@ | |||
| import { HitEffect, PlayerState } from "@/systems"; | |||
| import { CombatSystem } from "@/systems/CombatSystem"; | |||
| import { GameMode, PlayerArchetype, Position, TrigramStance } from "@/types"; | |||
| import "@pixi/layout"; | |||
There was a problem hiding this comment.
Missing proper PixiJS UI component imports. According to guidelines, you should ALWAYS use @pixi/ui components as base building blocks and ALWAYS implement @pixi/layout for responsive layouts. Add imports for @pixi/ui components like Button, Container, etc.
Copilot generated this review using guidance from copilot-instructions.md.
| @@ -20,6 +25,12 @@ import { CombatControls } from "./components/CombatControls"; | |||
| import { CombatHUD } from "./components/CombatHUD"; | |||
| import { CombatStatsPanel } from "./components/CombatStatsPanel"; | |||
|
|
|||
| // Register custom components for use as JSX tags in @pixi/react | |||
There was a problem hiding this comment.
The extend() call should include @pixi/ui components. According to guidelines, you should ALWAYS use @pixi/ui components as base building blocks. Consider extending Button, FancyButton, ScrollBox and other UI components.
Copilot generated this review using guidance from copilot-instructions.md.
| const isMobile = width < 400; | ||
| const buttonWidth = isMobile ? 70 : 90; | ||
| const buttonHeight = isMobile ? 30 : 40; | ||
| const isMobile = width < 300; // More specific mobile detection |
There was a problem hiding this comment.
Magic number 300 should be extracted to a named constant for consistency across components. CombatHUD uses 768 for mobile detection, creating inconsistency.
| const isMobile = width < 300; // More specific mobile detection | |
| const MOBILE_WIDTH_THRESHOLD = 300; // Threshold for mobile detection | |
| const isMobile = width < MOBILE_WIDTH_THRESHOLD; // More specific mobile detection |
Copilot uses AI. Check for mistakes.
| @@ -41,8 +41,8 @@ export const CombatHUD: React.FC<CombatHUDProps> = ({ | |||
| y = 0, | |||
| }) => { | |||
| const isMobile = width < 768; | |||
There was a problem hiding this comment.
Magic number 768 should be extracted to a named constant. Different components use different breakpoints (300 vs 768), which creates inconsistency in mobile detection logic.
| const isMobile = width < 768; | |
| const isMobile = width < MOBILE_BREAKPOINT; |
Copilot uses AI. Check for mistakes.
| left: Math.max(50, playerPositions[0].x - (width - 20) / 2), // Relative to arena container | ||
| top: Math.max(50, playerPositions[0].y - 120), // Account for HUD height |
There was a problem hiding this comment.
Complex positioning calculation with magic numbers (50, 20) should be extracted to helper functions or constants for better readability and maintainability.
| left: Math.max(50, playerPositions[0].x - (width - 20) / 2), // Relative to arena container | |
| top: Math.max(50, playerPositions[0].y - 120), // Account for HUD height | |
| left: Math.max(MIN_LEFT_POSITION, playerPositions[0].x - (width - ARENA_PADDING) / 2), // Relative to arena container | |
| top: Math.max(MIN_TOP_POSITION, playerPositions[0].y - HUD_HEIGHT), // Account for HUD height |
Copilot uses AI. Check for mistakes.
| > | ||
| <pixiGraphics | ||
| draw={(g) => { | ||
| const buttonWidth = isMobile ? 100 : 140; |
There was a problem hiding this comment.
Button width calculation is duplicated and uses magic numbers. This logic should be consistent with the responsive button sizing used in CombatControls component.
| const buttonWidth = isMobile ? 100 : 140; | |
| const buttonWidth = getButtonWidth(isMobile); |
Copilot uses AI. Check for mistakes.
|
Improve the layout and hit effects in CombatScreen and CombatControls to provide a better experience on mobile devices. Adjustments include smaller button sizes, compact positioning of elements, and refined graphics for improved visibility and usability.