search

zengm-games / zengm

Basketball GM (and other ZenGM games) are single-player sports management simulation games, made entirely in client-side JavaScript.
https://zengm.com
Other
349 stars 127 forks source link

Add total salary of selected players for 'Trading Block Page' #450

Open fearandesire opened 1 year ago

fearandesire commented 1 year ago

Hi,

This PR is concerning this TODO

I've added this simple text to show the contract sum of the current players selected. You can preview what it looks like in this image The code and display for it isn't perfect, and I'm open to making adjustments as @dumbmatter. I also understand if this feature isn't necessarily what should be added at this time, and OK with this being scrapped.

dumbmatter commented 1 year ago

Thanks for doing this!

A couple changes would be nice before merging:

  1. Rather than adding two new state variables and a new callback function to keep it in sync, just add up the contract sum in the body of the TradingBlock component. This will be shorter code, will be plenty fast, and is also generally what people recommend for React these days, for example https://kentcdodds.com/blog/dont-sync-state-derive-it

  2. Maybe move the sum somewhere else. Maybe right below/above the submit button? I'm not sure, it just looks a little out of place at the top.