Face global settings, team colors in Dashboard Face, and prefer player Face over blank-face.png #457
Conversation
(1) Allow faces to be submitted to Global settings and save to player face (2) Prefer player face over blank-face.png (3) Pull team colors to Dashboard player face
|
This is to address #440 |
| if (event.tid) { | ||
| team = await idb.getCopy.teamsPlus( | ||
| { | ||
| attrs: ["colors"], | ||
| tid: event.tid, | ||
| // season: event.season, | ||
| }, | ||
| "noCopyCache", | ||
| ); | ||
| } |
There was a problem hiding this comment.
For this change...
- Would be better to use the
getTeamInfoBySeasonfunction, because that can get the jersey colors for a specific season, in the case that they changed over time. Since past seasons can be viewed on this page. - Put team colors on the
eventobject, rather thanp.face, I think that is clearer. Which could be done inprocessEvents. - In
processEvents, it should find all the tids needed to display this page (either from tid parameter, or by scanning all events if tid is undefined), and then get the colors for all of them up front, rather than doing it separately for each event. SincegetTeamInfoBySeasonhas to read from the hard drive for past seasons. - Also make a similar change in Headlines.tsx for the dashboard. We should also make sure there's no performance issue from using
getTeamInfoBySeasonfrom the dashboard - maybe add a check up front for if it's the current season, and then doawait idb.cache.teamSeasons.indexGet("teamSeasonsByTidSeason", [tid, g.get("season")]);if it is, which bypasses hitting the hard drive.
If that sounds like too much, then maybe take the news feed changes out of this PR, and copy the above text to another issue for later :) honestly that might be better to have a separate PR, since it's a little complicated.
| @@ -20,7 +20,7 @@ const PlayerPicture = ({ | |||
| }) => { | |||
| const [wrapper, setWrapper] = useState<HTMLDivElement | null>(null); | |||
| useEffect(() => { | |||
| if (face && !imgURL && wrapper) { | |||
| if (face && (!imgURL || imgURL == "/img/blank-face.png") && wrapper) { | |||
There was a problem hiding this comment.
I think this change (treat the blank-face URL as special, and put it behind a cartoon face in priority) could be confusing. I would prefer keeping the old behavior - imgURL always has priority. You can set imgURL to an empty string (ugh why did I make that the "undefined" value rather than just using undefined) when applying the face objects.
(I think ideally I only would have supported either imgURL or face being present, probably by putting them on the same property of a player object, similar to how they're treated by this PR in the global data. But it's not worth a schema change to fix that now.)
Allow faces to be submitted to Global settings and save to player face
Prefer player face over blank-face.png
Pull team colors to Dashboard player face
I tested inputting player faces to Player Photos. It worked well, and pulling invalid JSON throws an error
With changes
Original
Original with all fictional players - you'll notice the team colors don't come through
