Enhancing GitHub Heatmap with Interactive Tooltips and Clickable Cells #1271
Conversation
Summary by CodeRabbit
WalkthroughThis pull request updates the GitHub heatmap utility by enhancing type safety, error handling, and tooltip functionality. It introduces new interfaces for structured data, improves error messaging, and ensures proper initialization of data structures. The tooltip feature is implemented to display contribution details on hover, and color themes have been updated for better visual representation. Additionally, the tracking of cell positions is introduced to manage tooltip visibility effectively. Changes
Assessment against linked issues
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
frontend/src/utils/helpers/githubHeatmap.ts (7)
14-14: Consider using date manipulation libraries for precise year calculationWhile the current approach of
startDate.setDate(endDate.getDate() - 7 * 52 - 1)works for setting the start date to approximately one year before the end date, consider using a more precise method like the date-fns library you're already importing to handle edge cases like leap years.-startDate.setDate(endDate.getDate() - 7 * 52 - 1) +import { subYears } from 'date-fns/subYears' +const startDate = subYears(endDate, 1)
23-25: Ensure intensity thresholds align with legend labelsYou've modified the intensity thresholds, but later in the legend (lines 298-303), there's a slight inconsistency for the highest threshold. The intensity function defines values >10 as grade4, but the legend labels grade4 as "10+", which is ambiguous.
Consider updating either the thresholds or the legend labels to be consistent:
- if (count <= 10) return '3' + if (count < 10) return '3'Or alternatively update the label in the legend to "11+" for consistency.
85-94: Consider extracting common theme propertiesThe newly added
blueRedtheme shares many properties with thebluetheme. Consider creating a base theme object to reduce duplication.const baseTheme = { background: '#1a2233', text: '#FFFFFF', meta: '#A6B1C1', grade0: '#202A37', grade1: '#46627b', grade2: '#5f87a8', grade3: '#4682b4', } const themes = { blue: { ...baseTheme, grade4: '#2a70d8', }, blueRed: { ...baseTheme, grade4: '#e64a4a', } }
129-129: Consider adding a default value for showTooltipsThe new
showTooltipsproperty has been added without a default value, which could lead to inconsistent behavior if not explicitly set.Consider providing a default value in the implementation:
interface Options { themeName?: keyof typeof themes customTheme?: Theme skipHeader?: boolean skipAxisLabel?: boolean username: string data: DataStruct fontFace?: string footerText?: string theme?: string - showTooltips?: boolean + showTooltips?: boolean } function getTheme(opts: Options): Theme { const { themeName } = opts const name = themeName ?? 'blue' return themes[name] ?? themes.blue } +// Add a helper to get the showTooltips option with a default value +function getShowTooltips(opts: Options): boolean { + return opts.showTooltips ?? false +}
216-217: Update function signature to reflect return value changeThe
drawYearfunction has been modified to collect and return cell data, but the function signature hasn't been updated to reflect this change. Consider adding a return type to make the function's behavior more predictable.-function drawYear(ctx: CanvasRenderingContext2D, opts: DrawYearOptions) { +interface CellData { + x: number; + y: number; + width: number; + height: number; + date: string; + count: number; +} + +function drawYear(ctx: CanvasRenderingContext2D, opts: DrawYearOptions): CellData[] {Also applies to: 243-252, 269-269
372-409: Consider performance optimizations for event handlersThe current mousemove event handler iterates through all cells on every mouse movement, which could be inefficient for large datasets.
Consider using a spatial data structure like a quadtree to improve performance, or at minimum, use early returns when a match is found:
canvas.addEventListener('mousemove', (e) => { // ...existing code... let found = false; - allCellsData.forEach(cell => { + for (const cell of allCellsData) { if ( x >= cell.x && x <= cell.x + cell.width && y >= cell.y && y <= cell.y + cell.height ) { // ...existing tooltip code... found = true; + break; // Exit the loop once we've found a match } - }) + } // ...existing code... });
412-435: Document the custom event for better API usabilityThe custom event 'heatmap-cell-click' is dispatched when a cell is clicked, but there's no documentation on how consumers should listen for this event.
Consider adding a JSDoc comment to document this feature:
+/** + * Draws GitHub-style contribution heatmap on the provided canvas element. + * + * @param canvas - The canvas element to draw on + * @param opts - Configuration options + * + * @example + * // Listen for cell click events + * canvas.addEventListener('heatmap-cell-click', (e: CustomEvent) => { + * const { date, count } = e.detail; + * console.log(`Clicked on ${date} with ${count} contributions`); + * }); + */ export function drawContributions(canvas: HTMLCanvasElement, opts: Options) {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/utils/helpers/githubHeatmap.ts(10 hunks)
🔇 Additional comments (4)
frontend/src/utils/helpers/githubHeatmap.ts (4)
76-83: Great improvements to the color scaleThe updated blue theme with more distinct color grades provides better visual contrast and helps users differentiate between activity levels more easily.
160-164: Box dimensions improvementsIncreasing the box dimensions and header height provides better visual spacing and accommodates the new legend. The consistent use of these constants throughout the code is good practice.
281-305: Great addition of the legend visualizationThe legend clearly shows what each color represents and helps users interpret the data correctly. This is a valuable enhancement to the visualization.
However, as mentioned earlier, ensure the legend labels accurately match the intensity thresholds defined in the
getIntensityfunction for consistency.
342-356: Good implementation of cell data collectionThe approach to collecting cell data from all years is well-implemented. The code checks if the cellsData returned from drawYear exists before adding it to the allCellsData array, which prevents potential errors.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
frontend/src/utils/helpers/githubHeatmap.ts (3)
300-300: Consider adding a return type annotationThe function is returning cell data, but the return type is not explicitly defined.
-function drawYear(ctx: CanvasRenderingContext2D, opts: DrawYearOptions) { +function drawYear(ctx: CanvasRenderingContext2D, opts: DrawYearOptions): Array<{x: number, y: number, width: number, height: number, date: string, count: number}> {
437-461: Consider performance optimization for event handlersWhile the click event implementation works well, there's potential for optimization. Instead of looping through all cells on every click, consider using a more efficient data structure like a grid or quadtree for hit detection with large datasets.
// At the top of the file, add this import + import { quadtree } from 'd3-quadtree'; // Then modify the click handler if (opts.showTooltips && typeof window !== 'undefined') { const tooltip = getOrCreateTooltip(); + // Create a quadtree for efficient hit detection + const cellQuadtree = quadtree() + .x(d => d.x + d.width/2) + .y(d => d.y + d.height/2) + .addAll(allCellsData); // ... mousemove and mouseout handlers ... // Make cells clickable canvas.addEventListener('click', (e) => { const rect = canvas.getBoundingClientRect(); const x = (e.clientX - rect.left) / scaleFactor; const y = (e.clientY - rect.top) / scaleFactor; + // Find the nearest cell within the click radius + const radius = Math.max(boxWidth, boxMargin) * 2; + const cell = cellQuadtree.find(x, y, radius); + + if (cell && + x >= cell.x && x <= cell.x + cell.width && + y >= cell.y && y <= cell.y + cell.height) { + // Dispatch custom event with cell data + const event = new CustomEvent('heatmap-cell-click', { + detail: { + date: cell.date, + count: cell.count + } + }); + canvas.dispatchEvent(event); + } }); }
417-420: Consider adding accessibility attributes to tooltipThe tooltip could benefit from improved accessibility.
tooltip.innerHTML = ` - <div>${date}</div> - <div>${cell.count} contribution${cell.count !== 1 ? 's' : ''}</div> + <div role="tooltip" aria-live="polite"> + <div>${date}</div> + <div>${cell.count} contribution${cell.count !== 1 ? 's' : ''}</div> + </div> `
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/utils/helpers/githubHeatmap.ts(9 hunks)
🔇 Additional comments (9)
frontend/src/utils/helpers/githubHeatmap.ts (9)
21-27: Improved intensity scale with better distributionThe adjusted thresholds create a more balanced distribution of intensity levels, making the visualization more informative. This aligns well with the PR objective of enhancing the color scale for better differentiation of activity levels.
74-95: Great enhancement with theme improvements and addition of blueRed themeThe updated blue theme with better contrast and the new blueRed theme provide more visual options and improve readability. The use of red for highest intensity in the blueRed theme creates a clear visual distinction for high activity days.
130-130: Proper interface extension for tooltipsThe addition of the
showTooltipsoption to the interface is clean and follows good TypeScript practices by making it optional with the?modifier.
160-166: Improved spacing and dimensions for better visibilityThe increased box dimensions and spacing improve the overall readability of the heatmap. The header height increase accommodates the new legend.
170-198: Well-implemented tooltip managementThe singleton pattern for tooltip management is a good approach, addressing previous review feedback. The cleanup function is particularly valuable for preventing memory leaks.
246-284: Efficient data collection for interactive featuresThe code efficiently collects cell data for tooltips and click events. The conditional check for
opts.showTooltipsensures this only happens when needed, which is good for performance.
309-347: Clear and informative legend implementationThe implementation of the legend with color boxes and labels greatly improves user understanding of the heatmap. The labels accurately reflect the intensity thresholds defined earlier.
378-392: Effective management of cell data collectionThe code properly collects and manages cell data from all years, which is essential for the tooltip and click functionality. The conditional check ensures backwards compatibility.
394-436: Well-implemented tooltip functionalityThe tooltip implementation provides a good user experience with smooth appearance/disappearance and clear formatting. The event listeners are properly added and the tooltip positioning is user-friendly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
frontend/src/utils/helpers/githubHeatmap.ts (4)
404-472: Consider optimizing hit detection for better performanceWhile the event handling implementation is good, the hit detection in both mouseMove and click handlers uses a linear search through all cells, which could be inefficient for large heatmaps.
Consider optimizing the hit detection algorithm by using a spatial data structure or grid-based approach:
let allCellsData = [] + // Create a map for faster cell lookup + const cellsByPosition = new Map() // When collecting cell data if (cellsData) { allCellsData = [...allCellsData, ...cellsData] + // Also store cells in a map using row/column as key + cellsData.forEach(cell => { + // Calculate grid position (row, col) + const col = Math.floor(cell.x / (boxWidth + boxMargin)) + const row = Math.floor(cell.y / (boxWidth + boxMargin)) + cellsByPosition.set(`${row},${col}`, cell) + }) } // Then in hit detection handleMouseMove = (e) => { // ...existing code... - let found = false - allCellsData.forEach(cell => { - if ( - x >= cell.x && - x <= cell.x + cell.width && - y >= cell.y && - y <= cell.y + cell.height - ) { - // ... tooltip code ... - } - }) + // Calculate which cell we're hovering over + const col = Math.floor(x / (boxWidth + boxMargin)) + const row = Math.floor(y / (boxWidth + boxMargin)) + const cell = cellsByPosition.get(`${row},${col}`) + + if (cell) { + // ... tooltip code ... + found = true + } }This approach is more efficient for large datasets as it replaces the O(n) linear search with an O(1) lookup.
457-465: Consider adding more information to the click eventThe click event currently provides the date and count, but you could enhance its utility by including additional information such as the intensity level.
const event = new CustomEvent('heatmap-cell-click', { detail: { date: cell.date, count: cell.count + intensity: getIntensity(cell.count), + position: { x: cell.x, y: cell.y } } })
404-439: Use TypeScript type safety for event parametersThe event handlers are lacking proper TypeScript type annotations for the event parameter.
-handleMouseMove = (e) => { +handleMouseMove = (e: MouseEvent) => { const rect = canvas.getBoundingClientRect() // rest of the code }
419-428: Consider a more accessible tooltip formatThe current tooltip design is simple but could be enhanced for better accessibility and user experience.
tooltip.innerHTML = ` - <div>${date}</div> - <div>${cell.count} contribution${cell.count !== 1 ? 's' : ''}</div> + <div style="font-weight: bold;">${date}</div> + <div> + <span style="color: ${getTheme(opts)[`grade${getIntensity(cell.count)}`]}">■</span> + ${cell.count} contribution${cell.count !== 1 ? 's' : ''} + </div> `This format includes the color indicator matching the cell's color and improves readability with bold styling for the date.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/utils/helpers/githubHeatmap.ts(9 hunks)
🔇 Additional comments (8)
frontend/src/utils/helpers/githubHeatmap.ts (8)
21-27: Well-tuned intensity scale for better differentiationThe modified thresholds now provide better contrast between the intensity levels, making it easier to distinguish between different contribution counts. The new scale aligns well with the legend labels added later in the code.
77-85: Improved blue theme with better contrastThe updated blue theme with a new background color and more distinct grade colors provides better visual contrast, making the heatmap easier to read.
86-95: Good addition of the blueRed themeThe new blueRed theme effectively highlights high-contribution days with the red color for the highest intensity, while maintaining the blue gradient for lower intensities. This provides users with more visual options that can help emphasize peak activity days.
130-130: LGTM: Clean interface extensionGood job extending the Options interface with an optional showTooltips property. This follows TypeScript best practices by making the feature opt-in.
161-166: Improved visual spacing and layoutThe increased dimensions for boxWidth, boxMargin, and headerHeight improve the overall layout and readability of the heatmap. The cells are now more distinguishable and the additional header space accommodates the new legend.
171-212: Well-implemented tooltip and event cleanup functionsThe tooltip implementation follows best practices by using a singleton pattern and providing proper cleanup functions. This addresses the concerns raised in previous reviews about potential memory leaks and multiple tooltip instances.
261-262: Good implementation of cell data collectionThe cellsData collection approach is clean and efficient. It gathers only the necessary information for tooltips and click events, and only when the showTooltips option is enabled.
Also applies to: 288-297, 314-314
325-351: Excellent legend implementationThe legend implementation with clear labels corresponding to the intensity thresholds greatly improves the heatmap's usability. Users can now understand exactly what each color represents.
|
@arkid15r any review for this please |
| ctx.fill() | ||
|
|
||
| ctx.fillStyle = theme.text | ||
| let label = '' |
There was a problem hiding this comment.
These are way our of bounds for the heatmap. And I'm not even sure if we want to show this on the heatmap to be honest 🤔
There was a problem hiding this comment.
can you give me some more details that how you want this heatmap to be looked @kasya
There was a problem hiding this comment.
i myself want to change some things like the tooltip for date should appear ,and colours for day to day activities should be a little bit more dfferent that it should be easy to understand, and one bar which represents which colour suggests what. is it sounds good to you?
|
|
i have made some changes can have a look on these @kasya , now i am only changing the colour contrast for different contribution and adding tooltip to heatmap for date specification |
Resolves #1270
improved the color scale with more distinct shades
Added a new theme option "blueRed" that uses red for highest activity levels
Created a comprehensive legend showing contribution counts for each colorPositioned at the top of the chart for better visibility
Clear labels indicate the meaning of each color shade
Implemented tooltip functionality that shows when hovering over cells
Added date and exact contribution count information on hover
Included clickable cells that dispatch custom events with cell data