From 0572bc28547636436cc3054e4c52126525103140 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 29 Aug 2021 21:54:21 +0200 Subject: [PATCH 01/11] First iteration to migrate to Chart.js 3. Making it compile --- package-lock.json | 63 ++++----------- package.json | 5 +- src/utils/helpers/charts.ts | 34 ++------ src/utils/helpers/numbers.ts | 2 +- src/visits/helpers/DefaultChart.tsx | 113 +++++++++++++++------------ src/visits/helpers/LineChartCard.tsx | 50 ++++++------ 6 files changed, 113 insertions(+), 154 deletions(-) diff --git a/package-lock.json b/package-lock.json index e3daeb0d..8653490d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6363,15 +6363,6 @@ "@babel/types": "^7.3.0" } }, - "@types/chart.js": { - "version": "2.9.31", - "resolved": "https://registry.npmjs.org/@types/chart.js/-/chart.js-2.9.31.tgz", - "integrity": "sha512-hzS6phN/kx3jClk3iYqEHNnYIRSi4RZrIGJ8CDLjgatpHoftCezvC44uqB3o3OUm9ftU1m7sHG8+RLyPTlACrA==", - "dev": true, - "requires": { - "moment": "^2.10.2" - } - }, "@types/cheerio": { "version": "0.22.22", "resolved": "https://registry.npmjs.org/@types/cheerio/-/cheerio-0.22.22.tgz", @@ -10578,30 +10569,9 @@ "dev": true }, "chart.js": { - "version": "2.9.4", - "resolved": "https://registry.npmjs.org/chart.js/-/chart.js-2.9.4.tgz", - "integrity": "sha512-B07aAzxcrikjAPyV+01j7BmOpxtQETxTSlQ26BEYJ+3iUkbNKaOJ/nDbT6JjyqYxseM0ON12COHYdU2cTIjC7A==", - "requires": { - "chartjs-color": "^2.1.0", - "moment": "^2.10.2" - } - }, - "chartjs-color": { - "version": "2.4.1", - "resolved": "https://registry.npmjs.org/chartjs-color/-/chartjs-color-2.4.1.tgz", - "integrity": "sha512-haqOg1+Yebys/Ts/9bLo/BqUcONQOdr/hoEr2LLTRl6C5LXctUdHxsCYfvQVg5JIxITrfCNUDr4ntqmQk9+/0w==", - "requires": { - "chartjs-color-string": "^0.6.0", - "color-convert": "^1.9.3" - } - }, - "chartjs-color-string": { - "version": "0.6.0", - "resolved": "https://registry.npmjs.org/chartjs-color-string/-/chartjs-color-string-0.6.0.tgz", - "integrity": "sha512-TIB5OKn1hPJvO7JcteW4WY/63v6KwEdt6udfnDE9iCAZgy+V4SrbSxoIbTw/xkUIapjEI4ExGtD0+6D3KyFd7A==", - "requires": { - "color-name": "^1.0.0" - } + "version": "3.5.1", + "resolved": "https://registry.npmjs.org/chart.js/-/chart.js-3.5.1.tgz", + "integrity": "sha512-m5kzt72I1WQ9LILwQC4syla/LD/N413RYv2Dx2nnTkRS9iv/ey1xLTt0DnPc/eWV4zI+BgEgDYBIzbQhZHc/PQ==" }, "check-types": { "version": "11.1.2", @@ -10957,6 +10927,7 @@ "version": "1.9.3", "resolved": "https://registry.yarnpkg.com/color-convert/-/color-convert-1.9.3.tgz", "integrity": "sha1-u3GFBpDh8TZWfeYp0tVHHe2kweg=", + "dev": true, "requires": { "color-name": "1.1.3" }, @@ -10964,14 +10935,16 @@ "color-name": { "version": "1.1.3", "resolved": "https://registry.yarnpkg.com/color-name/-/color-name-1.1.3.tgz", - "integrity": "sha1-p9BVi9icQveV3UIyj3QIMcpTvCU=" + "integrity": "sha1-p9BVi9icQveV3UIyj3QIMcpTvCU=", + "dev": true } } }, "color-name": { "version": "1.1.4", "resolved": "https://registry.yarnpkg.com/color-name/-/color-name-1.1.4.tgz", - "integrity": "sha1-wqCah6y95pVD3m9j+jmVyCbFNqI=" + "integrity": "sha1-wqCah6y95pVD3m9j+jmVyCbFNqI=", + "dev": true }, "color-string": { "version": "1.5.4", @@ -19000,11 +18973,6 @@ } } }, - "moment": { - "version": "2.29.1", - "resolved": "https://registry.npmjs.org/moment/-/moment-2.29.1.tgz", - "integrity": "sha512-kHmoybcPV8Sqy59DwNDY3Jefr64lK/by/da0ViFcuA4DH0vQg5Q6Ze5VimxkfQNSC+Mls/Kx53s7TjP1RhFEDQ==" - }, "moo": { "version": "0.5.1", "resolved": "https://registry.npmjs.org/moo/-/moo-0.5.1.tgz", @@ -24570,18 +24538,17 @@ } }, "react-chartjs-2": { - "version": "2.11.1", - "resolved": "https://registry.npmjs.org/react-chartjs-2/-/react-chartjs-2-2.11.1.tgz", - "integrity": "sha512-G7cNq/n2Bkh/v4vcI+GKx7Q1xwZexKYhOSj2HmrFXlvNeaURWXun6KlOUpEQwi1cv9Tgs4H3kGywDWMrX2kxfA==", + "version": "3.0.4", + "resolved": "https://registry.npmjs.org/react-chartjs-2/-/react-chartjs-2-3.0.4.tgz", + "integrity": "sha512-pcbFNpkPMTkGXXJ7k7hnukbRD0ZV01qB6JQY1ontITc2IYvhGlK6BBDy28VeydYs1Dl/c5ZpRgRVEtT5GUnxcQ==", "requires": { - "lodash": "^4.17.19", - "prop-types": "^15.7.2" + "lodash": "^4.17.19" }, "dependencies": { "lodash": { - "version": "4.17.20", - "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.20.tgz", - "integrity": "sha512-PlhdFcillOINfeV7Ni6oF1TAEayyZBoZ8bcshTHqOYJYlrqzRK5hagpagky5o4HfCzzd1TRkXPMFq6cKk9rGmA==" + "version": "4.17.21", + "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz", + "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==" } } }, diff --git a/package.json b/package.json index d2376a65..900cc2c5 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ "bootstrap": "^4.6.0", "bottlejs": "^2.0.0", "bowser": "^2.11.0", - "chart.js": "^2.9.4", + "chart.js": "^3.5.1", "classnames": "^2.2.6", "compare-versions": "^3.6.0", "csvjson": "^5.1.0", @@ -41,7 +41,7 @@ "qs": "^6.9.6", "ramda": "^0.27.1", "react": "^17.0.1", - "react-chartjs-2": "^2.11.1", + "react-chartjs-2": "^3.0.4", "react-color": "^2.19.3", "react-copy-to-clipboard": "^5.0.2", "react-datepicker": "^3.6.0", @@ -72,7 +72,6 @@ "@stryker-mutator/jest-runner": "^5.0.0", "@stryker-mutator/typescript-checker": "^5.0.0", "@svgr/webpack": "^5.5.0", - "@types/chart.js": "^2.9.31", "@types/classnames": "^2.2.11", "@types/enzyme": "^3.10.8", "@types/jest": "^26.0.20", diff --git a/src/utils/helpers/charts.ts b/src/utils/helpers/charts.ts index e47d1b22..256fc6a9 100644 --- a/src/utils/helpers/charts.ts +++ b/src/utils/helpers/charts.ts @@ -1,30 +1,12 @@ -import { ChangeEvent, FC } from 'react'; -import { ChartData, ChartTooltipItem } from 'chart.js'; +import { ActiveElement, ChartEvent, ChartType, TooltipItem } from 'chart.js'; import { prettify } from './numbers'; -export const pointerOnHover = ({ target }: ChangeEvent, chartElement: FC[]) => { - target.style.cursor = chartElement[0] ? 'pointer' : 'default'; +export const pointerOnHover = ({ native }: ChartEvent, [ firstElement ]: ActiveElement[]) => { + if (!native?.target) { + return; + } + + (native.target as any).style.cursor = firstElement ? 'pointer' : 'default'; }; -export const renderNonDoughnutChartLabel = (labelToPick: 'yLabel' | 'xLabel') => ( - item: ChartTooltipItem, - { datasets }: ChartData, -) => { - const { datasetIndex } = item; - const value = item[labelToPick]; - const datasetLabel = datasetIndex !== undefined && datasets?.[datasetIndex]?.label || ''; - - return `${datasetLabel}: ${prettify(Number(value))}`; -}; - -export const renderDoughnutChartLabel = ( - { datasetIndex, index }: ChartTooltipItem, - { labels, datasets }: ChartData, -) => { - const datasetLabel = index !== undefined && labels?.[index] || ''; - const value = datasetIndex !== undefined && index !== undefined - && datasets?.[datasetIndex]?.data?.[index] - || ''; - - return `${datasetLabel}: ${prettify(Number(value))}`; // eslint-disable-line @typescript-eslint/no-base-to-string -}; +export const renderChartLabel = ({ dataset, label }: TooltipItem) => `${dataset.label}: ${prettify(label)}`; diff --git a/src/utils/helpers/numbers.ts b/src/utils/helpers/numbers.ts index 559184bf..dbaf6406 100644 --- a/src/utils/helpers/numbers.ts +++ b/src/utils/helpers/numbers.ts @@ -2,6 +2,6 @@ const TEN_ROUNDING_NUMBER = 10; const { ceil } = Math; const formatter = new Intl.NumberFormat('en-US'); -export const prettify = (number: number) => formatter.format(number); +export const prettify = (number: number | string) => formatter.format(Number(number)); export const roundTen = (number: number) => ceil(number / TEN_ROUNDING_NUMBER) * TEN_ROUNDING_NUMBER; diff --git a/src/visits/helpers/DefaultChart.tsx b/src/visits/helpers/DefaultChart.tsx index e68b1e4c..21f62ce9 100644 --- a/src/visits/helpers/DefaultChart.tsx +++ b/src/visits/helpers/DefaultChart.tsx @@ -1,12 +1,12 @@ -import { useState } from 'react'; -import { Doughnut, HorizontalBar } from 'react-chartjs-2'; +import { useRef } from 'react'; +import { Doughnut, Bar } from 'react-chartjs-2'; import { keys, values } from 'ramda'; import classNames from 'classnames'; -import Chart, { ChartData, ChartDataSets, ChartOptions } from 'chart.js'; +import { Chart, ChartData, ChartDataset, ChartOptions, LegendItem } from 'chart.js'; import { fillTheGaps } from '../../utils/helpers/visits'; import { Stats } from '../types'; import { prettify } from '../../utils/helpers/numbers'; -import { pointerOnHover, renderDoughnutChartLabel, renderNonDoughnutChartLabel } from '../../utils/helpers/charts'; +import { pointerOnHover, renderChartLabel } from '../../utils/helpers/charts'; import { HIGHLIGHTED_COLOR, HIGHLIGHTED_COLOR_ALPHA, @@ -66,7 +66,7 @@ const generateGraphData = ( borderColor: HIGHLIGHTED_COLOR, borderWidth: 2, }, - ].filter(Boolean) as ChartDataSets[], + ].filter(Boolean) as ChartDataset[], }); const dropLabelIfHidden = (label: string) => label.startsWith('hidden') ? '' : label; @@ -79,27 +79,34 @@ const determineHeight = (isBarChart: boolean, labels: string[]): number | undefi return isBarChart && labels.length > 20 ? labels.length * 8 : undefined; }; -const renderPieChartLegend = ({ config }: Chart) => { - const { labels = [], datasets = [] } = config.data ?? {}; - const { defaultColor } = config.options ?? {} as any; - const [{ backgroundColor: colors }] = datasets; +const renderPieChartLegend = ({ config }: Chart): LegendItem[] => { + const { labels = [] /* , datasets = [] */ } = config.data ?? {}; + // const { defaultColor } = config.options ?? {} as any; + // const [{ backgroundColor: colors }] = datasets; - return ( -
    - {labels.map((label, index) => ( -
  • -
    - {label} -
    • - ))} -
- ); + return labels.map((label, datasetIndex) => ({ + datasetIndex, + text: label as string, + })); + + // TODO + // return ( + //
    + // {labels.map((label, index) => ( + //
  • + //
    + // {label} + //
    • + // ))} + //
+ // ); }; const chartElementAtEvent = (onClick?: (label: string) => void) => ([ chart ]: [{ _index: number; _chart: Chart }]) => { + // TODO Check this function actually works with Chart.js 3 if (!onClick || !chart) { return; } @@ -115,7 +122,8 @@ const statsAreDefined = (stats: Stats | undefined): stats is Stats => !!stats && const DefaultChart = ( { title, isBarChart = false, stats, max, highlightedStats, highlightedLabel, onClick }: DefaultChartProps, ) => { - const Component = isBarChart ? HorizontalBar : Doughnut; + const Component = isBarChart ? Bar : Doughnut; + const chartRef = useRef(); const labels = keys(stats).map(dropLabelIfHidden); const data = values( !statsAreDefined(highlightedStats) ? stats : keys(highlightedStats).reduce((acc, highlightedKey) => { @@ -127,34 +135,38 @@ const DefaultChart = ( }, { ...stats }), ); const highlightedData = statsAreDefined(highlightedStats) ? fillTheGaps(highlightedStats, labels) : undefined; - const [ chartRef, setChartRef ] = useState(); const options: ChartOptions = { - legend: { display: false }, - legendCallback: !isBarChart && renderPieChartLegend as any, - scales: !isBarChart ? undefined : { - xAxes: [ - { - ticks: { - beginAtZero: true, - precision: 0, - callback: prettify, - max, - }, - stacked: true, + plugins: { + legend: { + display: false, + labels: isBarChart ? undefined : { + generateLabels: renderPieChartLegend, + }, + }, + tooltip: { + intersect: !isBarChart, + // Do not show tooltip on items with empty label when in a bar chart + filter: ({ label }) => !isBarChart || label !== '', + callbacks: { + label: renderChartLabel, }, - ], - yAxes: [{ stacked: true }], - }, - tooltips: { - intersect: !isBarChart, - // Do not show tooltip on items with empty label when in a bar chart - filter: ({ yLabel }) => !isBarChart || yLabel !== '', - callbacks: { - label: isBarChart ? renderNonDoughnutChartLabel('xLabel') : renderDoughnutChartLabel, }, }, - onHover: !isBarChart ? undefined : (pointerOnHover) as any, // TODO Types seem to be incorrectly defined in @types/chart.js + scales: !isBarChart ? undefined : { + x: { + beginAtZero: true, + stacked: true, + max, + ticks: { + precision: 0, + callback: prettify, + }, + }, + y: { stacked: true }, + }, + onHover: isBarChart ? pointerOnHover : undefined, + indexAxis: isBarChart ? 'y' : 'x', }; const graphData = generateGraphData(title, isBarChart, labels, data, highlightedData, highlightedLabel); const height = determineHeight(isBarChart, labels); @@ -164,17 +176,20 @@ const DefaultChart = (
setChartRef(element ?? undefined)} + ref={(element) => { + chartRef.current = element ?? undefined; + }} key={height} data={graphData} options={options} height={height} - getElementAtEvent={chartElementAtEvent(onClick)} + getElementAtEvent={chartElementAtEvent(onClick) as any} /* TODO */ />
{!isBarChart && (
- {chartRef?.chartInstance.generateLegend()} + No Legend in v3.0 unfortunately :( + {/* {chartRef?.chartInstance.generateLegend()} */}
)}
diff --git a/src/visits/helpers/LineChartCard.tsx b/src/visits/helpers/LineChartCard.tsx index abc5ffc2..622aa5e7 100644 --- a/src/visits/helpers/LineChartCard.tsx +++ b/src/visits/helpers/LineChartCard.tsx @@ -21,14 +21,14 @@ import { startOfISOWeek, endOfISOWeek, } from 'date-fns'; -import Chart, { ChartData, ChartDataSets, ChartOptions } from 'chart.js'; +import { Chart, ChartData, ChartDataset, ChartOptions } from 'chart.js'; import { NormalizedVisit, Stats } from '../types'; import { fillTheGaps } from '../../utils/helpers/visits'; import { useToggle } from '../../utils/helpers/hooks'; import { rangeOf } from '../../utils/utils'; import ToggleSwitch from '../../utils/ToggleSwitch'; import { prettify } from '../../utils/helpers/numbers'; -import { pointerOnHover, renderNonDoughnutChartLabel } from '../../utils/helpers/charts'; +import { pointerOnHover, renderChartLabel } from '../../utils/helpers/charts'; import { HIGHLIGHTED_COLOR, MAIN_COLOR } from '../../utils/theme'; import './LineChartCard.scss'; @@ -134,11 +134,11 @@ const generateLabelsAndGroupedVisits = ( return [ labels, fillTheGaps(groupedVisitsWithGaps, labels) ]; }; -const generateDataset = (data: number[], label: string, color: string): ChartDataSets => ({ +const generateDataset = (data: number[], label: string, color: string): ChartDataset => ({ label, data, fill: false, - lineTension: 0.2, + tension: 0.2, borderColor: color, backgroundColor: color, }); @@ -189,32 +189,28 @@ const LineChartCard = ( datasets: [ generateDataset(groupedVisits, 'Visits', MAIN_COLOR), highlightedVisits.length > 0 && generateDataset(groupedHighlighted, highlightedLabel, HIGHLIGHTED_COLOR), - ].filter(Boolean) as ChartDataSets[], + ].filter(Boolean) as ChartDataset[], }; const options: ChartOptions = { maintainAspectRatio: false, - legend: { display: false }, - scales: { - yAxes: [ - { - ticks: { - beginAtZero: true, - precision: 0, - callback: prettify, - }, - }, - ], - xAxes: [ - { - scaleLabel: { display: true, labelString: STEPS_MAP[step] }, - }, - ], + plugins: { + legend: { display: false }, + tooltip: { + intersect: false, + axis: 'x', + callbacks: { label: renderChartLabel }, + }, }, - tooltips: { - intersect: false, - axis: 'x', - callbacks: { - label: renderNonDoughnutChartLabel('yLabel'), + scales: { + y: { + beginAtZero: true, + ticks: { + precision: 0, + callback: prettify, + }, + }, + x: { + title: { display: true, text: STEPS_MAP[step] }, }, }, onHover: (pointerOnHover) as any, // TODO Types seem to be incorrectly defined in @types/chart.js @@ -248,7 +244,7 @@ const LineChartCard = ( From d55160e8f60ae4b5286971df923b1d21cd3b00e4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 29 Aug 2021 22:04:04 +0200 Subject: [PATCH 02/11] Recovered function to render pie chart labels --- src/utils/helpers/charts.ts | 10 ++++++++-- src/visits/helpers/DefaultChart.tsx | 4 ++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/utils/helpers/charts.ts b/src/utils/helpers/charts.ts index 256fc6a9..e702dc09 100644 --- a/src/utils/helpers/charts.ts +++ b/src/utils/helpers/charts.ts @@ -6,7 +6,13 @@ export const pointerOnHover = ({ native }: ChartEvent, [ firstElement ]: ActiveE return; } - (native.target as any).style.cursor = firstElement ? 'pointer' : 'default'; + const canvas = native.target as HTMLCanvasElement; + + canvas.style.cursor = firstElement ? 'pointer' : 'default'; }; -export const renderChartLabel = ({ dataset, label }: TooltipItem) => `${dataset.label}: ${prettify(label)}`; +export const renderChartLabel = ({ dataset, formattedValue }: TooltipItem) => + `${dataset.label}: ${prettify(formattedValue)}`; + +export const renderPieChartLabel = ({ label, formattedValue }: TooltipItem) => + `${label}: ${prettify(formattedValue)}`; diff --git a/src/visits/helpers/DefaultChart.tsx b/src/visits/helpers/DefaultChart.tsx index 21f62ce9..7792aac7 100644 --- a/src/visits/helpers/DefaultChart.tsx +++ b/src/visits/helpers/DefaultChart.tsx @@ -6,7 +6,7 @@ import { Chart, ChartData, ChartDataset, ChartOptions, LegendItem } from 'chart. import { fillTheGaps } from '../../utils/helpers/visits'; import { Stats } from '../types'; import { prettify } from '../../utils/helpers/numbers'; -import { pointerOnHover, renderChartLabel } from '../../utils/helpers/charts'; +import { pointerOnHover, renderChartLabel, renderPieChartLabel } from '../../utils/helpers/charts'; import { HIGHLIGHTED_COLOR, HIGHLIGHTED_COLOR_ALPHA, @@ -149,7 +149,7 @@ const DefaultChart = ( // Do not show tooltip on items with empty label when in a bar chart filter: ({ label }) => !isBarChart || label !== '', callbacks: { - label: renderChartLabel, + label: isBarChart ? renderChartLabel : renderPieChartLabel, }, }, }, From 036c8aafcb6cb4e28a4f66c5be240b72887ee392 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 29 Aug 2021 22:20:36 +0200 Subject: [PATCH 03/11] Extracted PieChartLegend to its own component --- src/visits/helpers/DefaultChart.tsx | 46 +++---------------- ...{DefaultChart.scss => PieChartLegend.scss} | 0 src/visits/helpers/PieChartLegend.tsx | 24 ++++++++++ 3 files changed, 31 insertions(+), 39 deletions(-) rename src/visits/helpers/{DefaultChart.scss => PieChartLegend.scss} (100%) create mode 100644 src/visits/helpers/PieChartLegend.tsx diff --git a/src/visits/helpers/DefaultChart.tsx b/src/visits/helpers/DefaultChart.tsx index 7792aac7..33587161 100644 --- a/src/visits/helpers/DefaultChart.tsx +++ b/src/visits/helpers/DefaultChart.tsx @@ -1,8 +1,8 @@ -import { useRef } from 'react'; +import { useState } from 'react'; import { Doughnut, Bar } from 'react-chartjs-2'; import { keys, values } from 'ramda'; import classNames from 'classnames'; -import { Chart, ChartData, ChartDataset, ChartOptions, LegendItem } from 'chart.js'; +import { Chart, ChartData, ChartDataset, ChartOptions } from 'chart.js'; import { fillTheGaps } from '../../utils/helpers/visits'; import { Stats } from '../types'; import { prettify } from '../../utils/helpers/numbers'; @@ -16,7 +16,7 @@ import { PRIMARY_DARK_COLOR, PRIMARY_LIGHT_COLOR, } from '../../utils/theme'; -import './DefaultChart.scss'; +import { PieChartLegend } from './PieChartLegend'; export interface DefaultChartProps { title: Function | string; @@ -79,32 +79,6 @@ const determineHeight = (isBarChart: boolean, labels: string[]): number | undefi return isBarChart && labels.length > 20 ? labels.length * 8 : undefined; }; -const renderPieChartLegend = ({ config }: Chart): LegendItem[] => { - const { labels = [] /* , datasets = [] */ } = config.data ?? {}; - // const { defaultColor } = config.options ?? {} as any; - // const [{ backgroundColor: colors }] = datasets; - - return labels.map((label, datasetIndex) => ({ - datasetIndex, - text: label as string, - })); - - // TODO - // return ( - //
    - // {labels.map((label, index) => ( - //
  • - //
    - // {label} - //
    • - // ))} - //
- // ); -}; - const chartElementAtEvent = (onClick?: (label: string) => void) => ([ chart ]: [{ _index: number; _chart: Chart }]) => { // TODO Check this function actually works with Chart.js 3 if (!onClick || !chart) { @@ -123,7 +97,7 @@ const DefaultChart = ( { title, isBarChart = false, stats, max, highlightedStats, highlightedLabel, onClick }: DefaultChartProps, ) => { const Component = isBarChart ? Bar : Doughnut; - const chartRef = useRef(); + const [ chartRef, setChartRef ] = useState(); const labels = keys(stats).map(dropLabelIfHidden); const data = values( !statsAreDefined(highlightedStats) ? stats : keys(highlightedStats).reduce((acc, highlightedKey) => { @@ -138,12 +112,7 @@ const DefaultChart = ( const options: ChartOptions = { plugins: { - legend: { - display: false, - labels: isBarChart ? undefined : { - generateLabels: renderPieChartLegend, - }, - }, + legend: { display: false }, tooltip: { intersect: !isBarChart, // Do not show tooltip on items with empty label when in a bar chart @@ -177,7 +146,7 @@ const DefaultChart = (
{ - chartRef.current = element ?? undefined; + setChartRef(element ?? undefined); }} key={height} data={graphData} @@ -188,8 +157,7 @@ const DefaultChart = (
{!isBarChart && (
- No Legend in v3.0 unfortunately :( - {/* {chartRef?.chartInstance.generateLegend()} */} + {chartRef && }
)} diff --git a/src/visits/helpers/DefaultChart.scss b/src/visits/helpers/PieChartLegend.scss similarity index 100% rename from src/visits/helpers/DefaultChart.scss rename to src/visits/helpers/PieChartLegend.scss diff --git a/src/visits/helpers/PieChartLegend.tsx b/src/visits/helpers/PieChartLegend.tsx new file mode 100644 index 00000000..6b765068 --- /dev/null +++ b/src/visits/helpers/PieChartLegend.tsx @@ -0,0 +1,24 @@ +import { FC } from 'react'; +import { Chart } from 'chart.js'; +import './PieChartLegend.scss'; + +export const PieChartLegend: FC<{ chart: Chart }> = ({ chart }) => { + const { config } = chart; + const { labels = [], datasets = [] } = config.data ?? {}; + const { defaultColor } = config.options ?? {} as any; + const [{ backgroundColor: colors }] = datasets; + + return ( +
    + {(labels as string[]).map((label, index) => ( +
  • +
    + {label} +
    • + ))} +
+ ); +}; From f54460e8f87004f4a3e13f428a4c9a3a0c604ca8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 29 Aug 2021 22:50:05 +0200 Subject: [PATCH 04/11] First attempt to fix click event on charts --- src/visits/helpers/DefaultChart.tsx | 15 +++++++-------- src/visits/helpers/LineChartCard.tsx | 10 +++++----- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/visits/helpers/DefaultChart.tsx b/src/visits/helpers/DefaultChart.tsx index 33587161..eb135203 100644 --- a/src/visits/helpers/DefaultChart.tsx +++ b/src/visits/helpers/DefaultChart.tsx @@ -79,16 +79,15 @@ const determineHeight = (isBarChart: boolean, labels: string[]): number | undefi return isBarChart && labels.length > 20 ? labels.length * 8 : undefined; }; -const chartElementAtEvent = (onClick?: (label: string) => void) => ([ chart ]: [{ _index: number; _chart: Chart }]) => { - // TODO Check this function actually works with Chart.js 3 +const chartElementAtEvent = ( + labels: string[], + onClick?: (label: string) => void, +) => ([ chart ]: [{ index: number }]) => { if (!onClick || !chart) { return; } - const { _index, _chart: { data } } = chart; - const { labels } = data; - - onClick(labels?.[_index] as string); + onClick(labels[chart.index]); }; const statsAreDefined = (stats: Stats | undefined): stats is Stats => !!stats && Object.keys(stats).length > 0; @@ -97,7 +96,7 @@ const DefaultChart = ( { title, isBarChart = false, stats, max, highlightedStats, highlightedLabel, onClick }: DefaultChartProps, ) => { const Component = isBarChart ? Bar : Doughnut; - const [ chartRef, setChartRef ] = useState(); + const [ chartRef, setChartRef ] = useState(); // Cannot use useRef here const labels = keys(stats).map(dropLabelIfHidden); const data = values( !statsAreDefined(highlightedStats) ? stats : keys(highlightedStats).reduce((acc, highlightedKey) => { @@ -152,7 +151,7 @@ const DefaultChart = ( data={graphData} options={options} height={height} - getElementAtEvent={chartElementAtEvent(onClick) as any} /* TODO */ + getElementAtEvent={chartElementAtEvent(labels, onClick) as any} /> {!isBarChart && ( diff --git a/src/visits/helpers/LineChartCard.tsx b/src/visits/helpers/LineChartCard.tsx index 622aa5e7..58271723 100644 --- a/src/visits/helpers/LineChartCard.tsx +++ b/src/visits/helpers/LineChartCard.tsx @@ -21,7 +21,7 @@ import { startOfISOWeek, endOfISOWeek, } from 'date-fns'; -import { Chart, ChartData, ChartDataset, ChartOptions } from 'chart.js'; +import { ChartData, ChartDataset, ChartOptions } from 'chart.js'; import { NormalizedVisit, Stats } from '../types'; import { fillTheGaps } from '../../utils/helpers/visits'; import { useToggle } from '../../utils/helpers/hooks'; @@ -146,15 +146,15 @@ const generateDataset = (data: number[], label: string, color: string): ChartDat let selectedLabel: string | null = null; const chartElementAtEvent = ( + labels: string[], datasetsByPoint: Record, setSelectedVisits?: (visits: NormalizedVisit[]) => void, -) => ([ chart ]: [{ _index: number; _chart: Chart }]) => { +) => ([ chart ]: [{ index: number }]) => { if (!setSelectedVisits || !chart) { return; } - const { _index: index, _chart: { data } } = chart; - const { labels } = data as { labels: string[] }; + const { index } = chart; if (selectedLabel === labels[index]) { setSelectedVisits([]); @@ -244,7 +244,7 @@ const LineChartCard = ( From 039a56f41070e7b4a4447a0a9acbd9c64a4393b7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 18 Sep 2021 12:07:05 +0200 Subject: [PATCH 05/11] Fixed tooltips in bar charts --- src/visits/helpers/DefaultChart.tsx | 89 ++++++++++++++----------- src/visits/helpers/GraphCard.tsx | 3 +- src/visits/helpers/LineChartCard.tsx | 37 ++++++---- src/visits/helpers/SortableBarGraph.tsx | 1 + 4 files changed, 77 insertions(+), 53 deletions(-) diff --git a/src/visits/helpers/DefaultChart.tsx b/src/visits/helpers/DefaultChart.tsx index eb135203..16daadad 100644 --- a/src/visits/helpers/DefaultChart.tsx +++ b/src/visits/helpers/DefaultChart.tsx @@ -19,7 +19,6 @@ import { import { PieChartLegend } from './PieChartLegend'; export interface DefaultChartProps { - title: Function | string; stats: Stats; isBarChart?: boolean; max?: number; @@ -28,45 +27,56 @@ export interface DefaultChartProps { onClick?: (label: string) => void; } -const generateGraphData = ( - title: Function | string, +const generateChartDatasets = ( + isBarChart: boolean, + data: number[], + highlightedData: number[], + highlightedLabel?: string, +): ChartDataset[] => { + const mainDataset: ChartDataset = { + label: highlightedLabel ? 'Non-selected' : 'Visits', + data, + backgroundColor: isBarChart ? MAIN_COLOR_ALPHA : [ + '#97BBCD', + '#F7464A', + '#46BFBD', + '#FDB45C', + '#949FB1', + '#57A773', + '#414066', + '#08B2E3', + '#B6C454', + '#DCDCDC', + '#463730', + ], + borderColor: isBarChart ? MAIN_COLOR : isDarkThemeEnabled() ? PRIMARY_DARK_COLOR : PRIMARY_LIGHT_COLOR, + borderWidth: 2, + }; + + if (!isBarChart) { + return [ mainDataset ]; + } + + const highlightedDataset: ChartDataset = { + label: highlightedLabel ?? 'Selected', + data: highlightedData, + backgroundColor: HIGHLIGHTED_COLOR_ALPHA, + borderColor: HIGHLIGHTED_COLOR, + borderWidth: 2, + }; + + return [ mainDataset, highlightedDataset ]; +}; + +const generateChartData = ( isBarChart: boolean, labels: string[], data: number[], - highlightedData?: number[], + highlightedData: number[], highlightedLabel?: string, ): ChartData => ({ labels, - datasets: [ - { - title, - label: highlightedData ? 'Non-selected' : 'Visits', - data, - backgroundColor: isBarChart ? MAIN_COLOR_ALPHA : [ - '#97BBCD', - '#F7464A', - '#46BFBD', - '#FDB45C', - '#949FB1', - '#57A773', - '#414066', - '#08B2E3', - '#B6C454', - '#DCDCDC', - '#463730', - ], - borderColor: isBarChart ? MAIN_COLOR : isDarkThemeEnabled() ? PRIMARY_DARK_COLOR : PRIMARY_LIGHT_COLOR, - borderWidth: 2, - }, - highlightedData && { - title, - label: highlightedLabel ?? 'Selected', - data: highlightedData, - backgroundColor: HIGHLIGHTED_COLOR_ALPHA, - borderColor: HIGHLIGHTED_COLOR, - borderWidth: 2, - }, - ].filter(Boolean) as ChartDataset[], + datasets: generateChartDatasets(isBarChart, data, highlightedData, highlightedLabel), }); const dropLabelIfHidden = (label: string) => label.startsWith('hidden') ? '' : label; @@ -93,7 +103,7 @@ const chartElementAtEvent = ( const statsAreDefined = (stats: Stats | undefined): stats is Stats => !!stats && Object.keys(stats).length > 0; const DefaultChart = ( - { title, isBarChart = false, stats, max, highlightedStats, highlightedLabel, onClick }: DefaultChartProps, + { isBarChart = false, stats, max, highlightedStats, highlightedLabel, onClick }: DefaultChartProps, ) => { const Component = isBarChart ? Bar : Doughnut; const [ chartRef, setChartRef ] = useState(); // Cannot use useRef here @@ -107,13 +117,14 @@ const DefaultChart = ( return acc; }, { ...stats }), ); - const highlightedData = statsAreDefined(highlightedStats) ? fillTheGaps(highlightedStats, labels) : undefined; + const highlightedData = fillTheGaps(highlightedStats ?? {}, labels); const options: ChartOptions = { plugins: { legend: { display: false }, tooltip: { intersect: !isBarChart, + mode: isBarChart ? 'y' : 'index', // Do not show tooltip on items with empty label when in a bar chart filter: ({ label }) => !isBarChart || label !== '', callbacks: { @@ -136,10 +147,10 @@ const DefaultChart = ( onHover: isBarChart ? pointerOnHover : undefined, indexAxis: isBarChart ? 'y' : 'x', }; - const graphData = generateGraphData(title, isBarChart, labels, data, highlightedData, highlightedLabel); + const chartData = generateChartData(isBarChart, labels, data, highlightedData, highlightedLabel); const height = determineHeight(isBarChart, labels); - // Provide a key based on the height, so that every time the dataset changes, a new graph is rendered + // Provide a key based on the height, so that every time the dataset changes, a new chart is rendered return (
@@ -148,7 +159,7 @@ const DefaultChart = ( setChartRef(element ?? undefined); }} key={height} - data={graphData} + data={chartData} options={options} height={height} getElementAtEvent={chartElementAtEvent(labels, onClick) as any} diff --git a/src/visits/helpers/GraphCard.tsx b/src/visits/helpers/GraphCard.tsx index c0709493..57388835 100644 --- a/src/visits/helpers/GraphCard.tsx +++ b/src/visits/helpers/GraphCard.tsx @@ -4,6 +4,7 @@ import DefaultChart, { DefaultChartProps } from './DefaultChart'; import './GraphCard.scss'; interface GraphCardProps extends DefaultChartProps { + title: Function | string; footer?: ReactNode; } @@ -11,7 +12,7 @@ const GraphCard = ({ title, footer, ...rest }: GraphCardProps) => ( {typeof title === 'function' ? title() : title} - + {footer && {footer}} diff --git a/src/visits/helpers/LineChartCard.tsx b/src/visits/helpers/LineChartCard.tsx index 58271723..8cca1e9f 100644 --- a/src/visits/helpers/LineChartCard.tsx +++ b/src/visits/helpers/LineChartCard.tsx @@ -1,4 +1,4 @@ -import { useState, useMemo } from 'react'; +import { useState, useMemo, FC } from 'react'; import { Card, CardHeader, @@ -183,14 +183,19 @@ const LineChartCard = ( () => fillTheGaps(groupVisitsByStep(step, reverse(highlightedVisits)), labels), [ highlightedVisits, step, labels ], ); + const generateChartDatasets = (): ChartDataset[] => { + const mainDataset = generateDataset(groupedVisits, 'Visits', MAIN_COLOR); - const data: ChartData = { - labels, - datasets: [ - generateDataset(groupedVisits, 'Visits', MAIN_COLOR), - highlightedVisits.length > 0 && generateDataset(groupedHighlighted, highlightedLabel, HIGHLIGHTED_COLOR), - ].filter(Boolean) as ChartDataset[], + if (highlightedVisits.length === 0) { + return [ mainDataset ]; + } + + const highlightedDataset = generateDataset(groupedHighlighted, highlightedLabel, HIGHLIGHTED_COLOR); + + return [ mainDataset, highlightedDataset ]; }; + const generateChartData = (): ChartData => ({ labels, datasets: generateChartDatasets() }); + const options: ChartOptions = { maintainAspectRatio: false, plugins: { @@ -213,8 +218,15 @@ const LineChartCard = ( title: { display: true, text: STEPS_MAP[step] }, }, }, - onHover: (pointerOnHover) as any, // TODO Types seem to be incorrectly defined in @types/chart.js + onHover: pointerOnHover, }; + const LineChart: FC = () => ( + + ); return ( @@ -241,11 +253,10 @@ const LineChartCard = (
- + {/* It's VERY IMPORTANT to render two different components here, as one has 1 dataset and the other has 2 */} + {/* Using the same component causes a crash when switching from 1 to 2 datasets, and then back to 1 dataset */} + {highlightedVisits.length > 0 && } + {highlightedVisits.length === 0 && } ); diff --git a/src/visits/helpers/SortableBarGraph.tsx b/src/visits/helpers/SortableBarGraph.tsx index 6e2efae5..9a4bd429 100644 --- a/src/visits/helpers/SortableBarGraph.tsx +++ b/src/visits/helpers/SortableBarGraph.tsx @@ -14,6 +14,7 @@ const pickKeyFromPair = ([ key ]: StatsRow) => key; const pickValueFromPair = ([ , value ]: StatsRow) => value; interface SortableBarGraphProps extends DefaultChartProps { + title: Function | string; sortingItems: Record; withPagination?: boolean; extraHeaderContent?: Function; From 58ee123cefe7f40b1d2366905f3708c5240cdb2a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 18 Sep 2021 12:29:15 +0200 Subject: [PATCH 06/11] Memoized DefaultChart to make sure it does not change unless its props also change --- .eslintrc | 5 ++++- src/short-urls/ShortUrlForm.tsx | 2 +- src/visits/helpers/DefaultChart.tsx | 35 +++++++++++++++++------------ 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/.eslintrc b/.eslintrc index 1349351c..78ada1c2 100644 --- a/.eslintrc +++ b/.eslintrc @@ -14,5 +14,8 @@ "process": true, "setImmediate": true }, - "ignorePatterns": ["src/service*.ts"] + "ignorePatterns": ["src/service*.ts"], + "rules": { + "complexity": "off" + } } diff --git a/src/short-urls/ShortUrlForm.tsx b/src/short-urls/ShortUrlForm.tsx index 368da62e..96fc6eea 100644 --- a/src/short-urls/ShortUrlForm.tsx +++ b/src/short-urls/ShortUrlForm.tsx @@ -43,7 +43,7 @@ const toDate = (date?: string | Date): Date | undefined => typeof date === 'stri export const ShortUrlForm = ( TagsSelector: FC, DomainSelector: FC, -): FC => ({ mode, saving, onSave, initialState, selectedServer }) => { // eslint-disable-line complexity +): FC => ({ mode, saving, onSave, initialState, selectedServer }) => { const [ shortUrlData, setShortUrlData ] = useState(initialState); const isEdit = mode === 'edit'; const changeTags = (tags: string[]) => setShortUrlData({ ...shortUrlData, tags: tags.map(normalizeTag) }); diff --git a/src/visits/helpers/DefaultChart.tsx b/src/visits/helpers/DefaultChart.tsx index 16daadad..1694d52a 100644 --- a/src/visits/helpers/DefaultChart.tsx +++ b/src/visits/helpers/DefaultChart.tsx @@ -1,4 +1,4 @@ -import { useState } from 'react'; +import { useState, memo } from 'react'; import { Doughnut, Bar } from 'react-chartjs-2'; import { keys, values } from 'ramda'; import classNames from 'classnames'; @@ -53,7 +53,7 @@ const generateChartDatasets = ( borderWidth: 2, }; - if (!isBarChart) { + if (!isBarChart || highlightedData.every((value) => value === 0)) { return [ mainDataset ]; } @@ -102,7 +102,7 @@ const chartElementAtEvent = ( const statsAreDefined = (stats: Stats | undefined): stats is Stats => !!stats && Object.keys(stats).length > 0; -const DefaultChart = ( +const DefaultChart = memo(( { isBarChart = false, stats, max, highlightedStats, highlightedLabel, onClick }: DefaultChartProps, ) => { const Component = isBarChart ? Bar : Doughnut; @@ -150,20 +150,27 @@ const DefaultChart = ( const chartData = generateChartData(isBarChart, labels, data, highlightedData, highlightedLabel); const height = determineHeight(isBarChart, labels); + const renderChartComponent = (customKey: string) => ( + { + setChartRef(element ?? undefined); + }} + key={`${height}_${customKey}`} + data={chartData} + options={options} + height={height} + getElementAtEvent={chartElementAtEvent(labels, onClick) as any} + /> + ); + // Provide a key based on the height, so that every time the dataset changes, a new chart is rendered return (
- { - setChartRef(element ?? undefined); - }} - key={height} - data={chartData} - options={options} - height={height} - getElementAtEvent={chartElementAtEvent(labels, onClick) as any} - /> + {/* It's VERY IMPORTANT to render two different components here, as one has 1 dataset and the other has 2 */} + {/* Using the same component causes a crash when switching from 1 to 2 datasets, and then back to 1 dataset */} + {highlightedStats !== undefined && renderChartComponent('with_stats')} + {highlightedStats === undefined && renderChartComponent('without_stats')}
{!isBarChart && (
@@ -172,6 +179,6 @@ const DefaultChart = ( )}
); -}; +}); export default DefaultChart; From 382d7b1c9f1e5a954f86e83d5d790b24b328a812 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 18 Sep 2021 12:34:14 +0200 Subject: [PATCH 07/11] Improved comment --- src/visits/helpers/DefaultChart.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/visits/helpers/DefaultChart.tsx b/src/visits/helpers/DefaultChart.tsx index 1694d52a..73107748 100644 --- a/src/visits/helpers/DefaultChart.tsx +++ b/src/visits/helpers/DefaultChart.tsx @@ -150,6 +150,7 @@ const DefaultChart = memo(( const chartData = generateChartData(isBarChart, labels, data, highlightedData, highlightedLabel); const height = determineHeight(isBarChart, labels); + // Provide a key based on the height, to force re-render every time the dataset changes (example, due to pagination) const renderChartComponent = (customKey: string) => ( { @@ -163,7 +164,6 @@ const DefaultChart = memo(( /> ); - // Provide a key based on the height, so that every time the dataset changes, a new chart is rendered return (
From 7e5397dd38754132638e0f437829f9611cf1d918 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 18 Sep 2021 12:59:54 +0200 Subject: [PATCH 08/11] Created PieChartLegend test --- src/visits/helpers/PieChartLegend.scss | 8 ++--- src/visits/helpers/PieChartLegend.tsx | 18 ++++++----- test/visits/helpers/PieChartLegend.test.tsx | 33 +++++++++++++++++++++ 3 files changed, 48 insertions(+), 11 deletions(-) create mode 100644 test/visits/helpers/PieChartLegend.test.tsx diff --git a/src/visits/helpers/PieChartLegend.scss b/src/visits/helpers/PieChartLegend.scss index e7a0bd9f..2d4956f6 100644 --- a/src/visits/helpers/PieChartLegend.scss +++ b/src/visits/helpers/PieChartLegend.scss @@ -1,6 +1,6 @@ @import '../../utils/base'; -.default-chart__pie-chart-legend { +.pie-chart-legend { list-style-type: none; padding: 0; margin: 0; @@ -10,11 +10,11 @@ } } -.default-chart__pie-chart-legend-item:not(:first-child) { +.pie-chart-legend__item:not(:first-child) { margin-top: .3rem; } -.default-chart__pie-chart-legend-item-color { +.pie-chart-legend__item-color { width: 20px; min-width: 20px; height: 20px; @@ -22,7 +22,7 @@ border-radius: 10px; } -.default-chart__pie-chart-legend-item-text { +.pie-chart-legend__item-text { white-space: nowrap; overflow: hidden; text-overflow: ellipsis; diff --git a/src/visits/helpers/PieChartLegend.tsx b/src/visits/helpers/PieChartLegend.tsx index 6b765068..e6e51d27 100644 --- a/src/visits/helpers/PieChartLegend.tsx +++ b/src/visits/helpers/PieChartLegend.tsx @@ -2,21 +2,25 @@ import { FC } from 'react'; import { Chart } from 'chart.js'; import './PieChartLegend.scss'; -export const PieChartLegend: FC<{ chart: Chart }> = ({ chart }) => { +interface PieChartLegendProps { + chart: Chart; +} + +export const PieChartLegend: FC = ({ chart }) => { const { config } = chart; const { labels = [], datasets = [] } = config.data ?? {}; - const { defaultColor } = config.options ?? {} as any; const [{ backgroundColor: colors }] = datasets; + const { defaultColor } = config.options ?? {} as any; return ( -
    +
      {(labels as string[]).map((label, index) => ( -
    • +
    • - {label} + {label}
      • ))}
    diff --git a/test/visits/helpers/PieChartLegend.test.tsx b/test/visits/helpers/PieChartLegend.test.tsx new file mode 100644 index 00000000..eebf9365 --- /dev/null +++ b/test/visits/helpers/PieChartLegend.test.tsx @@ -0,0 +1,33 @@ +import { shallow } from 'enzyme'; +import { Mock } from 'ts-mockery'; +import { Chart, ChartDataset } from 'chart.js'; +import { PieChartLegend } from '../../../src/visits/helpers/PieChartLegend'; + +describe('', () => { + const labels = [ 'foo', 'bar', 'baz', 'foo2', 'bar2' ]; + const colors = [ 'foo_color', 'bar_color', 'baz_color' ]; + const defaultColor = 'red'; + const datasets = [ Mock.of({ backgroundColor: colors }) ]; + const chart = Mock.of({ + config: { + data: { labels, datasets }, + options: { defaultColor } as any, + }, + }); + + test('renders the expected amount of items with expected colors and labels', () => { + const wrapper = shallow(); + const items = wrapper.find('li'); + + expect.assertions(labels.length * 2 + 1); + expect(items).toHaveLength(labels.length); + labels.forEach((label, index) => { + const item = items.at(index); + + expect(item.find('.pie-chart-legend__item-color').prop('style')).toEqual({ + backgroundColor: colors[index] ?? defaultColor, + }); + expect(item.find('.pie-chart-legend__item-text').text()).toEqual(label); + }); + }); +}); From a15917b1ae033b3f7112c0186052a1cc7725269a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 18 Sep 2021 13:17:04 +0200 Subject: [PATCH 09/11] Fixed tests --- src/visits/helpers/LineChartCard.tsx | 8 ++-- test/visits/helpers/DefaultChart.test.tsx | 49 +++++++++++----------- test/visits/helpers/LineChartCard.test.tsx | 43 ++++++++++--------- 3 files changed, 52 insertions(+), 48 deletions(-) diff --git a/src/visits/helpers/LineChartCard.tsx b/src/visits/helpers/LineChartCard.tsx index 8cca1e9f..f6a8bcd3 100644 --- a/src/visits/helpers/LineChartCard.tsx +++ b/src/visits/helpers/LineChartCard.tsx @@ -1,4 +1,4 @@ -import { useState, useMemo, FC } from 'react'; +import { useState, useMemo } from 'react'; import { Card, CardHeader, @@ -220,7 +220,7 @@ const LineChartCard = ( }, onHover: pointerOnHover, }; - const LineChart: FC = () => ( + const renderLineChart = () => ( {/* It's VERY IMPORTANT to render two different components here, as one has 1 dataset and the other has 2 */} {/* Using the same component causes a crash when switching from 1 to 2 datasets, and then back to 1 dataset */} - {highlightedVisits.length > 0 && } - {highlightedVisits.length === 0 && } + {highlightedVisits.length > 0 && renderLineChart()} + {highlightedVisits.length === 0 && renderLineChart()} ); diff --git a/test/visits/helpers/DefaultChart.test.tsx b/test/visits/helpers/DefaultChart.test.tsx index 2e3a2e7b..fc7a6488 100644 --- a/test/visits/helpers/DefaultChart.test.tsx +++ b/test/visits/helpers/DefaultChart.test.tsx @@ -1,5 +1,5 @@ import { shallow, ShallowWrapper } from 'enzyme'; -import { Doughnut, HorizontalBar } from 'react-chartjs-2'; +import { Doughnut, Bar } from 'react-chartjs-2'; import { keys, values } from 'ramda'; import DefaultChart from '../../../src/visits/helpers/DefaultChart'; import { prettify } from '../../../src/utils/helpers/numbers'; @@ -15,19 +15,18 @@ describe('', () => { afterEach(() => wrapper?.unmount()); it('renders Doughnut when is not a bar chart', () => { - wrapper = shallow(); + wrapper = shallow(); const doughnut = wrapper.find(Doughnut); - const horizontal = wrapper.find(HorizontalBar); + const horizontal = wrapper.find(Bar); const cols = wrapper.find('.col-sm-12'); expect(doughnut).toHaveLength(1); expect(horizontal).toHaveLength(0); - const { labels, datasets } = doughnut.prop('data') as any; - const [{ title, data, backgroundColor, borderColor }] = datasets; - const { legend, scales, ...options } = doughnut.prop('options') ?? {}; + const { labels, datasets } = doughnut.prop('data'); + const [{ data, backgroundColor, borderColor }] = datasets; + const { plugins, scales } = doughnut.prop('options') ?? {}; - expect(title).toEqual('The chart'); expect(labels).toEqual(keys(stats)); expect(data).toEqual(values(stats)); expect(datasets).toHaveLength(1); @@ -45,36 +44,36 @@ describe('', () => { '#463730', ]); expect(borderColor).toEqual('white'); - expect(legend).toEqual({ display: false }); - expect(typeof options.legendCallback).toEqual('function'); + expect(plugins.legend).toEqual({ display: false }); expect(scales).toBeUndefined(); expect(cols).toHaveLength(2); }); it('renders HorizontalBar when is not a bar chart', () => { - wrapper = shallow(); + wrapper = shallow(); const doughnut = wrapper.find(Doughnut); - const horizontal = wrapper.find(HorizontalBar); + const horizontal = wrapper.find(Bar); const cols = wrapper.find('.col-sm-12'); expect(doughnut).toHaveLength(0); expect(horizontal).toHaveLength(1); - const { datasets: [{ backgroundColor, borderColor }] } = horizontal.prop('data') as any; - const { legend, scales, ...options } = horizontal.prop('options') ?? {}; + const { datasets: [{ backgroundColor, borderColor }] } = horizontal.prop('data'); + const { plugins, scales } = horizontal.prop('options') ?? {}; expect(backgroundColor).toEqual(MAIN_COLOR_ALPHA); expect(borderColor).toEqual(MAIN_COLOR); - expect(legend).toEqual({ display: false }); - expect(typeof options.legendCallback).toEqual('boolean'); + expect(plugins.legend).toEqual({ display: false }); expect(scales).toEqual({ - xAxes: [ - { - ticks: { beginAtZero: true, precision: 0, callback: prettify }, - stacked: true, + x: { + beginAtZero: true, + stacked: true, + ticks: { + precision: 0, + callback: prettify, }, - ], - yAxes: [{ stacked: true }], + }, + y: { stacked: true }, }); expect(cols).toHaveLength(1); }); @@ -86,12 +85,12 @@ describe('', () => { [{ bar: 20, foo: 13 }, [ 110, 436 ], [ 13, 20 ]], [ undefined, [ 123, 456 ], undefined ], ])('splits highlighted data from regular data', (highlightedStats, expectedData, expectedHighlightedData) => { - wrapper = shallow(); - const horizontal = wrapper.find(HorizontalBar); + wrapper = shallow(); + const horizontal = wrapper.find(Bar); - const { datasets: [{ data, label }, highlightedData ] } = horizontal.prop('data') as any; + const { datasets: [{ data, label }, highlightedData ] } = horizontal.prop('data'); - expect(label).toEqual(highlightedStats ? 'Non-selected' : 'Visits'); + expect(label).toEqual('Visits'); expect(data).toEqual(expectedData); expectedHighlightedData && expect(highlightedData.data).toEqual(expectedHighlightedData); !expectedHighlightedData && expect(highlightedData).toBeUndefined(); diff --git a/test/visits/helpers/LineChartCard.test.tsx b/test/visits/helpers/LineChartCard.test.tsx index f7948102..0ef89866 100644 --- a/test/visits/helpers/LineChartCard.test.tsx +++ b/test/visits/helpers/LineChartCard.test.tsx @@ -7,6 +7,7 @@ import LineChartCard from '../../../src/visits/helpers/LineChartCard'; import ToggleSwitch from '../../../src/utils/ToggleSwitch'; import { NormalizedVisit } from '../../../src/visits/types'; import { prettify } from '../../../src/utils/helpers/numbers'; +import { pointerOnHover, renderChartLabel } from '../../../src/utils/helpers/charts'; describe('', () => { let wrapper: ShallowWrapper; @@ -54,23 +55,27 @@ describe('', () => { expect(chart.prop('options')).toEqual(expect.objectContaining({ maintainAspectRatio: false, - legend: { display: false }, - scales: { - yAxes: [ - { - ticks: { beginAtZero: true, precision: 0, callback: prettify }, - }, - ], - xAxes: [ - { - scaleLabel: { display: true, labelString: 'Month' }, - }, - ], + plugins: { + legend: { display: false }, + tooltip: { + intersect: false, + axis: 'x', + callbacks: { label: renderChartLabel }, + }, }, - tooltips: expect.objectContaining({ - intersect: false, - axis: 'x', - }), + scales: { + y: { + beginAtZero: true, + ticks: { + precision: 0, + callback: prettify, + }, + }, + x: { + title: { display: true, text: 'Month' }, + }, + }, + onHover: pointerOnHover, })); }); @@ -80,7 +85,7 @@ describe('', () => { ])('renders chart with expected data', (visits, highlightedVisits, expectedLines) => { const wrapper = createWrapper(visits, highlightedVisits); const chart = wrapper.find(Line); - const { datasets } = chart.prop('data') as any; + const { datasets } = chart.prop('data'); expect(datasets).toHaveLength(expectedLines); }); @@ -91,8 +96,8 @@ describe('', () => { Mock.of({ date: '2016-01-01' }), ]); - expect((wrapper.find(Line).prop('data') as any).labels).toHaveLength(2); + expect(wrapper.find(Line).prop('data').labels).toHaveLength(2); wrapper.find(ToggleSwitch).simulate('change'); - expect((wrapper.find(Line).prop('data') as any).labels).toHaveLength(4); + expect(wrapper.find(Line).prop('data').labels).toHaveLength(4); }); }); From 19f0dc2920d561ebe324256da50249fea92a0ab5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 18 Sep 2021 13:18:48 +0200 Subject: [PATCH 10/11] Updated changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ade230a7..8d534920 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * [#469](https://github.com/shlinkio/shlink-web-client/pull/469) Added support `errorCorrection` in QR codes, when consuming Shlink 2.8 or higher. ### Changed -* *Nothing* +* [#408](https://github.com/shlinkio/shlink-web-client/pull/408) Updated to Chart.js 3.5 ### Deprecated * *Nothing* From 1b158b3df4a41eb2bb532bc2b41fe186ccf9487f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 18 Sep 2021 13:22:11 +0200 Subject: [PATCH 11/11] Fixed links to issues defined as PRs in changelog --- CHANGELOG.md | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d534920..1b36d99e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,19 +6,19 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased] ### Added -* [#465](https://github.com/shlinkio/shlink-web-client/pull/465) Added new page to manage domains and their redirects, when consuming Shlink 2.8 or higher. -* [#460](https://github.com/shlinkio/shlink-web-client/pull/460) Added dynamic title on hover for tags with a very long title. -* [#462](https://github.com/shlinkio/shlink-web-client/pull/462) Now it is possible to paste multiple comma-separated tags in the tags selector, making all of them to be added as individual tags. -* [#463](https://github.com/shlinkio/shlink-web-client/pull/463) The strategy to determine which tags to suggest in the TagsSelector during short URL creation, can now be configured: +* [#465](https://github.com/shlinkio/shlink-web-client/issues/465) Added new page to manage domains and their redirects, when consuming Shlink 2.8 or higher. +* [#460](https://github.com/shlinkio/shlink-web-client/issues/460) Added dynamic title on hover for tags with a very long title. +* [#462](https://github.com/shlinkio/shlink-web-client/issues/462) Now it is possible to paste multiple comma-separated tags in the tags selector, making all of them to be added as individual tags. +* [#463](https://github.com/shlinkio/shlink-web-client/issues/463) The strategy to determine which tags to suggest in the TagsSelector during short URL creation, can now be configured: * `startsWith`: Suggests tags that start with the input. This is the default behavior for keep it as it was so far. * `includes`: Suggests tags that contain the input. -* [#464](https://github.com/shlinkio/shlink-web-client/pull/464) Added support to download QR codes. This feature requires an unreleased version of Shlink, so it comes disabled, and will get enabled as soon as Shlink v2.9 is released. -* [#469](https://github.com/shlinkio/shlink-web-client/pull/469) Added support `errorCorrection` in QR codes, when consuming Shlink 2.8 or higher. +* [#464](https://github.com/shlinkio/shlink-web-client/issues/464) Added support to download QR codes. This feature requires an unreleased version of Shlink, so it comes disabled, and will get enabled as soon as Shlink v2.9 is released. +* [#469](https://github.com/shlinkio/shlink-web-client/issues/469) Added support `errorCorrection` in QR codes, when consuming Shlink 2.8 or higher. ### Changed -* [#408](https://github.com/shlinkio/shlink-web-client/pull/408) Updated to Chart.js 3.5 +* [#408](https://github.com/shlinkio/shlink-web-client/issues/408) Updated to Chart.js 3.5 ### Deprecated * *Nothing* @@ -44,9 +44,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * *Nothing* ### Fixed -* [#478](https://github.com/shlinkio/shlink-web-client/pull/478) Fixed tags including special chars not being properly URL encoded before using them as query params. -* [#480](https://github.com/shlinkio/shlink-web-client/pull/480) Fixed servers import on Chromium-based browsers when using windows. -* [#482](https://github.com/shlinkio/shlink-web-client/pull/480) Fixed end date not being set to the end of the day when filtering visits using a "smart filter" (last 7 days, last 30 days, etc). +* [#478](https://github.com/shlinkio/shlink-web-client/issues/478) Fixed tags including special chars not being properly URL encoded before using them as query params. +* [#480](https://github.com/shlinkio/shlink-web-client/issues/480) Fixed servers import on Chromium-based browsers when using windows. +* [#482](https://github.com/shlinkio/shlink-web-client/issues/480) Fixed end date not being set to the end of the day when filtering visits using a "smart filter" (last 7 days, last 30 days, etc). ## [3.2.0] - 2021-07-12 @@ -58,16 +58,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * `SHLINK_SERVER_NAME`: A name you want to give to this server. Defaults to *Shlink* if not provided. * [#432](https://github.com/shlinkio/shlink-web-client/pull/432) Added support to provide the `servers.json` file inside a `conf.d` folder. -* [#440](https://github.com/shlinkio/shlink-web-client/pull/440) Added hint of what visits come potentially from a bot, in the visits table, when consuming Shlink >=2.7. -* [#431](https://github.com/shlinkio/shlink-web-client/pull/431) Added support to filter out visits from potential bots in visits sections, when consuming Shlink >=2.7. -* [#430](https://github.com/shlinkio/shlink-web-client/pull/430) Added support to set new and existing short URLs as crawlable, when consuming Shlink >=2.7. -* [#450](https://github.com/shlinkio/shlink-web-client/pull/450) Improved landing page design. -* [#449](https://github.com/shlinkio/shlink-web-client/pull/449) Improved PWA update banner, allowing to restart the app directly from it without having to close the tab. +* [#440](https://github.com/shlinkio/shlink-web-client/issues/440) Added hint of what visits come potentially from a bot, in the visits table, when consuming Shlink >=2.7. +* [#431](https://github.com/shlinkio/shlink-web-client/issues/431) Added support to filter out visits from potential bots in visits sections, when consuming Shlink >=2.7. +* [#430](https://github.com/shlinkio/shlink-web-client/issues/430) Added support to set new and existing short URLs as crawlable, when consuming Shlink >=2.7. +* [#450](https://github.com/shlinkio/shlink-web-client/issues/450) Improved landing page design. +* [#449](https://github.com/shlinkio/shlink-web-client/issues/449) Improved PWA update banner, allowing to restart the app directly from it without having to close the tab. ### Changed -* [#442](https://github.com/shlinkio/shlink-web-client/pull/442) Visits filtering now goes through the corresponding reducer. -* [#337](https://github.com/shlinkio/shlink-web-client/pull/337) Replaced moment.js with date-fns. -* [#360](https://github.com/shlinkio/shlink-web-client/pull/360) Changed component used to generate a tags selector, switching from `react-tagsinput`, which is no longer maintained, to `react-tag-autocomplete`. +* [#442](https://github.com/shlinkio/shlink-web-client/issues/442) Visits filtering now goes through the corresponding reducer. +* [#337](https://github.com/shlinkio/shlink-web-client/issues/337) Replaced moment.js with date-fns. +* [#360](https://github.com/shlinkio/shlink-web-client/issues/360) Changed component used to generate a tags selector, switching from `react-tagsinput`, which is no longer maintained, to `react-tag-autocomplete`. ### Deprecated * *Nothing* @@ -76,7 +76,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * *Nothing* ### Fixed -* [#438](https://github.com/shlinkio/shlink-web-client/pull/438) Fixed horizontal scrolling in short URLs list on mobile devices when the long URL didn't have words to break. +* [#438](https://github.com/shlinkio/shlink-web-client/issues/438) Fixed horizontal scrolling in short URLs list on mobile devices when the long URL didn't have words to break. ## [3.1.2] - 2021-06-06