Merged
Conversation
planAction() - clean action card
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the game's action planning functionality by improving how players can clean up and manage planned actions during the planification phase. The changes include better handling of null/undefined action cards, improved UI interactions for removing planned cards, and updated component interfaces to support action cleanup.
- Added support for explicitly removing planned action cards by passing
nullvalues - Enhanced UI to allow clicking on planned cards to remove them from the timeline
- Updated component prop names for better clarity (e.g.,
nextAction→nextActionCard)
Reviewed Changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/game-context/use-timeline.tsx |
Enhanced planAction to handle null values for removing cards and updated state management |
src/components/current-phase/current-phase.tsx |
Added click handlers for planned cards and updated prop names |
src/components/player-hand/player-hand-controller.tsx |
Updated to explicitly pass undefined for unused action slots |
src/components/cards/card.tsx |
Restructured card layout with improved text handling |
src/components/player-inventory/player-inventory.tsx |
Added new player inventory component |
| setNext( | ||
| next.slice(1).map((timelineCard) => ({ ...timelineCard, commited: true })) | ||
| ); | ||
| setNext(next.slice(1).map((timelineCard) => ({ ...timelineCard }))); |
There was a problem hiding this comment.
The spread operator { ...timelineCard } creates a shallow copy but doesn't modify any properties. This appears unnecessary since the original comment mentioned losing committed state was an issue. Consider if this line should maintain the committed property or if the spread is redundant.
Suggested change
| setNext(next.slice(1).map((timelineCard) => ({ ...timelineCard }))); | |
| setNext(next.slice(1).map((timelineCard) => ({ ...timelineCard, commited: timelineCard.commited }))); |
Comment on lines
+39
to
+43
| function nextNotCommitedActionCard( | ||
| timelineCards: TimelineCard[] | ||
| ): ActionCard | undefined { | ||
| return timelineCards.find((timelineCard) => !timelineCard.commited) | ||
| ?.card as ActionCard; |
There was a problem hiding this comment.
Using type assertion as ActionCard without proper type checking could lead to runtime errors if the card is not actually an ActionCard. Consider adding proper type guards or handling the case where the card might be a different type.
Suggested change
| function nextNotCommitedActionCard( | |
| timelineCards: TimelineCard[] | |
| ): ActionCard | undefined { | |
| return timelineCards.find((timelineCard) => !timelineCard.commited) | |
| ?.card as ActionCard; | |
| function isActionCard(card: any): card is ActionCard { | |
| return card && typeof card === "object" && "actionType" in card; | |
| } | |
| function nextNotCommitedActionCard( | |
| timelineCards: TimelineCard[] | |
| ): ActionCard | undefined { | |
| const timelineCard = timelineCards.find((timelineCard) => !timelineCard.commited); | |
| if (timelineCard && isActionCard(timelineCard.card)) { | |
| return timelineCard.card; | |
| } | |
| return undefined; |
Comment on lines
17
to
+18
| } | ||
|
No newline at end of file |
There was a problem hiding this comment.
There's trailing whitespace at the end of the JSON file which should be removed for consistency.
Suggested change
| } | |
| } |
Comment on lines
14
to
16
| "upgrade-settlement": "Improve this settlement to next level", | ||
| "recruit-soldier": "Recruit a solider on this settlement", | ||
| "recruit-knight": "Recruit a knight on this settlement" |
There was a problem hiding this comment.
The properties upgrade-settlement, recruit-soldier, and recruit-knight are still using string format while other entries have been converted to arrays. This inconsistency could cause issues when rendering card text.
Suggested change
| "upgrade-settlement": "Improve this settlement to next level", | |
| "recruit-soldier": "Recruit a solider on this settlement", | |
| "recruit-knight": "Recruit a knight on this settlement" | |
| "upgrade-settlement": ["Improve this settlement to next level"], | |
| "recruit-soldier": ["Recruit a solider on this settlement"], | |
| "recruit-knight": ["Recruit a knight on this settlement"] |
manuartero
pushed a commit
that referenced
this pull request
Jul 21, 2025
# [0.6.0](v0.5.0...v0.6.0) (2025-07-21) ### Features * clean up planned actions ([#9](#9)) ([58383c0](58383c0)), closes [/#diff-90bbcc39ac60a1eab7e6860e67e12c343d2b382c50dcfad35d19340c050ec9caL17-R22](https://github.com///issues/diff-90bbcc39ac60a1eab7e6860e67e12c343d2b382c50dcfad35d19340c050ec9caL17-R22) [/#diff-90bbcc39ac60a1eab7e6860e67e12c343d2b382c50dcfad35d19340c050ec9caL33-R32](https://github.com///issues/diff-90bbcc39ac60a1eab7e6860e67e12c343d2b382c50dcfad35d19340c050ec9caL33-R32) [/#diff-712bd1a6c29ea78c94b3564ec54f6cb24128ebf049456b5daa74487087bf6ae3L20-R26](https://github.com///issues/diff-712bd1a6c29ea78c94b3564ec54f6cb24128ebf049456b5daa74487087bf6ae3L20-R26) [/#diff-90bbcc39ac60a1eab7e6860e67e12c343d2b382c50dcfad35d19340c050ec9caR43-R60](https://github.com///issues/diff-90bbcc39ac60a1eab7e6860e67e12c343d2b382c50dcfad35d19340c050ec9caR43-R60) [/#diff-42cbd032f984cc3e888bb85ab6d90b9abba1269d5e603a516d5faf700dd76796L48-R91](https://github.com///issues/diff-42cbd032f984cc3e888bb85ab6d90b9abba1269d5e603a516d5faf700dd76796L48-R91) [/#diff-aa484547f46facfade941a61018907628c6fd9e2f274e354a06f894397b66b6aL2-R71](https://github.com///issues/diff-aa484547f46facfade941a61018907628c6fd9e2f274e354a06f894397b66b6aL2-R71) [/#diff-aa484547f46facfade941a61018907628c6fd9e2f274e354a06f894397b66b6aR80](https://github.com///issues/diff-aa484547f46facfade941a61018907628c6fd9e2f274e354a06f894397b66b6aR80) [/#diff-039843c463359e376aea21c630e005846b44ff667aea0ca666b63c5635ab01b6L30-L38](https://github.com///issues/diff-039843c463359e376aea21c630e005846b44ff667aea0ca666b63c5635ab01b6L30-L38) [/#diff-a5505dcfecf093662fecb1130e743c8d7f1c6321a058aee81d67964f277bc101L19-R47](https://github.com///issues/diff-a5505dcfecf093662fecb1130e743c8d7f1c6321a058aee81d67964f277bc101L19-R47) [/#diff-42cbd032f984cc3e888bb85ab6d90b9abba1269d5e603a516d5faf700dd76796L18-R22](https://github.com///issues/diff-42cbd032f984cc3e888bb85ab6d90b9abba1269d5e603a516d5faf700dd76796L18-R22) [/#diff-86f9b8039451f73a5f04c82c7b6dbe9163db1d78c52d043a76a07734bdade148L2-R18](https://github.com///issues/diff-86f9b8039451f73a5f04c82c7b6dbe9163db1d78c52d043a76a07734bdade148L2-R18) [/#diff-890c6b16786ccd82295d0bd567aa25e29e78098cdb08451af41c0983389e070fL3-R3](https://github.com///issues/diff-890c6b16786ccd82295d0bd567aa25e29e78098cdb08451af41c0983389e070fL3-R3) [/#diff-890c6b16786ccd82295d0bd567aa25e29e78098cdb08451af41c0983389e070fL135-L192](https://github.com///issues/diff-890c6b16786ccd82295d0bd567aa25e29e78098cdb08451af41c0983389e070fL135-L192) * **planification:** clean up planning cards (next and future) ([cc976c9](cc976c9))
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
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 updates to the card rendering system, layout structure, and game phase logic. It also includes improvements to CSS styling, JSON structure, and test snapshots. The changes aim to enhance flexibility, maintainability, and user experience.
Card Rendering and Behavior Updates:
CardandCardSilhouettecomponents to use new CSS classes (.silhouette,.title,.text) and replaced<div>elements with semantic<article>tags for better accessibility. ([[1]](https://github.com/manuartero/storming/pull/9/files#diff-90bbcc39ac60a1eab7e6860e67e12c343d2b382c50dcfad35d19340c050ec9caL17-R22),[[2]](https://github.com/manuartero/storming/pull/9/files#diff-90bbcc39ac60a1eab7e6860e67e12c343d2b382c50dcfad35d19340c050ec9caL33-R32),[[3]](https://github.com/manuartero/storming/pull/9/files#diff-712bd1a6c29ea78c94b3564ec54f6cb24128ebf049456b5daa74487087bf6ae3L20-R26))ActionCardContentsto dynamically render multi-line card descriptions using a mapped array from JSON data. ([src/components/cards/card.tsxR43-R60](https://github.com/manuartero/storming/pull/9/files#diff-90bbcc39ac60a1eab7e6860e67e12c343d2b382c50dcfad35d19340c050ec9caR43-R60))PlanningPhaseto allow clearing selected action cards via click events, using the newonCleanActionCardcallback. ([src/components/current-phase/current-phase.tsxL48-R91](https://github.com/manuartero/storming/pull/9/files#diff-42cbd032f984cc3e888bb85ab6d90b9abba1269d5e603a516d5faf700dd76796L48-R91))CSS and Styling Enhancements:
card.module.cssto improve layout consistency, including updated dimensions, spacing, and shadow effects. Introduced new classes like.silhouette,.heading,.content, and.icon. ([[1]](https://github.com/manuartero/storming/pull/9/files#diff-aa484547f46facfade941a61018907628c6fd9e2f274e354a06f894397b66b6aL2-R71),[[2]](https://github.com/manuartero/storming/pull/9/files#diff-aa484547f46facfade941a61018907628c6fd9e2f274e354a06f894397b66b6aR80))current-phase.module.cssfile by removing unused styles such as.planificationand.action. ([src/components/current-phase/current-phase.module.cssL30-L38](https://github.com/manuartero/storming/pull/9/files#diff-039843c463359e376aea21c630e005846b44ff667aea0ca666b63c5635ab01b6L30-L38))Game Phase Logic Improvements:
CurrentPhaseControllerby introducing a helper functionnextNotCommitedActionCardto determine the next uncommitted action card. ([src/components/current-phase/current-phase-controller.tsxL19-R47](https://github.com/manuartero/storming/pull/9/files#diff-a5505dcfecf093662fecb1130e743c8d7f1c6321a058aee81d67964f277bc101L19-R47))CurrentPhaseandPlanningPhasecomponents to usenextActionCardandfutureActionCardproperties instead ofnextActionandfutureAction. ([src/components/current-phase/current-phase.tsxL18-R22](https://github.com/manuartero/storming/pull/9/files#diff-42cbd032f984cc3e888bb85ab6d90b9abba1269d5e603a516d5faf700dd76796L18-R22))JSON Structure Changes:
card-text.jsonto use arrays for multi-line descriptions, improving flexibility for rendering card text. ([src/components/cards/card-text.jsonL2-R18](https://github.com/manuartero/storming/pull/9/files#diff-86f9b8039451f73a5f04c82c7b6dbe9163db1d78c52d043a76a07734bdade148L2-R18))Test Snapshot Updates:
<div>with<section>and<article>tags in test snapshots for better semantic alignment. Updated snapshots to reflect the new CSS class changes and dynamic card text rendering. ([[1]](https://github.com/manuartero/storming/pull/9/files#diff-890c6b16786ccd82295d0bd567aa25e29e78098cdb08451af41c0983389e070fL3-R3),[[2]](https://github.com/manuartero/storming/pull/9/files#diff-890c6b16786ccd82295d0bd567aa25e29e78098cdb08451af41c0983389e070fL135-L192))These changes collectively improve the component structure, styling consistency, and user interaction while ensuring better maintainability and alignment with modern development practices.