feat: implement responsive mobile board interface with orientation fl…#710
Open
raymondidahor-bot wants to merge 1 commit into
Open
feat: implement responsive mobile board interface with orientation fl…#710raymondidahor-bot wants to merge 1 commit into
raymondidahor-bot wants to merge 1 commit into
Conversation
…ip and touch optimizations
Contributor
📝 WalkthroughWalkthroughAdds board orientation state management to enable chessboard flipping, implementing coordinate translation logic in ChessboardComponent to remap displayed positions when flipped, updates responsive sizing with ResizeObserver, and introduces unit tests validating orientation behavior with algebraic notation assertions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
|
@raymondidahor-bot Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (5)
frontend/app/play/[slug]/page.tsx (1)
297-305: Consider extracting duplicated flip button logic.The flip button onClick handler and label logic is duplicated between mobile (lines 297-305) and desktop (lines 413-421) controls. Consider extracting to a shared handler and label variable.
♻️ Proposed refactor
+ const handleFlipBoard = useCallback(() => { + setBoardOrientation((prev) => (prev === "white" ? "black" : "white")); + }, []); + + const flipButtonLabel = `Flip to ${boardOrientation === "white" ? "Black" : "White"}`;Then use
onClick={handleFlipBoard}and{flipButtonLabel}in both button instances.Also applies to: 413-421
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/play/`[slug]/page.tsx around lines 297 - 305, Extract the duplicated flip logic by creating a single handler and label used by both buttons: implement a handleFlipBoard function that calls setBoardOrientation(prev => prev === "white" ? "black" : "white") and derive a flipButtonLabel constant from boardOrientation (e.g., "Flip to Black" vs "Flip to White"); then replace the inline onClick and text in both mobile and desktop button instances with onClick={handleFlipBoard} and the {flipButtonLabel} variable so both controls share the same behavior and label.frontend/components/chess/ChessboardComponent.tsx (2)
101-122: Consider debouncing resize updates.The
updateBoardSizecallback is invoked on everyResizeObserverentry and window resize event. On rapid resizing, this could cause frequent re-renders. The threshold check on line 118 helps but a debounce would be more robust.♻️ Optional debounce approach
// At the top of the component, add a ref for debounce const resizeTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); const updateBoardSize = useCallback( (parentWidth?: number) => { if (resizeTimeoutRef.current) { clearTimeout(resizeTimeoutRef.current); } resizeTimeoutRef.current = setTimeout(() => { // existing logic... }, 16); // ~1 frame }, [width], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/chess/ChessboardComponent.tsx` around lines 101 - 122, The updateBoardSize callback is firing too often on ResizeObserver/window resize; add a debounce using a ref (e.g., resizeTimeoutRef) to coalesce rapid calls: create const resizeTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null), have updateBoardSize clear any existing timeout and set a new one (small delay like 16–100ms) that runs the existing sizing logic (using boardRef and setBoardWidth), and ensure you clear the timeout in a cleanup (useEffect return) to avoid leaks on unmount; keep the existing threshold check to avoid unnecessary state updates.
364-378: Verify touch end behavior when dragging off the board.If a user drags their finger off the board and releases,
getSquareFromTouchreturnsnull(line 336-337), but thetouchStartSquareref is still cleared (line 373). This is correct behavior, but the move is silently dropped. Consider providing visual feedback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/chess/ChessboardComponent.tsx` around lines 364 - 378, handleTouchEnd currently clears touchStartSquare and selected/hovered state when the finger is released off-board (getSquareFromTouch returns null), which drops the move silently; update handleTouchEnd so when sq is null it provides visual feedback instead of just clearing state: locate handleTouchEnd, getSquareFromTouch, touchStartSquare, setHoveredSquare, setSelectedSquare and attemptMove and, in the branch where sq is null, set the hovered/selected square back to the original touchStartSquare (or call an existing UI feedback helper like showInvalidMove) and trigger a short timeout/animation to indicate the move was cancelled before clearing touchStartSquare and selected/hovered state, leaving the normal attemptMove path unchanged when sq is present.frontend/components/chess/__tests__/ChessboardComponent.test.tsx (1)
5-10: Mock passes limited props to img element.The mock only forwards the
altprop. If future tests need to assert on imagesrcor other attributes, the mock will need to be expanded.♻️ More complete mock for future-proofing
vi.mock("next/image", () => ({ __esModule: true, - default: (props: { alt: string }) => React.createElement("img", { - alt: props.alt, - }), + default: (props: { alt: string; src?: string }) => + React.createElement("img", { + alt: props.alt, + src: typeof props.src === "string" ? props.src : "", + }), }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/chess/__tests__/ChessboardComponent.test.tsx` around lines 5 - 10, The mock for next/image currently only forwards the alt prop to the img element, which is limiting; update the mocked default export (the function returned in the vi.mock call) to forward all incoming props (e.g., spread props or explicitly include src, width, height, layout, etc.) when creating the "img" element so tests can assert on src and other attributes; keep __esModule: true and replace the body of default: (props: { alt: string }) => React.createElement("img", { alt: props.alt }) with a version that forwards all props (e.g., React.createElement("img", { ...props })) so future tests are supported.frontend/vitest.config.ts (1)
5-9: Remove redundantjsxFactoryandjsxFragmentwithjsx: "automatic".When
jsxis set to"automatic", esbuild uses the new JSX transform (React 17+) and ignoresjsxFactoryandjsxFragmentsettings. These options only apply whenjsxis set to"transform".♻️ Proposed cleanup
esbuild: { jsx: "automatic", - jsxFactory: "React.createElement", - jsxFragment: "React.Fragment", },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/vitest.config.ts` around lines 5 - 9, The esbuild config is redundant: when esbuild.esbuild.jsx is set to "automatic" the jsxFactory and jsxFragment options are ignored; remove the jsxFactory and jsxFragment entries from the esbuild config (the object containing jsx: "automatic") so only jsx remains, keeping jsx: "automatic" and any other relevant esbuild settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/app/play/`[slug]/page.tsx:
- Around line 297-305: Extract the duplicated flip logic by creating a single
handler and label used by both buttons: implement a handleFlipBoard function
that calls setBoardOrientation(prev => prev === "white" ? "black" : "white") and
derive a flipButtonLabel constant from boardOrientation (e.g., "Flip to Black"
vs "Flip to White"); then replace the inline onClick and text in both mobile and
desktop button instances with onClick={handleFlipBoard} and the
{flipButtonLabel} variable so both controls share the same behavior and label.
In `@frontend/components/chess/__tests__/ChessboardComponent.test.tsx`:
- Around line 5-10: The mock for next/image currently only forwards the alt prop
to the img element, which is limiting; update the mocked default export (the
function returned in the vi.mock call) to forward all incoming props (e.g.,
spread props or explicitly include src, width, height, layout, etc.) when
creating the "img" element so tests can assert on src and other attributes; keep
__esModule: true and replace the body of default: (props: { alt: string }) =>
React.createElement("img", { alt: props.alt }) with a version that forwards all
props (e.g., React.createElement("img", { ...props })) so future tests are
supported.
In `@frontend/components/chess/ChessboardComponent.tsx`:
- Around line 101-122: The updateBoardSize callback is firing too often on
ResizeObserver/window resize; add a debounce using a ref (e.g.,
resizeTimeoutRef) to coalesce rapid calls: create const resizeTimeoutRef =
useRef<ReturnType<typeof setTimeout> | null>(null), have updateBoardSize clear
any existing timeout and set a new one (small delay like 16–100ms) that runs the
existing sizing logic (using boardRef and setBoardWidth), and ensure you clear
the timeout in a cleanup (useEffect return) to avoid leaks on unmount; keep the
existing threshold check to avoid unnecessary state updates.
- Around line 364-378: handleTouchEnd currently clears touchStartSquare and
selected/hovered state when the finger is released off-board (getSquareFromTouch
returns null), which drops the move silently; update handleTouchEnd so when sq
is null it provides visual feedback instead of just clearing state: locate
handleTouchEnd, getSquareFromTouch, touchStartSquare, setHoveredSquare,
setSelectedSquare and attemptMove and, in the branch where sq is null, set the
hovered/selected square back to the original touchStartSquare (or call an
existing UI feedback helper like showInvalidMove) and trigger a short
timeout/animation to indicate the move was cancelled before clearing
touchStartSquare and selected/hovered state, leaving the normal attemptMove path
unchanged when sq is present.
In `@frontend/vitest.config.ts`:
- Around line 5-9: The esbuild config is redundant: when esbuild.esbuild.jsx is
set to "automatic" the jsxFactory and jsxFragment options are ignored; remove
the jsxFactory and jsxFragment entries from the esbuild config (the object
containing jsx: "automatic") so only jsx remains, keeping jsx: "automatic" and
any other relevant esbuild settings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3a754381-b1e6-425f-86e0-49ec5d9b9214
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
frontend/app/play/[slug]/page.tsxfrontend/components/chess/ChessboardComponent.tsxfrontend/components/chess/__tests__/ChessboardComponent.test.tsxfrontend/vitest.config.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Close #590
Frontend: Responsive Mobile Board Interface
Description
This PR implements a responsive mobile board interface for the chess game, enhancing user experience on mobile devices with fluid interactions and efficient resource utilization.
Changes Made
ChessboardComponent Enhancements:
orientationpropPlay Page Updates:
Testing & Quality:
Performance Optimizations:
Testing
Acceptance Criteria Met
…ip and touch optimizations
Summary by CodeRabbit