Skip to content

Fix UserJobEfficiencyComponenet and other dashboard errors #2129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aestoltm wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Fix UserJobEfficiencyComponenet and other dashboard errors #2129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3930216
Fix errors with dashboard components. Also fix bug with UserJobEffici…
aestoltm Dec 3, 2025
d4599d7
Merge branch 'main' into fix_user_efficiency_component
aestoltm Jan 12, 2026
bbc107f
Merge branch 'main' into fix_user_efficiency_component
aestoltm Mar 30, 2026
b490055
Merge branch 'main' into fix_user_efficiency_component
aestoltm Apr 15, 2026
9cbd3db
Merge branch 'main' into fix_user_efficiency_component
aestoltm Apr 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion html/gui/js/PlotlyChartWrapper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
// Format timezone -- Plotly does not support timezones
// Therefore, we need to utilize moment JS.
const axis = baseChartOptions.layout.swapXY ? 'yaxis' : 'xaxis';
if (baseChartOptions.layout[axis].timeseries) {
if (baseChartOptions.layout[axis] && baseChartOptions.layout[axis].timeseries) {

Check warning on line 94 in html/gui/js/PlotlyChartWrapper.js

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Prefer using an optional chain expression instead, as it's more concise and easier to read. 


See more on https://sonarcloud.io/project/issues?id=ubccr_xdmod&issues=AZ0_RVmmO-N_sr9_jBhw&open=AZ0_RVmmO-N_sr9_jBhw&pullRequest=2129
const axisName = axis.slice(0, 1);
baseChartOptions.data.forEach((series) => {
series[axisName].forEach((elem, index, seriesArr) => {
Expand Down
71 changes: 37 additions & 34 deletions html/gui/js/libraries/PlotlyUtilities.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,13 +315,13 @@ function isTopLegend(layout) {
* @return {Object} update - Contains Plotly JS relayout updates and subtitle line count
*/
function adjustSubtitle(layout, subtitleIndex, legendTopCenter, firstRender) {
let prevLineCount = layout.annotations[1].text.match(/<br \/>/g);
let prevLineCount = layout.annotations[`${subtitleIndex}`].text.match(/<br \/>/g);
Copy link
Copy Markdown
Member

@jpwhite4 jpwhite4 Apr 15, 2026

Choose a reason for hiding this comment

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

how come you are converting the subtitleIndex variable to a string to do the array lookup? What is the difference between yyour code and layout.annotations[subtitleIndex]

if (prevLineCount) {
prevLineCount = prevLineCount.length;
} else {
prevLineCount = 1;
}
const subtitle = layout.annotations[1].text.replace(/<br \/>/g, '');
const subtitle = layout.annotations[`${subtitleIndex}`].text.replace(/<br \/>/g, '');
const update = { chartUpdates: {}, subtitleLineCount: 0 };
if (subtitle.length > 0) {
if (!layout.width) {
Expand Down Expand Up @@ -390,19 +390,6 @@ function relayoutChart(chartDiv, adjHeight, firstRender = false, isExport = fals
}
}

const isPie = chartDiv._fullData.length > 0 && chartDiv._fullData[0].type === 'pie';

const subtitleUpdates = adjustSubtitle(chartDiv._fullLayout, subtitleIndex, topCenter, firstRender);
update = subtitleUpdates.chartUpdates;

if (isPie && topCenter && subtitleUpdates.subtitleLineCount > 0) {
extraShift -= 10;
}

if (subtitleUpdates.subtitleLineCount > 0 && firstRender) {
marginTop = subtitleUpdates.chartUpdates['margin.t'];
}

// Handle <br> in title
// Grab the contents inbetween leading and trailing <br> tags
// Eslint throws invalid syntax on regex even though it is valid.
Expand All @@ -415,33 +402,49 @@ function relayoutChart(chartDiv, adjHeight, firstRender = false, isExport = fals
}
}

// Observed inconsistency with margin when subtitle is one line long. Unsure of the cause.
if (lineBreakCount > 0) {
if (firstRender) {
update['margin.t'] = marginTop + (titleHeight * lineBreakCount);
} else if (subtitleUpdates.subtitleLineCount === 1) {
marginTop = subtitleUpdates.chartUpdates['margin.t'] - (titleHeight * lineBreakCount);
} else {
marginTop -= (titleHeight * lineBreakCount);
let subtitleLineCount = 0;
if (subtitleIndex !== -1) {
const subtitleUpdates = adjustSubtitle(chartDiv._fullLayout, subtitleIndex, topCenter, firstRender);
update = subtitleUpdates.chartUpdates;
subtitleLineCount = subtitleUpdates.subtitleLineCount;

if (subtitleLineCount > 0 && firstRender) {
marginTop = subtitleUpdates.chartUpdates['margin.t'];
}
if (topCenter) {
if (subtitleUpdates.subtitleLineCount > 0) {
marginTop += subtitleHeight + 5;
update['legend.y'] -= 0.025;
update['margin.t'] += chartDiv._fullLayout.legend._height + subtitleHeight;

// Observed inconsistency with margin when subtitle is one line long. Unsure of the cause.
Copy link
Copy Markdown
Member

@jpwhite4 jpwhite4 Apr 15, 2026

Choose a reason for hiding this comment

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

This comment is not particularly useful - what is the inconsistency? how does this relate the code that the comment is next to?

if (lineBreakCount > 0) {
if (firstRender) {
Copy link
Copy Markdown
Member

@jpwhite4 jpwhite4 Apr 15, 2026

Choose a reason for hiding this comment

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

please can you explain the logic here. It seems like on the first render you ignore the subtitle settings. How come?

update['margin.t'] = marginTop + (titleHeight * lineBreakCount);
} else if (subtitleLineCount === 1) {
marginTop = subtitleUpdates.chartUpdates['margin.t'] - (titleHeight * lineBreakCount);
} else {
marginTop -= chartDiv._fullLayout.legend._height / 2;
marginTop -= (titleHeight * lineBreakCount);
}
if (topCenter) {
if (subtitleLineCount > 0) {
marginTop += subtitleHeight + 5;
update['legend.y'] -= 0.025;
update['margin.t'] += chartDiv._fullLayout.legend._height + subtitleHeight;
} else {
marginTop -= chartDiv._fullLayout.legend._height / 2;
}
}
}
}

if (topCenter && subtitleUpdates.subtitleLineCount === 0 && !isPie) {
const isPie = chartDiv._fullData.length > 0 && chartDiv._fullData[0].type === 'pie';
if (isPie && topCenter && subtitleLineCount > 0) {
extraShift -= 10;
}

if (topCenter && subtitleLineCount === 0 && !isPie) {
extraShift += 15;
}

marginTop += extraShift;

if ((titleIndex === -1 || chartDiv._fullLayout.annotations[titleIndex].text.length === 0) && subtitleUpdates.subtitleLineCount === 0) {
if ((titleIndex === -1 || chartDiv._fullLayout.annotations[titleIndex].text.length === 0) && subtitleLineCount === 0) {
if (topCenter) {
update['legend.y'] = 1.0;
} else {
Expand All @@ -452,17 +455,17 @@ function relayoutChart(chartDiv, adjHeight, firstRender = false, isExport = fals
const titleYShift = (marginTop + legendHeight) - titleHeight;

if (titleIndex !== -1) {
update[`annotations[${titleIndex}].yshift`] = subtitleUpdates.subtitleLineCount >= 3 ? titleYShift + 5 : titleYShift;
update[`annotations[${titleIndex}].yshift`] = subtitleLineCount >= 3 ? titleYShift + 5 : titleYShift;
}

if (subtitleIndex !== -1) {
update[`annotations[${subtitleIndex}].yshift`] = titleYShift - (subtitleHeight * subtitleUpdates.subtitleLineCount);
update[`annotations[${subtitleIndex}].yshift`] = titleYShift - (subtitleHeight * subtitleLineCount);
}

const marginBottom = chartDiv._fullLayout._size.b;
let pieChartXShift = 0;
if (isPie) {
pieChartXShift = subtitleUpdates.subtitleLineCount > 0 ? 2 : 1;
pieChartXShift = subtitleLineCount > 0 ? 2 : 1;
}

const shiftYDown = marginBottom * -1;
Expand Down