feature(demo-game): replace decision toggles with number input fields #77
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis pull request introduces significant changes to the demo game application, focusing on transforming decision-making from boolean to percentage-based representations. The modifications span multiple files, including package dependencies, component rendering, form handling, and service logic. The changes primarily involve updating the Changes
Possibly related PRs
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
apps/demo-game/src/services/ActionsReducer.ts (1)
9-9: Enum & Type Consolidation
By introducingActionTypes.NONEand simplifyingPayloadTypeandStateto rely on theDecisionstype, the code eliminates separate boolean flags in favor of a single numeric-based decisions object. This improves maintainability and readability.Also applies to: 13-13, 19-19, 22-22
apps/demo-game/src/services/PeriodResultService.ts (1)
31-31: Default Decision Allocations
Initializing decisions with{ bank: 100, bonds: 0, stocks: 0 }sets a clear default baseline. If user flows permit partial or different defaults, consider adjusting or making it configurable to avoid unintended assumptions.apps/demo-game/src/components/DecisionsDisplay.tsx (1)
115-130: Replacing On/Off Icons with Percent Values
Commenting outOnOffIconin favor of direct percentage display conveys exact user allocations more transparently. Good alignment with your new numeric representation.apps/demo-game/src/services/SegmentResultService.ts (1)
80-89: Deprecated Allocation Logic
Removing the bucket-based distribution logic (commented out) is a sensible step if partial or gradual investments are no longer needed. If you plan to revert or refine this in the future, consider preserving it as a separate helper rather than inline comments.apps/demo-game/src/pages/play/welcome.tsx (2)
120-120: Minor text and styling addition.
This line adds a helpful introduction block. Confirm the text matches the established tone and style guidelines.
179-179: Clarify label's purpose.
The new label "Avatar" is descriptive but ensure users fully understand they are selecting an avatar image. Consider adding help text if confusion is possible.apps/demo-game/src/pages/admin/games/[id].tsx (4)
680-680: Visible period name inputs.
UsingFormikTextFieldhere allows for a clear textual name for game periods. Verify that any constraints (e.g., length) are validated.
706-706: Seed field conversion.
A numeric seed input can simplify random generation scenarios. Check for valid integer inputs and handle invalid or negative seeds gracefully.
734-734: Bond trend usage.
trendBondsrepresents expected returns. Add hints or instructions so users understand typical or recommended ranges (e.g., between -1.0 and 1.0 if you’re working with percentages).
743-743: Gap for bonds.
gapBondsindicates possible variance around the trend. Consider restricting negative values or clarifying how big the gap can be.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
apps/demo-game/package.json(1 hunks)apps/demo-game/src/components/DecisionsDisplay.tsx(2 hunks)apps/demo-game/src/pages/admin/games/[id].tsx(9 hunks)apps/demo-game/src/pages/admin/reports/[id].tsx(1 hunks)apps/demo-game/src/pages/play/cockpit.tsx(5 hunks)apps/demo-game/src/pages/play/welcome.tsx(4 hunks)apps/demo-game/src/services/ActionsReducer.ts(2 hunks)apps/demo-game/src/services/PeriodResultService.ts(2 hunks)apps/demo-game/src/services/SegmentResultService.ts(1 hunks)apps/demo-game/src/types/facts.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/demo-game/src/pages/admin/reports/[id].tsx
🔇 Additional comments (22)
apps/demo-game/src/services/ActionsReducer.ts (2)
5-5: Importing Decisions
Using a specific Decisions type clarifies the structure of the decision payload and promotes type-safety across the reducer.
34-34: Direct Assignment Improves Simplicity
Replacing separate conditional updates with a direct assignment of action.payload.playerArgs to draft.result.decisions greatly simplifies logic at the cost of removing older checks. Make sure any validation needed is performed before dispatching actions.
apps/demo-game/src/pages/play/cockpit.tsx (5)
3-3: New Imports for Form Controls
Importing Button, FormikNumberField, and Switch from @uzh-bf/design-system provides cohesive UI components, ensuring consistency in styling and behavior throughout the cockpit.
54-55: Formik & Yup Integration
Introducing Formik for form management and yup for validation streamlines the handling of user input and error states. This setup is much cleaner than manually controlling state and validations.
817-817: Future Value Logic
Using assets.totalAssets * resultFactsDecisions.<asset> in each row ensures that the future value is proportionate to total assets. Confirm that these calculations align with any other parts of the code that expect a different breakdown (e.g., partial allocations).
Also applies to: 824-824, 831-831
853-881: Validation Schema for Asset Distribution
The schema enforces integer values between 0 and 100 for each allocation and ensures they sum to 100. This is a robust approach to capturing valid user input and preventing unbalanced allocations.
955-1024: Formik-based Form for Decisions
The form neatly ties user input to updated decisions. The final mutation with performAction properly sends numeric values for each asset category. Be mindful to catch any server-side errors or business logic constraints beyond the client-side validations.
apps/demo-game/src/services/PeriodResultService.ts (1)
50-52: Consistent Re-initialization
Reassigning numerical values for bank, bonds, and stocks within the produce block reaffirms the default. Double-check if you want to carry over any prior state or user preferences before overriding.
apps/demo-game/src/components/DecisionsDisplay.tsx (1)
27-29: Shift from Boolean to Numeric
Updating bank, bonds, and stocks to numeric types clarifies that these represent percentages rather than simple on/off states. This change aligns with the new design requirements.
apps/demo-game/src/services/SegmentResultService.ts (1)
91-93: Target Assets with Numeric Percentages
Multiplying facts.decisions.<asset> * 0.01 * totalAssets is simpler and more intuitive than before. Verify edge cases such as sums exceeding 100 or negative allocations to prevent unexpected behaviors.
apps/demo-game/src/pages/play/welcome.tsx (5)
6-7: Use of standard Formik components.
The switch from NewFormikSelectField / NewFormikTextField to FormikSelectField / FormikTextField helps align the code with the updated design system. Ensure that all form props and validation requirements remain consistent with the new components.
18-18: Reorganized import for clarity.
Importing LogoSelector higher up makes it more visible and maintains a cleaner import organization structure.
160-160: Consistent naming conventions.
Using FormikTextField approximates a more modern approach than the previous custom fields. Double-check that any custom logic previously attached to NewFormikTextField is fully migrated.
165-165: Selector alignment with design guidelines.
The shift to FormikSelectField is consistent with the rest of the application’s changes. Verify that the items prop and the className usage remain in sync with the updated design system.
185-185: Expanding color options.
Reusing FormikSelectField for color selection is a good approach. Verify that the color keys in COLORS map well to user-friendly labels or icons.
[approve]
apps/demo-game/src/pages/admin/games/[id].tsx (5)
11-12: Standardized form fields.
Replacing NewFromikNumberField/NewFormikTextField with FormikNumberField/FormikTextField improves consistency across the form. Confirm that no custom properties were lost in the process.
687-687: Improved numeric input for segments.
FormikNumberField is well-suited for numeric data. Consider adding min/max or range validation to ensure valid segment counts.
720-720: Interest rate validation.
Ensure user inputs for interestBank are sensible (e.g., a positive rate or 0). Possibly add range checks or helpful error messages.
757-757: Stock trend usage.
trendStocks also uses numeric inputs. Confirm usage consistency with the bond trend field, so the user experience is uniform.
823-823: Segment countdown control.
FormikNumberField for countdownSeconds is precise. Validate the lower bound (e.g., ensure the user can’t set a negative or zero duration).
apps/demo-game/src/types/facts.ts (1)
14-16: Shift from boolean to numeric decisions.
Changing bank, bonds, and stocks to numbers offers more flexibility (e.g., percentage allocations). Confirm all references to these fields handle numeric logic (sum checks, validations, etc.).
apps/demo-game/package.json (1)
20-20: Updated design system version.
The @uzh-bf/design-system upgrade indicates new components or breaking changes. Ensure that all replaced or new Formik fields and styles remain compatible.
| } | ||
| ) | ||
| .exhaustive() | ||
| draft.result.decisions = action.payload.playerArgs |
There was a problem hiding this comment.
TODO: validate again to ensure every item is between 0-100 and they sum to 100 -> prevent any potential issues in games by (future) frontend bugs that allow sending inconsistent numbers
if it is not valid input, throw an error (not necessary to do frontend work for now, but in the future we should catch errors in the UI and show a toast with a message)
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores