Refactor CircularHeatmapComponent: Extract D3 Logic to D3HeatmapService and Enhance Type Safety #454#477
Conversation
There was a problem hiding this comment.
Thank you @Biswas-Samrat for splitting this into two different files. The heatmap was a bit bulky. Good to separate the d3 logic. However, it does not bear the normal resemblance of a service. Maybe we'd better call it a "renderer". (See suggestion below.)
However, clicking the heatmap does not longer work. It is supposed to open a list of activities in the clicked sector. My console reads: 12.845: Heat: Clicked disabled sector: 'undefined' Level: undefined
Are you able to reproduce this, @Biswas-Samrat?
In the console log, I get:
circular-heatmap.component.ts:144 ERROR TypeError: Cannot set properties of undefined (setting 'colors')
at D3HeatmapService.updateThemeColors (d3-heatmap.service.ts:208:23)
at Object.next (circular-heatmap.component.ts:157:27)
Could you check out this, @Biswas-Samrat ?
I see that you had to bend to the linter's wishes. ; )
Linting is good. But a bit picky for my liking, sometimes.
I (often) find it easier to read console messages that are on one line, than broken up. (Depends on how long and complex it is, obviously, but still.)
For the console messages that I prefer having on one code line, I append // eslint-disable-line at the end. Making it even longer, but hopefully more readable.
Feel free to use the same method in DSOMM where you find that a better solution.
| import { ThemeService } from '../../service/theme.service'; | ||
| import { TitleService } from '../../service/title.service'; | ||
| import { SettingsService } from 'src/app/service/settings/settings.service'; | ||
| import { D3HeatmapService, HeatmapColors } from '../../service/d3-heatmap.service'; |
There was a problem hiding this comment.
The class in not really a typical service. I reckon we're better off calling it a "renderer". It' doesn't really fit anywhere, but we can create new folder, in case we get others later.
import { D3HeatmapRenderer, HeatmapColors } from '../../renderer/d3-heatmap.renderer';
This PR addresses issue #454 by refactoring the
CircularHeatmapComponent
to improve maintainability, reduce cognitive load, and enforce strict type safety. The primary goal was to separate Angular component logic from complex D3.js SVG manipulations.
Key Changes
Separation of Concerns: Extracted all D3.js rendering, SVG construction, and animation logic into a new dedicated service: