Skip to content

Refactor CircularHeatmapComponent: Extract D3 Logic to D3HeatmapService and Enhance Type Safety #454#477

Open
Biswas-Samrat wants to merge 2 commits intodevsecopsmaturitymodel:mainfrom
Biswas-Samrat:refactor-circular-heatmap-454
Open

Refactor CircularHeatmapComponent: Extract D3 Logic to D3HeatmapService and Enhance Type Safety #454#477
Biswas-Samrat wants to merge 2 commits intodevsecopsmaturitymodel:mainfrom
Biswas-Samrat:refactor-circular-heatmap-454

Conversation

@Biswas-Samrat
Copy link
Contributor

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:

@wurstbrot wurstbrot requested review from 0x41head and vbakke January 30, 2026 17:54
Copy link
Collaborator

@vbakke vbakke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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';

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants