Skip to content
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.

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

[ENT-9538]: Make analytics V2 skills scatter chart bubble marker size consistent with V1 #1324

Merged
merged 1 commit into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 src/components/AdvanceAnalyticsV2/charts/ChartWrapper.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ ChartWrapper.propTypes = {
hovertemplate: PropTypes.string.isRequired,
xAxisTitle: PropTypes.string,
yAxisTitle: PropTypes.string,
markerSizeKey: PropTypes.string,
markerSizes: PropTypes.arrayOf(PropTypes.number), // An array of sizes for the markers.
customDataKeys: PropTypes.arrayOf(PropTypes.string),
}).isRequired,
loadingMessage: PropTypes.string.isRequired,
Expand Down
12 changes: 7 additions & 5 deletions src/components/AdvanceAnalyticsV2/charts/ScatterChart.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ import messages from '../messages';
* @param {string} hovertemplate - A template for the hover text.
* @param {string} xAxisTitle - The title for the x-axis.
* @param {string} yAxisTitle - The title for the y-axis.
* @param {string} markerSizeKey - Key for the marker size values in the data objects.
* @param {number[]} markerSizes - An array of sizes for the markers.
* @param {string[]} customDataKeys - Array of keys for custom data to be included in the hover template.
*
* @returns The rendered Plotly scatter chart.
*/
const ScatterChart = ({
data, xKey, yKey, colorKey, colorMap, hovertemplate, xAxisTitle, yAxisTitle, markerSizeKey, customDataKeys,
data, xKey, yKey, colorKey, colorMap, hovertemplate, xAxisTitle, yAxisTitle, markerSizes, customDataKeys,
}) => {
const intl = useIntl();
const categories = Object.keys(colorMap);
Expand All @@ -36,12 +36,14 @@ const ScatterChart = ({
name: messages[category] ? intl.formatMessage(messages[category]) : category,
marker: {
color: colorMap[category],
size: filteredData.map(item => item[markerSizeKey] * 0.015).map(size => (size < 5 ? size + 6 : size)),
size: filteredData.map((item, index) => markerSizes[index]), // Use the pre-calculated sizes from props
sizeref: 0.2,
sizemode: 'area',
},
customdata: customDataKeys.length ? filteredData.map(item => customDataKeys.map(key => item[key])) : [],
hovertemplate,
};
}), [data, xKey, yKey, colorKey, colorMap, hovertemplate, categories, markerSizeKey, customDataKeys, intl]);
}), [data, xKey, yKey, colorKey, colorMap, hovertemplate, categories, markerSizes, customDataKeys, intl]);

const layout = {
margin: { t: 0 },
Expand Down Expand Up @@ -87,7 +89,7 @@ ScatterChart.propTypes = {
hovertemplate: PropTypes.string.isRequired,
xAxisTitle: PropTypes.string,
yAxisTitle: PropTypes.string.isRequired,
markerSizeKey: PropTypes.string.isRequired,
markerSizes: PropTypes.arrayOf(PropTypes.number).isRequired,
customDataKeys: PropTypes.arrayOf(PropTypes.string),
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe('ScatterChart', () => {
hovertemplate,
xAxisTitle: 'X Axis',
yAxisTitle: 'Y Axis',
markerSizeKey: 'weight',
markerSizes: [6.045, 6.075],
customDataKeys: ['category'],
};

Expand All @@ -44,7 +44,6 @@ describe('ScatterChart', () => {
expect(traces[0].marker.color).toBe('red');
expect(traces[1].marker.color).toBe('blue');
expect(traces[0].marker.size).toEqual([6.045]);
expect(traces[1].marker.size).toEqual([6.075]);
expect(traces[0].customdata[0]).toEqual(['A']);
expect(traces[1].customdata[0]).toEqual(['B']);
traces.forEach(trace => {
Expand Down
29 changes: 29 additions & 0 deletions src/components/AdvanceAnalyticsV2/data/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,3 +193,32 @@
}
return Object.values(modifiedData);
};

/**
* Calculates the marker sizes for a set of data objects based on a specified numeric property.
* Marker sizes are mapped to a range between a minimum and maximum size.
*
* @param {Array<Object>} dataArray - An array of objects containing the data.
* @param {string} property - The key of the numeric property used to calculate the sizes (e.g., 'completions').
* @param {number} [minSize=10] - The minimum marker size in pixels.
* @param {number} [maxSize=60] - The maximum marker size in pixels.
* @returns {Array<number>} - An array of marker sizes corresponding to the values of the specified property.
*/
export const calculateMarkerSizes = (dataArray = [], property, minSize = 10, maxSize = 60) => {
if (!dataArray.length || !property) {
return [];
}

const propertyValues = dataArray.map(d => d[property]);
const minValue = Math.min(...propertyValues);
const maxValue = Math.max(...propertyValues);

return dataArray.map(item => {
if (maxValue - minValue === 0) {
return minSize; // Avoid division by zero if all values are the same

Check warning on line 218 in src/components/AdvanceAnalyticsV2/data/utils.js

View check run for this annotation

Codecov / codecov/patch

src/components/AdvanceAnalyticsV2/data/utils.js#L218

Added line #L218 was not covered by tests
}

// Scale the marker size between minSize and maxSize based on the specified property
return ((item[property] - minValue) / (maxValue - minValue)) * (maxSize - minSize) + minSize;
});
};
34 changes: 33 additions & 1 deletion src/components/AdvanceAnalyticsV2/data/utils.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// Jest test for utils.js

import { createIntl } from '@edx/frontend-platform/i18n';
import { applyCalculation, applyGranularity, constructChartHoverTemplate } from './utils';
import {
applyCalculation, applyGranularity, constructChartHoverTemplate, calculateMarkerSizes,
} from './utils';
import { CALCULATION, GRANULARITY } from './constants';

describe('utils', () => {
Expand Down Expand Up @@ -201,6 +203,36 @@ describe('utils', () => {
]);
});
});
describe('calculateMarkerSizes', () => {
it('should calculate correct marker sizes based on a property', () => {
const data = {
topSkills: [
{
skill_name: 'Test Skill 1',
completions: 5,
},
{
skill_name: 'Test Skill 2',
completions: 10,
},
{
skill_name: 'Test Skill 3',
completions: 15,
},
],
};
const expectedSizes = [10, 35, 60];

const result = calculateMarkerSizes(data?.topSkills, 'completions');

expect(result).toEqual(expectedSizes);
});

it('should return empty array if data is empty', () => {
const result = calculateMarkerSizes([], 'completions');
expect(result).toEqual([]);
});
});
});

describe('constructChartHoverTemplate', () => {
Expand Down
6 changes: 4 additions & 2 deletions src/components/AdvanceAnalyticsV2/tabs/Skills.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
} from '../data/constants';
import { useEnterpriseAnalyticsData } from '../data/hooks';
import ChartWrapper from '../charts/ChartWrapper';
import { constructChartHoverTemplate } from '../data/utils';
import { constructChartHoverTemplate, calculateMarkerSizes } from '../data/utils';
import DownloadCSVButton from '../DownloadCSVButton';

const Skills = ({ startDate, endDate, enterpriseId }) => {
Expand All @@ -22,6 +22,8 @@ const Skills = ({ startDate, endDate, enterpriseId }) => {
endDate,
});

const markerSizes = calculateMarkerSizes(data?.topSkills, 'completions');

return (
<div className="tab-skills mt-4">
<div className="top-skill-chart-container mb-4">
Expand Down Expand Up @@ -64,7 +66,7 @@ const Skills = ({ startDate, endDate, enterpriseId }) => {
defaultMessage: 'Completions',
description: 'Y-axis title for the top skills chart.',
}),
markerSizeKey: 'completions',
markerSizes, // Pass marker sizes directly to ScatterChart
customDataKeys: ['skillName', 'skillType'],
hovertemplate: constructChartHoverTemplate(intl, {
skill: '%{customdata[0]}',
Expand Down