Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
apps/scouting/frontend/src/strategy/components/HeatMapIntensityCanvas.tsx
Outdated
Show resolved
Hide resolved
apps/scouting/frontend/src/strategy/components/HeatMapIntensityCanvas.tsx
Outdated
Show resolved
Hide resolved
apps/scouting/frontend/src/strategy/components/HeatMapIntensityCanvas.tsx
Outdated
Show resolved
Hide resolved
apps/scouting/frontend/src/strategy/components/HeatMapIntensityCanvas.tsx
Outdated
Show resolved
Hide resolved
apps/scouting/frontend/src/strategy/components/HeatMapIntensityCanvas.tsx
Outdated
Show resolved
Hide resolved
apps/scouting/frontend/src/strategy/components/HeatMapIntensityColorizer.ts
Outdated
Show resolved
Hide resolved
| const clamped = Math.min(NORMALIZED_MAX, Math.max(NORMALIZED_MIN, value)); | ||
| for (const [index, start] of COLOR_RAMP.slice(COLOR_RAMP_OFFSET, -RAMP_LAST_OFFSET).entries()) { | ||
| const end = COLOR_RAMP[index + RAMP_INDEX_STEP]; | ||
| if (clamped >= start.stop && clamped <= end.stop) { |
apps/scouting/frontend/src/strategy/components/HeatMapIntensityColorizer.ts
Outdated
Show resolved
Hide resolved
| const COLOR_STOP_COOL = 0.2; | ||
| const COLOR_STOP_CYAN = 0.4; | ||
| const COLOR_STOP_GREEN = 0.6; | ||
| const COLOR_STOP_WARM = 0.8; |
There was a problem hiding this comment.
if you are already putting the colors in the object just put the stops too
| interface HeatMapProps { | ||
| positions: Point[]; | ||
| path: string; | ||
| aspectRatio: number; |
There was a problem hiding this comment.
consider adding comment of aspectRatio = width/height or vice versa, just actually write down which one you're using
|
|
||
| interface HeatMapProps { | ||
| positions: Point[]; | ||
| path: string; |
There was a problem hiding this comment.
not sure this belongs in the heatmap, if it does, name it appropriately (backgroundImagePath for example)
| } | ||
|
|
||
| export const HeatMap: FC<HeatMapProps> = ({ positions, path, aspectRatio }) => { | ||
| const { heatmapLayerRef, imgRef, fallbackPoints, handleImageLoad, radius, overlaySize } = useHeatMap( |
There was a problem hiding this comment.
if these are all important props, just use heatmap as object, don't split them unnecessarily it's cluttered and it hurts to read
| @@ -0,0 +1,129 @@ | |||
| // בס"ד | |||
There was a problem hiding this comment.
this entire file works, but it is not legible, it is not documented, and nobody in the team can concisely explain what it does. i'm passing it because it works, but you need to clean this up
| }); | ||
|
|
||
| const getRampColor = (value: number): ColorRGB => { | ||
| const clamped = Math.min(NORMALIZED_MAX, Math.max(NORMALIZED_MIN, value)); |
There was a problem hiding this comment.
assuming value is not between 0 to 1 (why) - inverse lerp it so it sits between the two (and clamp)
example - normalized = clamp01((n - min) / (max - min))
| const ramp = COLOR_RAMP.slice(COLOR_RAMP_OFFSET, -RAMP_LAST_OFFSET); | ||
| const rampPairs = ramp.slice(COLOR_RAMP_OFFSET, -RAMP_INDEX_STEP); |
| blue: Math.round(lerp(start.blue, end.blue, t)), | ||
| }); | ||
|
|
||
| const getRampColor = (value: number): ColorRGB => { |
There was a problem hiding this comment.
homework: answer what the hell does value mean
| @@ -0,0 +1,88 @@ | |||
| // בס"ד | |||
There was a problem hiding this comment.
despite active protesting from the affected programmer - this file is also incomprehensible
fix it, but not now
|
|
||
| export const useHeatMap = ( | ||
| positions: Point[], | ||
| path: string, |
|
|
||
| return { | ||
| heatmapLayerRef, | ||
| imgRef, |
| ); | ||
| }, [positions]); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
correction - if not remove, justify why it exists alongside onImageLoad
|
3 participants
crazy vro👽👽👽👽