avgupta456 / statbotics

📈 Modernizing Data Analytics for FRC Robotics
https://statbotics.io/
MIT License
64 stars 9 forks source link

Fix colors not loading for events with offseason team numbers #347

Closed jonahsnider closed 7 months ago

jonahsnider commented 7 months ago

Fixes a bug where some events included teams with very large numbers for their offseason robots, which the FRC Colors API does not accept as valid.

ex. team 1234B is represented as team 123400001, but should be mapped to be just 1234

Without this PR, the request to FRC Colors fails, and no colors are returned

vercel[bot] commented 7 months ago

Someone is attempting to deploy a commit to a Personal Account owned by @avgupta456 on Vercel.

@avgupta456 first needs to authorize it.

avgupta456 commented 7 months ago

Thanks for working on this! Is it also possible to handle the case where the bubble is completely white (maybe in the context provider so it is centralized?)

jonahsnider commented 7 months ago

Sure, how do you think that should be implemented? Check if the luminance of the primary color is below a threshold (so, poor contrast against a white background), and then exclude it from the colors Map? (which would cause it to render as just the default blue color)

avgupta456 commented 7 months ago

I think there should be some visual difference between having a blue color and having a light color. Also not sure what the ideal solution is. Maybe note which teams are light in the context and draw a thin gray/black outline in the figure?

jonahsnider commented 7 months ago

I think ideally if we are drawing an outline for each point, it would use the secondary color for teams instead of just black/gray. This is what the 581 scouting app does currently:screenshot of graph

Basically every team with a white/very light primary color will have a secondary color that is much darker & easier to see on a white background.

jonahsnider commented 7 months ago

I tried seeing if either the bubble or scatter plot allow setting the border color of the points, and neither seemed like they had an easy API to do so. Most likely it would need to be implemented in a fairly hands-on way, maybe styling the individual points on the DOM themselves by setting inline styles on them?

avgupta456 commented 7 months ago

I'm planning on switching all figures to https://airbnb.io/visx soon, so maybe we can table this until then.

jonahsnider commented 7 months ago

That works too. Should I make a PR (or update this one) to just set the points to be the default blue if the contrast is poor? As a temporary improvement.

avgupta456 commented 7 months ago

Yeah that works for now. You can just add it to this PR I think

jonahsnider commented 7 months ago

@avgupta456 are you able to review this please?

avgupta456 commented 7 months ago

Instead of depending on color-luminance, can you just define and use one of these functions?

//Uses Rec. 709 (HDTV) coefficients
//R * 0.2126 + G * 0.7152 + B * 0.0722
var y1 = luminance.rec709(r, g, b);

//Uses Rec. 601 (PAL/NTSC) coefficients
//R * 0.299 + G * 0.587 + B * 0.114
var y2 = luminance.rec601(r, g, b);

And then just for simplicity, can you not use a for ... of loop so we don't need to bump ES5 yet. I'll do that alongside some more structural changes in a couple months.

jonahsnider commented 7 months ago

@avgupta456 updated the PR

vercel[bot] commented 7 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
statbotics ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 23, 2023 9:19pm