feat: improve recruit dialog #12
Merged
Conversation
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
Now considering if the building has unlocked the knights
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Pull Request Overview
This PR improves the recruit dialog by enhancing the UI with visual representations of units, adding conditional logic to enable/disable options based on game state, and refactoring the Tile component to support fake tiles for display purposes.
Key changes include:
- Refactored Tile component to support "fake" tiles (without IDs) for UI display
- Enhanced dialog components with visual unit previews and conditional availability
- Moved piece offset functionality from board-specific location to elements for reuse
Reviewed Changes
Copilot reviewed 21 out of 48 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/elements/tiles/use-piece-offset.ts | New utility for adding random positioning offset to pieces |
| src/elements/tiles/tile.tsx | Refactored to support fake tiles and union type Props |
| src/components/cards/card.silhouette.tsx | Enhanced with visual unit previews and proper semantic structure |
| src/components/board/recruit-dialog.tsx | Improved with conditional knight recruitment and better UX |
| src/components/board/build-dialog.tsx | Enhanced with conditional options and visual improvements |
| src/components/board/board-controller.tsx | Simplified state management with unified SelectedTile type |
Comments suppressed due to low confidence (3)
src/elements/tiles/tile.tsx:8
- The type name '_BaseProps' uses an unconventional underscore prefix. Consider renaming to 'BaseProps' to follow standard TypeScript naming conventions.
type _BaseProps = React.PropsWithChildren<{
src/elements/tiles/tile.tsx:17
- The type name '_TileProps' uses an unconventional underscore prefix. Consider renaming to 'TileProps' to follow standard TypeScript naming conventions.
type _TileProps = _BaseProps & {
src/elements/tiles/tile.tsx:22
- The type name '_FakeTile' uses an unconventional underscore prefix. Consider renaming to 'FakeTileProps' to follow standard TypeScript naming conventions and be more descriptive.
type _FakeTile = _BaseProps & {
|
|
||
| const knightsUnlocked = | ||
| selectedTile?.mode === "recruiting" && | ||
| board[selectedTile.tile]?.building?.type !== "tower"; |
There was a problem hiding this comment.
The logic for 'knightsUnlocked' is inverted. Knights should be unlocked when the building type IS 'tower', not when it's NOT 'tower'. The condition should be '=== "tower"' instead of '!== "tower"'.
Suggested change
| board[selectedTile.tile]?.building?.type !== "tower"; | |
| board[selectedTile.tile]?.building?.type === "tower"; |
Owner
Author
There was a problem hiding this comment.
nop its the opposite, so tower -> does not unlock knights, castle or citadel -> does unlock knights
manuartero
force-pushed
the
feat/improve-recruit-dialog
branch
from
3d1ae72 to
4ab05af
Compare
manuartero
pushed a commit
that referenced
this pull request
Jul 24, 2025
# [0.8.0](v0.7.0...v0.8.0) (2025-07-24) ### Features * **BuildingDialog:** upgrade settlement rendering a tile+castle ([955c471](955c471)) * improve recruit dialog ([#12](#12)) ([82bfa79](82bfa79)), closes [/#diff-0b9f0eaba232e2375eb818ecaca50bf9d9d5d1eae7d870db081df76bab4fbd87R10-R14](https://github.com///issues/diff-0b9f0eaba232e2375eb818ecaca50bf9d9d5d1eae7d870db081df76bab4fbd87R10-R14) [/#diff-0b9f0eaba232e2375eb818ecaca50bf9d9d5d1eae7d870db081df76bab4fbd87L19-R166](https://github.com///issues/diff-0b9f0eaba232e2375eb818ecaca50bf9d9d5d1eae7d870db081df76bab4fbd87L19-R166) [/#diff-0b9f0eaba232e2375eb818ecaca50bf9d9d5d1eae7d870db081df76bab4fbd87L175-R181](https://github.com///issues/diff-0b9f0eaba232e2375eb818ecaca50bf9d9d5d1eae7d870db081df76bab4fbd87L175-R181) [/#diff-0b9f0eaba232e2375eb818ecaca50bf9d9d5d1eae7d870db081df76bab4fbd87L19-R166](https://github.com///issues/diff-0b9f0eaba232e2375eb818ecaca50bf9d9d5d1eae7d870db081df76bab4fbd87L19-R166) [/#diff-0b9f0eaba232e2375eb818ecaca50bf9d9d5d1eae7d870db081df76bab4fbd87L185-R226](https://github.com///issues/diff-0b9f0eaba232e2375eb818ecaca50bf9d9d5d1eae7d870db081df76bab4fbd87L185-R226) [/#diff-a7d39e1ce62f057dcb0b9a496cf7158d020f420cc42c981e857e3c632025b183R4-R43](https://github.com///issues/diff-a7d39e1ce62f057dcb0b9a496cf7158d020f420cc42c981e857e3c632025b183R4-R43) [/#diff-ae895059e339fadc8c0447879380255bda2d52c21a2bec1cb836e4a413eed5caR4-R37](https://github.com///issues/diff-ae895059e339fadc8c0447879380255bda2d52c21a2bec1cb836e4a413eed5caR4-R37) [/#diff-f42224d836ecad44d9ca2adbf24bb87055b6575dccbdf0b945dd59a7577e1bf8L4-R4](https://github.com///issues/diff-f42224d836ecad44d9ca2adbf24bb87055b6575dccbdf0b945dd59a7577e1bf8L4-R4) [/#diff-712bd1a6c29ea78c94b3564ec54f6cb24128ebf049456b5daa74487087bf6ae3L2-R5](https://github.com///issues/diff-712bd1a6c29ea78c94b3564ec54f6cb24128ebf049456b5daa74487087bf6ae3L2-R5) [/#diff-890c6b16786ccd82295d0bd567aa25e29e78098cdb08451af41c0983389e070fL12-R14](https://github.com///issues/diff-890c6b16786ccd82295d0bd567aa25e29e78098cdb08451af41c0983389e070fL12-R14) [/#diff-bc84d163557cb76a2cb1a2ac41a8b9fdfc65ba791d80dc07f1fa35f9dc5d65a5L7-R12](https://github.com///issues/diff-bc84d163557cb76a2cb1a2ac41a8b9fdfc65ba791d80dc07f1fa35f9dc5d65a5L7-R12) [/#diff-6fce9e3455939f50ca95a4e0f13b100eb728d17851da36982f2f6bf04f789300R1-R46](https://github.com///issues/diff-6fce9e3455939f50ca95a4e0f13b100eb728d17851da36982f2f6bf04f789300R1-R46) [/#diff-4fd8f6fc0b6667d97e3bd367df26244f38515681f6340680c939afbd7ec7b872R1-R16](https://github.com///issues/diff-4fd8f6fc0b6667d97e3bd367df26244f38515681f6340680c939afbd7ec7b872R1-R16) * **RecruitDialog:** improve recruit dialog ([2d632e4](2d632e4)) * **RecruitDialog:** now with player colors ([b4b9d7c](b4b9d7c))
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This pull request introduces significant improvements to the
BoardControllercomponent and related files, focusing on consolidating tile state management, enhancing the user interface for building and recruiting actions, and improving code organization. The most important changes include replacing multiple tile state variables with a unifiedSelectedTiletype, updating the dialog components for better usability and clarity, and refactoring imports for consistency.State Management Improvements:
selectedTile,buildingTile, andrecruitingTilestate variables with a singleSelectedTiletype, simplifying state management and improving code readability (src/components/board/board-controller.tsx) [1] [2] [3].resolveBuildOnTile,resolveRecruitOnTile) to use the newSelectedTilestructure, reducing redundant state updates (src/components/board/board-controller.tsx) [1] [2].Dialog Component Enhancements:
BuildDialogandRecruitDialogcomponents to use more descriptive props (buildWalls,upgradeBuilding,recruitSoldier,recruitKnight) and added support for disabling unavailable actions (src/components/board/build-dialog.tsx,src/components/board/recruit-dialog.tsx) [1] [2]..columnCSS class (src/components/board/recruit-dialog.module.css).Codebase Organization:
TileandPiececomponents into a singleelementsmodule for consistency (src/components/board/board.tsx,src/components/cards/card.silhouette.tsx) [1] [2].usePieceOffsethook, simplifying the codebase (src/components/board/tiles/use-piece-offset.ts).UI and Accessibility Updates:
CardSilhouetteto supportaria-labelattributes and disabled states, improving accessibility and usability (src/components/cards/card.silhouette.tsx).CurrentPhaseControllerand related tests to use<h1>for phase headings, aligning with semantic HTML best practices (src/components/current-phase/__snapshots__/current-phase.test.tsx.snap,src/components/current-phase/current-phase-controller.test.tsx) [1] [2].Testing and Snapshots:
CardandCurrentPhaseControllercomponents to reflect changes in structure and accessibility (src/components/cards/__snapshots__/card.test.tsx.snap,src/components/cards/card.test.tsx) [1] [2].These changes collectively improve the maintainability, functionality, and user experience of the board and related components.