From 9421035eaae000c020e273016445bcacf74fd043 Mon Sep 17 00:00:00 2001 From: Maham Akif Date: Tue, 1 Oct 2024 13:08:47 +0500 Subject: [PATCH] fix: make v2 skills scatter chart marker size consistent with v1 --- .../charts/ChartWrapper.jsx | 2 +- .../charts/ScatterChart.jsx | 12 ++++--- .../charts/ScatterChart.test.jsx | 3 +- .../AdvanceAnalyticsV2/data/utils.js | 29 ++++++++++++++++ .../AdvanceAnalyticsV2/data/utils.test.js | 34 ++++++++++++++++++- .../AdvanceAnalyticsV2/tabs/Skills.jsx | 6 ++-- 6 files changed, 75 insertions(+), 11 deletions(-) diff --git a/src/components/AdvanceAnalyticsV2/charts/ChartWrapper.jsx b/src/components/AdvanceAnalyticsV2/charts/ChartWrapper.jsx index ea00b26b4c..d1c3677a12 100644 --- a/src/components/AdvanceAnalyticsV2/charts/ChartWrapper.jsx +++ b/src/components/AdvanceAnalyticsV2/charts/ChartWrapper.jsx @@ -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, diff --git a/src/components/AdvanceAnalyticsV2/charts/ScatterChart.jsx b/src/components/AdvanceAnalyticsV2/charts/ScatterChart.jsx index d26e53b2b5..2b2426c3de 100644 --- a/src/components/AdvanceAnalyticsV2/charts/ScatterChart.jsx +++ b/src/components/AdvanceAnalyticsV2/charts/ScatterChart.jsx @@ -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); @@ -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 }, @@ -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), }; diff --git a/src/components/AdvanceAnalyticsV2/charts/ScatterChart.test.jsx b/src/components/AdvanceAnalyticsV2/charts/ScatterChart.test.jsx index 39cc252232..d83d36b302 100644 --- a/src/components/AdvanceAnalyticsV2/charts/ScatterChart.test.jsx +++ b/src/components/AdvanceAnalyticsV2/charts/ScatterChart.test.jsx @@ -24,7 +24,7 @@ describe('ScatterChart', () => { hovertemplate, xAxisTitle: 'X Axis', yAxisTitle: 'Y Axis', - markerSizeKey: 'weight', + markerSizes: [6.045, 6.075], customDataKeys: ['category'], }; @@ -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 => { diff --git a/src/components/AdvanceAnalyticsV2/data/utils.js b/src/components/AdvanceAnalyticsV2/data/utils.js index a17b9bd7cf..4a011c54e0 100644 --- a/src/components/AdvanceAnalyticsV2/data/utils.js +++ b/src/components/AdvanceAnalyticsV2/data/utils.js @@ -193,3 +193,32 @@ export const modifyDataToIntroduceEnrollTypeCount = (data, uniqueKey, countKey, } 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} 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} - 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 + } + + // Scale the marker size between minSize and maxSize based on the specified property + return ((item[property] - minValue) / (maxValue - minValue)) * (maxSize - minSize) + minSize; + }); +}; diff --git a/src/components/AdvanceAnalyticsV2/data/utils.test.js b/src/components/AdvanceAnalyticsV2/data/utils.test.js index b0f6713f28..5eab1d8086 100644 --- a/src/components/AdvanceAnalyticsV2/data/utils.test.js +++ b/src/components/AdvanceAnalyticsV2/data/utils.test.js @@ -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', () => { @@ -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', () => { diff --git a/src/components/AdvanceAnalyticsV2/tabs/Skills.jsx b/src/components/AdvanceAnalyticsV2/tabs/Skills.jsx index dbe8b1a9b6..bd35117cc4 100644 --- a/src/components/AdvanceAnalyticsV2/tabs/Skills.jsx +++ b/src/components/AdvanceAnalyticsV2/tabs/Skills.jsx @@ -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 }) => { @@ -22,6 +22,8 @@ const Skills = ({ startDate, endDate, enterpriseId }) => { endDate, }); + const markerSizes = calculateMarkerSizes(data?.topSkills, 'completions'); + return (
@@ -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]}',