Skip to content

Commit 262cac5

Browse files
committed
fix: rename D3HeatmapService to D3HeatmapRenderer and fix click/theme bugs
Addresses maintainer feedback on PR #477: - Renamed D3HeatmapService to D3HeatmapRenderer per suggestion - Moved file from service/ to renderer/ folder - Fixed TypeError when updateThemeColors called before initialize - Fixed undefined sector issue in click/hover handlers by adding fallback lookup - Added isInitialized() method to guard against premature theme updates - Improved type safety by replacing 'any' with proper TypeScript types
1 parent 55e11bd commit 262cac5

2 files changed

Lines changed: 66 additions & 25 deletions

File tree

src/app/pages/circular-heatmap/circular-heatmap.component.ts

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import { downloadYamlFile } from 'src/app/util/download';
2727
import { ThemeService } from '../../service/theme.service';
2828
import { TitleService } from '../../service/title.service';
2929
import { SettingsService } from 'src/app/service/settings/settings.service';
30-
import { D3HeatmapService, HeatmapColors } from '../../service/d3-heatmap.service';
30+
import { D3HeatmapRenderer, HeatmapColors } from '../../renderer/d3-heatmap.renderer';
3131

3232
@Component({
3333
selector: 'app-circular-heatmap',
@@ -71,7 +71,7 @@ export class CircularHeatmapComponent implements OnInit, OnDestroy {
7171
private route: ActivatedRoute,
7272
private location: Location,
7373
public modal: ModalMessageComponent,
74-
private heatmapService: D3HeatmapService
74+
private heatmapRenderer: D3HeatmapRenderer
7575
) {
7676
this.theme = this.themeService.getTheme();
7777
}
@@ -153,14 +153,16 @@ export class CircularHeatmapComponent implements OnInit, OnDestroy {
153153
};
154154
console.debug(`${perfNow()}s: themeService.pipe: Heatmap theme colors:`, this.theme_colors);
155155

156-
// Repaint segments with new theme
157-
this.heatmapService.updateThemeColors(this.theme_colors);
158-
this.reColorHeatmap();
156+
// Only update theme colors if renderer is initialized
157+
if (this.heatmapRenderer.isInitialized()) {
158+
this.heatmapRenderer.updateThemeColors(this.theme_colors);
159+
this.reColorHeatmap();
160+
}
159161
});
160162
}
161163

162164
private initializeHeatmap() {
163-
this.heatmapService.initialize(
165+
this.heatmapRenderer.initialize(
164166
'#chart',
165167
{
166168
imageWidth: 1200,
@@ -172,13 +174,13 @@ export class CircularHeatmapComponent implements OnInit, OnDestroy {
172174
sector => this.getSectorProgress(sector)
173175
);
174176

175-
this.heatmapService.render(
177+
this.heatmapRenderer.render(
176178
this.allSectors,
177179
{
178180
onClick: (sector, index, id) => {
179181
this.selectedSector = sector;
180182
if (this.selectedSector?.activities?.length) {
181-
this.heatmapService.setSectorCursor('#selected', id);
183+
this.heatmapRenderer.setSectorCursor('#selected', id);
182184
this.showActivityCard = this.selectedSector;
183185
console.log(
184186
`${perfNow()}: Heat: Clicked sector: '${this.selectedSector.dimension}' Level: ${
@@ -187,7 +189,7 @@ export class CircularHeatmapComponent implements OnInit, OnDestroy {
187189
);
188190
} else {
189191
this.showActivityCard = null;
190-
this.heatmapService.setSectorCursor('#selected', '');
192+
this.heatmapRenderer.setSectorCursor('#selected', '');
191193
console.log(
192194
`${perfNow()}: Heat: Clicked disabled sector: '${
193195
this.selectedSector?.dimension
@@ -197,18 +199,18 @@ export class CircularHeatmapComponent implements OnInit, OnDestroy {
197199
},
198200
onMouseOver: (sector, index, id) => {
199201
if (sector?.activities?.length) {
200-
this.heatmapService.setSectorCursor('#hover', id);
202+
this.heatmapRenderer.setSectorCursor('#hover', id);
201203
this.titleService.setTitle({
202204
level: sector.level,
203205
dimension: sector.dimension,
204206
// subdimension: sector.subdimension, // Sector interface doesn't have subdimension
205207
});
206208
} else {
207-
this.heatmapService.setSectorCursor('#hover', '');
209+
this.heatmapRenderer.setSectorCursor('#hover', '');
208210
}
209211
},
210212
onMouseOut: () => {
211-
this.heatmapService.setSectorCursor('#hover', '');
213+
this.heatmapRenderer.setSectorCursor('#hover', '');
212214
this.titleService.clearTitle();
213215
},
214216
},
@@ -451,7 +453,7 @@ export class CircularHeatmapComponent implements OnInit, OnDestroy {
451453
})`
452454
);
453455

454-
this.heatmapService.recolorSector(index, progressValue);
456+
this.heatmapRenderer.recolorSector(index, progressValue);
455457
}
456458

457459
exportTeamProgress() {
Lines changed: 51 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,15 @@ export interface HeatmapConfig {
2121
@Injectable({
2222
providedIn: 'root',
2323
})
24-
export class D3HeatmapService {
25-
private svg: d3.Selection<SVGGElement, any, HTMLElement, any> | null = null;
26-
private config!: HeatmapConfig;
24+
export class D3HeatmapRenderer {
25+
private svg: d3.Selection<SVGGElement, unknown, HTMLElement, unknown> | null = null;
26+
private config: HeatmapConfig | null = null;
2727
private segmentHeight!: number;
2828
private innerRadius!: number;
2929
private segmentLabelHeight!: number;
3030
private outerRadius!: number;
3131
private colorScale!: d3.ScaleLinear<string, string>;
32+
private dataset: Sector[] = [];
3233

3334
public initialize(
3435
domElement: string,
@@ -72,7 +73,7 @@ export class D3HeatmapService {
7273
`translate(${margin.left + this.segmentLabelHeight}, ${
7374
margin.top + this.segmentLabelHeight
7475
})`
75-
) as any;
76+
) as d3.Selection<SVGGElement, unknown, HTMLElement, unknown>;
7677
}
7778

7879
public render(
@@ -84,7 +85,10 @@ export class D3HeatmapService {
8485
},
8586
sectorProgressFn: (sector: Sector) => number
8687
): void {
87-
if (!this.svg) return;
88+
if (!this.svg || !this.config) return;
89+
90+
// Store the dataset reference for later use
91+
this.dataset = dataset;
8892

8993
const { dimLabels, colors } = this.config;
9094
const numSegments = dimLabels.length;
@@ -119,13 +123,38 @@ export class D3HeatmapService {
119123
}
120124
return this.colorScale(sectorProgressFn(d));
121125
})
122-
.on('click', (event, d) => {
123-
const index = dataset.indexOf(d);
124-
handlers.onClick(d, index, 'index-' + index);
126+
.on('click', (event: MouseEvent, d: Sector) => {
127+
// Use the stored dataset to find the index
128+
const index = this.dataset.indexOf(d);
129+
if (index !== -1) {
130+
handlers.onClick(d, index, 'index-' + index);
131+
} else {
132+
// Fallback: find by dimension and level
133+
const fallbackIndex = this.dataset.findIndex(
134+
sector => sector.dimension === d.dimension && sector.level === d.level
135+
);
136+
if (fallbackIndex !== -1) {
137+
handlers.onClick(this.dataset[fallbackIndex], fallbackIndex, 'index-' + fallbackIndex);
138+
}
139+
}
125140
})
126-
.on('mouseover', (event, d) => {
127-
const index = dataset.indexOf(d);
128-
handlers.onMouseOver(d, index, 'index-' + index);
141+
.on('mouseover', (event: MouseEvent, d: Sector) => {
142+
const index = this.dataset.indexOf(d);
143+
if (index !== -1) {
144+
handlers.onMouseOver(d, index, 'index-' + index);
145+
} else {
146+
// Fallback: find by dimension and level
147+
const fallbackIndex = this.dataset.findIndex(
148+
sector => sector.dimension === d.dimension && sector.level === d.level
149+
);
150+
if (fallbackIndex !== -1) {
151+
handlers.onMouseOver(
152+
this.dataset[fallbackIndex],
153+
fallbackIndex,
154+
'index-' + fallbackIndex
155+
);
156+
}
157+
}
129158
})
130159
.on('mouseout', () => {
131160
handlers.onMouseOut();
@@ -194,7 +223,7 @@ export class D3HeatmapService {
194223
}
195224

196225
public recolorSector(index: number, progressValue: number): void {
197-
if (!this.svg) return;
226+
if (!this.svg || !this.config) return;
198227

199228
this.svg
200229
.select('#index-' + index)
@@ -205,10 +234,20 @@ export class D3HeatmapService {
205234
}
206235

207236
public updateThemeColors(colors: HeatmapColors): void {
237+
// Guard against being called before initialize()
238+
if (!this.config) {
239+
console.warn('D3HeatmapRenderer: updateThemeColors called before initialize, skipping.');
240+
return;
241+
}
242+
208243
this.config.colors = colors;
209244
this.colorScale.range([colors.background, colors.filled]);
210245
if (this.svg) {
211246
this.svg.selectAll('.circular-heat path').attr('stroke', colors.stroke);
212247
}
213248
}
249+
250+
public isInitialized(): boolean {
251+
return this.config !== null && this.svg !== null;
252+
}
214253
}

0 commit comments

Comments
 (0)