Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This pull request refactors the chess book generation system with a new greedy batching algorithm for optimal game-to-page distribution and adds new content pages (stats, notes, spine). The changes aim to improve page layout efficiency and prevent content overflow.
Key changes:
- Complete rewrite of the game-to-page conversion algorithm using greedy batching with move count validation
- Addition of stats page showing game/player counts and highlighting the best game
- Refactored six-game page layout with improved column pairing logic
- New notes pages and spine cover page for the book
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
utils/convertGamesToPages.ts |
Complete algorithm rewrite from cluster-based to greedy batching approach with validation functions for 3-game and 6-game page layouts |
types.ts |
Updated ChessPage type to support flexible game array sizes (1-6 games) for three and six page types |
components/Book/SixGamePage.tsx |
Refactored game pairing logic and move list display decisions based on combined move counts |
components/Book/index.tsx |
Added stats page, two notes pages, and spine cover page to the book structure |
styles/globals.css |
Changed chessboard background from CSS gradient to tiled PNG image |
components/Book/Page.tsx |
Minor className formatting improvements (cosmetic changes only) |
next.config.js |
Added devIndicators: false configuration and commented output export option |
public/pieces/background.png |
New background image file added for chessboard tiling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for (let j = i + 1; j < batch.length && !foundPairForThree; j++) { | ||
| const pair = [batch[i], batch[j]]; | ||
| if (validateGamesFor3Page(pair)) { | ||
| pages.push({ type: "three", games: pair as any }); |
There was a problem hiding this comment.
Multiple instances of as any type assertions are used throughout the function (lines 72, 84, 97, 117, 135). These bypass TypeScript's type checking system and can lead to runtime errors. Since the types in types.ts have been updated to support flexible game arrays, these assertions should be replaced with proper type guards or explicit tuple constructors to maintain type safety.
| pages.push({ type: "three", games: pair as any }); | |
| pages.push({ type: "three", games: pair as [Game, Game] }); |
|
|
||
| const newColumn: [number, number | undefined] = [i, undefined]; | ||
| const key = offset - i; | ||
| if (offset - 1 > 2 && sortedByMoveCountGames[key]) { |
There was a problem hiding this comment.
The condition checks if offset - 1 > 2 but this logic seems inverted or unclear. When offset (which is sortedByMoveCountGames.length - 1) minus 1 is greater than 2, it means we have at least 4 games. However, the comment says "if offset - 1 > 2" suggests checking if we have enough games for a second column. Consider clarifying this logic or adding a comment explaining the condition, as the relationship between offset and column pairing is not immediately obvious.
| if (offset - 1 > 2 && sortedByMoveCountGames[key]) { | |
| // Pair the i-th game with the (length - 1 - i)-th game if there are enough games for a pair in this column | |
| if (sortedByMoveCountGames.length > i * 2 + 1 && sortedByMoveCountGames[key]) { |
components/Book/index.tsx
Outdated
| <div | ||
| className="absolute inset-0 bg-cover bg-middle z-0" | ||
| style={{ | ||
| backgroundImage: `url(/covers/10.png?${cover}.jpg)`, |
There was a problem hiding this comment.
The background image URL has a syntax error. It's trying to concatenate /covers/10.png?${cover}.jpg, which will result in a malformed URL like /covers/10.png?1.jpg (assuming cover is a number). The query parameter syntax is incorrect, and mixing .png with .jpg extension is confusing. This should likely be either /covers/${cover}.jpg or /covers/10.jpg?v=${cover} for cache busting.
| backgroundImage: `url(/covers/10.png?${cover}.jpg)`, | |
| backgroundImage: `url(/covers/${cover}.jpg)`, |
1 participant
In short, an attempt at optimisations and improved logic with which games can go where to prevent text spilling the page or container.
AI Summary
This pull request introduces several significant improvements to the chess book generation logic and UI. The main changes include a major refactor of the game-to-page conversion algorithm for better page packing, the addition of new stat and notes pages, enhancements to the six-game page layout, and some UI adjustments for better presentation.
Game-to-Page Conversion Refactor:
convertGamesToPagesto use a new greedy batching algorithm, improving how games are grouped into pages. The new logic prioritizes the strongest game for a full page, then tries to create three-game and six-game pages while respecting move count constraints, resulting in more balanced and visually appealing layouts.ChessPagetype intypes.tsto support more flexible groupings for three- and six-game pages, accommodating the new batching logic.New Pages and Layout Enhancements:
Six-Game Page Improvements:
SixGamePageto better pair games into columns and display move lists more intelligently based on the number of moves in each game, improving readability and page balance. [1] [2] [3]UI and Styling Adjustments:
Page.tsxfor improved readability and maintainability. [1] [2]