Added player rating changes for player profile (basketball and football)#280
Conversation
|
This is cool, but I was thinking... do it for all the ratings! Not just ovr and pot. Or do you think that would look too busy? |
dumbmatter
left a comment
dumbmatter
left a comment
There was a problem hiding this comment.
Thanks, this looks almost perfect!
|
|
||
| const RatingsOverview = ({ ratings }: { ratings: PlayerRatings[] }) => { | ||
| const r = ratings.length - 1; | ||
| const lastSeason: PlayerRatings[] = |
There was a problem hiding this comment.
I think it would be cleaner if you did something like:
let lastSeason: PlayerRatings = ratings[r];
// Search backwards to find the last entry from last season, in the case where there are multiple rows due to injuries
for (let i = ratings.length - 1; i--; i >= 0) {
if (ratings[i].season === ratings[r].season - 1) {
lastSeason = ratings[i];
break;
}
}
and then you can get rid of all the Math.max calls below.
There was a problem hiding this comment.
(Also the Math.max calls are not even what we want, we'd want Math.min because ratings can currently only go down within a season, and we want to compare with the last ratings from last season. However my suggested change is more future proof, in case eventually there is some way for ratings to increase.)
| <div className="col-lg-8"> | ||
| <h2>Overall: {ratings[r].ovr}</h2> | ||
| <h2> | ||
| Overall: |
There was a problem hiding this comment.
Unless there's a good reason for it shouldn't be used. You could use either {" "} or move the <RatingWithChange up to this line, like Overall: <RatingWithChange. Actually doesn't matter which you pick, I think Prettier will ultimately decide between them :)
1 participant
No description provided.