Skip to content

Commit

Permalink
Merge pull request #53 from pfizer-opensource/cfd-tooltip-fixes
Browse files Browse the repository at this point in the history 
Fix the CFD tooltip
  • Loading branch information
@ClaudiaGivan
ClaudiaGivan authored Nov 29, 2024
2 parents 1168e75 + 30801f5 commit f50e233
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 34 deletions.
3 changes: 1 addition & 2 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,14 @@
"peerDependencies": {
"css-loader": "^6.8.1",
"style-loader": "^3.3.3"

},
"devDependencies": {
"css-loader": "^6.8.1",
"eslint": "^8.44.0",
"eslint-config-prettier": "^8.8.0",
"eslint-plugin-prettier": "^4.2.1",
"jest": "^29.7.0",
"prettier": "^3.0.0",
"css-loader": "^6.8.1",
"style-loader": "^3.3.3"
}
}
89 changes: 62 additions & 27 deletions src/graphs/cfd/CFDRenderer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ class CFDRenderer extends UIControlsRenderer {
[this.width, this.focusHeight - this.margin.top + 1],
])
.on('brush', ({ selection }) => {
this.selectedTimeRange = selection.map(this.x.invert, this.x);
this.selectedTimeRange = selection.map(this.x?.invert, this.x);
this.updateGraph(this.selectedTimeRange);
if (this.isManualBrushUpdate && this.eventBus) {
this.eventBus?.emitEvents(`change-time-range-${this.chartName}`, this.selectedTimeRange);
Expand Down Expand Up @@ -160,7 +160,7 @@ class CFDRenderer extends UIControlsRenderer {
this.drawYAxis(this.gy, this.currentYScale);

this.chartArea
.selectAll('path')
?.selectAll('path')
.attr('class', (d) => 'area ' + d.key)
.style('fill', (d) => this.#statesColors(d.key))
.attr('d', this.#createAreaGenerator(this.currentXScale, this.currentYScale));
Expand Down Expand Up @@ -193,7 +193,7 @@ class CFDRenderer extends UIControlsRenderer {
#drawArea() {
this.chartArea = this.addClipPath(this.svg, `${this.chartName}-clip`);
this.chartArea
.append('rect')
?.append('rect')
.attr('width', '100%')
.attr('height', '100%')
.attr('id', `${this.chartName}-area`)
Expand Down Expand Up @@ -412,7 +412,7 @@ class CFDRenderer extends UIControlsRenderer {
const triangleBase = 11;
const trianglePath = `M${-triangleBase / 2},0 L${triangleBase / 2},0 L0,-${triangleHeight} Z`;
this.chartArea
.selectAll('observations')
?.selectAll('observations')
.data(observations?.data?.filter((d) => d.chart_type === this.chartType))
.join('path')
.attr('class', 'observation-marker')
Expand All @@ -436,14 +436,49 @@ class CFDRenderer extends UIControlsRenderer {
*/
#showTooltipAndMovingLine(event) {
!this.tooltip && this.#createTooltipAndMovingLine(event.lineX, event.lineY);
const tooltipWidth = this.tooltip.node().getBoundingClientRect().width;
const cfdGraphRect = d3.select(this.graphElementSelector).node().getBoundingClientRect();
const tooltipTop = window.scrollY + cfdGraphRect.top - 50;
let { tooltipWidth, tooltipTop } = this.computeTooltipWidthAndTop(event);

this.#clearTooltipAndMovingLine(event.lineX, event.lineY);
this.#positionTooltip(event.tooltipLeft, tooltipTop, tooltipWidth);
this.#populateTooltip(event);
}

computeTooltipWidthAndTop(event) {
const tooltipRect = this.tooltip.node().getBoundingClientRect();
const tooltipWidth = tooltipRect.width;
const tooltipHeight = tooltipRect.height;
const graphRect = d3.select(this.graphElementSelector).node().getBoundingClientRect();
const padding = 10;
let tooltipLeft = event.tooltipLeft;
let tooltipTop = event.lineY - tooltipHeight - padding;

// Get viewport dimensions
const viewportWidth = window.innerWidth;
const viewportHeight = window.innerHeight;

// Adjust tooltipLeft to prevent overflow on the right
if (tooltipLeft + tooltipWidth + padding > viewportWidth) {
tooltipLeft = viewportWidth - tooltipWidth - padding;
}

// Adjust tooltipLeft to prevent overflow on the left
if (tooltipLeft < padding) {
tooltipLeft = padding;
}

// Adjust tooltipTop to prevent overflow on the top
if (tooltipTop < graphRect.top) {
// Position the tooltip below the event point if there's not enough space above
tooltipTop = event.lineY + padding;

// Ensure the tooltip doesn't overflow the bottom of the viewport
if (tooltipTop + tooltipHeight + padding > viewportHeight) {
tooltipTop = viewportHeight - tooltipHeight - padding;
}
}
return { tooltipWidth, tooltipTop };
}

/**
* Hides the tooltip and the moving line on the chart.
*/
Expand All @@ -460,9 +495,9 @@ class CFDRenderer extends UIControlsRenderer {
* @private
*/
#createTooltipAndMovingLine(x, y) {
this.tooltip = d3.select('body').append('div').attr('class', styles.tooltip).attr('id', 'c-tooltip').style('opacity', 0);
this.tooltip = d3.select('body').append('div').attr('class', styles.chartTooltip).attr('id', 'c-tooltip').style('opacity', 0);
this.cfdLine = this.chartArea
.append('line')
?.append('line')
.attr('id', `${this.chartName}-line`)
.attr('stroke', 'black')
.attr('y1', 0)
Expand All @@ -481,7 +516,7 @@ class CFDRenderer extends UIControlsRenderer {
*/
#positionTooltip(left, top, width) {
this.tooltip?.transition().duration(100).style('opacity', 0.9).style('pointer-events', 'auto');
this.tooltip?.style('left', left - width + 'px').style('top', top + 30 + 'px');
this.tooltip?.style('left', left - width + 'px').style('top', top + 'px');
}

/**
Expand All @@ -492,7 +527,7 @@ class CFDRenderer extends UIControlsRenderer {
#populateTooltip(event) {
this.tooltip?.append('p').text(formatDateToLocalString(event.date)).attr('class', 'text-center');
const gridContainer = this.tooltip?.append('div').attr('class', 'grid grid-cols-2');
if (event.metrics.averageCycleTime > 0) {
if (event.metrics?.averageCycleTime > 0) {
gridContainer
.append('span')
.text('Cycle time:')
Expand All @@ -501,12 +536,12 @@ class CFDRenderer extends UIControlsRenderer {
.style('color', this.#cycleTimeColor);
gridContainer
.append('span')
.text(`${event.metrics.averageCycleTime} days`)
.text(`${event.metrics?.averageCycleTime} days`)
.attr('class', 'pl-1')
.style('text-align', 'start')
.style('color', this.#cycleTimeColor);
}
if (event.metrics.averageLeadTime > 0) {
if (event.metrics?.averageLeadTime > 0) {
gridContainer
.append('span')
.text('Lead time:')
Expand Down Expand Up @@ -566,8 +601,8 @@ class CFDRenderer extends UIControlsRenderer {
return; // Exit the function if metrics are already enabled
}
this.#areMetricsEnabled = true;
this.chartArea.on('mousemove', (event) => this.#handleMouseEvent(event, `${this.chartName}-mousemove`));
this.chartArea.on('click', (event) => this.#handleMouseEvent(event, `${this.chartName}-click`));
this.chartArea?.on('mousemove', (event) => this.#handleMouseEvent(event, `${this.chartName}-mousemove`));
this.chartArea?.on('click', (event) => this.#handleMouseEvent(event, `${this.chartName}-click`));
this.#setupMouseLeaveHandler();
}

Expand All @@ -590,8 +625,8 @@ class CFDRenderer extends UIControlsRenderer {
return;
}

const date = this.currentXScale.invert(xPosition);
const cumulativeCountOfWorkItems = this.currentYScale.invert(yPosition);
const date = this.currentXScale?.invert(xPosition);
const cumulativeCountOfWorkItems = this.currentYScale?.invert(yPosition);
const excludeCycleTime = eventName.includes('mousemove') && !eventName.includes(this.chartName);

const metrics = this.computeMetrics(date, Math.floor(cumulativeCountOfWorkItems), excludeCycleTime);
Expand Down Expand Up @@ -619,7 +654,7 @@ class CFDRenderer extends UIControlsRenderer {
* @private
*/
#setupMouseLeaveHandler() {
this.chartArea.on('mouseleave', () => this.hideTooltipAndMovingLine());
this.chartArea?.on('mouseleave', () => this.hideTooltipAndMovingLine());
}

/**
Expand Down Expand Up @@ -729,8 +764,8 @@ class CFDRenderer extends UIControlsRenderer {
* @private
*/
#drawMetricLines({
averageCycleTime,
averageLeadTime,
averageCycleTime = '',
averageLeadTime = '',
leadTimeDateBefore,
cycleTimeDateBefore,
currentDate,
Expand Down Expand Up @@ -769,12 +804,12 @@ class CFDRenderer extends UIControlsRenderer {
* @private
*/
#drawHorizontalMetricLine(dateBefore, dateAfter, noOfItems, cssClass, color, width) {
this.chartArea.selectAll(`.${cssClass}`).remove();
this.chartArea?.selectAll(`.${cssClass}`).remove();
const x1 = this.currentXScale(dateBefore);
const x2 = this.currentXScale(dateAfter);
const y = this.currentYScale(noOfItems);
this.chartArea
.append('line')
?.append('line')
.attr('x1', x1)
.attr('y1', y)
.attr('x2', x2)
Expand All @@ -795,12 +830,12 @@ class CFDRenderer extends UIControlsRenderer {
* @private
*/
#drawVerticalMetricLine(date, noOfItemsBefore, noOfItemsAfter, cssClass, color) {
this.chartArea.selectAll(`.${cssClass}`).remove();
this.chartArea?.selectAll(`.${cssClass}`).remove();
const y1 = this.currentYScale(noOfItemsBefore);
const y2 = this.currentYScale(noOfItemsAfter);
const x = this.currentXScale(date);
this.chartArea
.append('line')
?.append('line')
.attr('x1', x)
.attr('y1', y1)
.attr('x2', x)
Expand All @@ -815,9 +850,9 @@ class CFDRenderer extends UIControlsRenderer {
* @private
*/
#removeMetricsLines() {
this.chartArea.selectAll('.wip-line').remove();
this.chartArea.selectAll('.cycle-time-line').remove();
this.chartArea.selectAll('.lead-time-line').remove();
this.chartArea?.selectAll('.wip-line').remove();
this.chartArea?.selectAll('.cycle-time-line').remove();
this.chartArea?.selectAll('.lead-time-line').remove();
}

/**
Expand Down
6 changes: 3 additions & 3 deletions src/graphs/scatterplot/ScatterplotRenderer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ class ScatterplotRenderer extends UIControlsRenderer {
* @param {Object} observations - Observations data for the renderer.
*/
setupObservationLogging(observations) {
if (observations.data.length > 0) {
if (observations?.data?.length > 0) {
this.displayObservationMarkers(observations);
this.enableMetrics();
}
Expand All @@ -457,7 +457,7 @@ class ScatterplotRenderer extends UIControlsRenderer {
* @private
*/
#removeObservationMarkers() {
this.chartArea.selectAll('.ring').remove();
this.chartArea?.selectAll('.ring')?.remove();
}

/**
Expand Down Expand Up @@ -510,7 +510,7 @@ class ScatterplotRenderer extends UIControlsRenderer {
* @private
*/
#createTooltip() {
this.tooltip = d3.select('body').append('div').attr('class', styles.tooltip).attr('id', 's-tooltip').style('opacity', 0);
this.tooltip = d3.select('body').append('div').attr('class', styles.chartTooltip).attr('id', 's-tooltip').style('opacity', 0);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/graphs/tooltipStyles.module.css
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.tooltip {
.chartTooltip {
position: absolute;
text-align: center;
width: max-content;
Expand All @@ -11,7 +11,7 @@
z-index: 1;
}

.tooltip p {
.chartTooltip p {
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
Expand Down

0 comments on commit f50e233

Please sign in to comment.