From 3fea8b55053cfa297096a2abf3d422905fd80fce Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 13 Sep 2020 10:03:02 +0200 Subject: [PATCH 1/6] Ensured page numbers in paginators are prettified --- src/common/SimplePaginator.tsx | 10 ++++++++-- src/short-urls/Paginator.tsx | 4 ++-- src/utils/helpers/pagination.ts | 4 ++++ 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/common/SimplePaginator.tsx b/src/common/SimplePaginator.tsx index 2b648962..478d19ce 100644 --- a/src/common/SimplePaginator.tsx +++ b/src/common/SimplePaginator.tsx @@ -1,7 +1,13 @@ import React, { FC } from 'react'; import classNames from 'classnames'; import { Pagination, PaginationItem, PaginationLink } from 'reactstrap'; -import { pageIsEllipsis, keyForPage, NumberOrEllipsis, progressivePagination } from '../utils/helpers/pagination'; +import { + pageIsEllipsis, + keyForPage, + NumberOrEllipsis, + progressivePagination, + prettifyPageNumber, +} from '../utils/helpers/pagination'; import './SimplePaginator.scss'; interface SimplePaginatorProps { @@ -29,7 +35,7 @@ const SimplePaginator: FC = ({ pagesCount, currentPage, se disabled={pageIsEllipsis(pageNumber)} active={currentPage === pageNumber} > - {pageNumber} + {prettifyPageNumber(pageNumber)} ))} = pagesCount}> diff --git a/src/short-urls/Paginator.tsx b/src/short-urls/Paginator.tsx index eeccb727..bf6c24e1 100644 --- a/src/short-urls/Paginator.tsx +++ b/src/short-urls/Paginator.tsx @@ -1,7 +1,7 @@ import React from 'react'; import { Link } from 'react-router-dom'; import { Pagination, PaginationItem, PaginationLink } from 'reactstrap'; -import { pageIsEllipsis, keyForPage, progressivePagination } from '../utils/helpers/pagination'; +import { pageIsEllipsis, keyForPage, progressivePagination, prettifyPageNumber } from '../utils/helpers/pagination'; import { ShlinkPaginator } from '../utils/services/types'; import './Paginator.scss'; @@ -28,7 +28,7 @@ const Paginator = ({ paginator, serverId }: PaginatorProps) => { tag={Link} to={`/server/${serverId}/list-short-urls/${pageNumber}`} > - {pageNumber} + {prettifyPageNumber(pageNumber)} )); diff --git a/src/utils/helpers/pagination.ts b/src/utils/helpers/pagination.ts index eca76cb7..0fa2b87b 100644 --- a/src/utils/helpers/pagination.ts +++ b/src/utils/helpers/pagination.ts @@ -1,4 +1,5 @@ import { max, min, range } from 'ramda'; +import { prettify } from './numbers'; const DELTA = 2; @@ -29,4 +30,7 @@ export const progressivePagination = (currentPage: number, pageCount: number): N export const pageIsEllipsis = (pageNumber: NumberOrEllipsis): pageNumber is Ellipsis => pageNumber === ELLIPSIS; +export const prettifyPageNumber = (pageNumber: NumberOrEllipsis): string => + pageIsEllipsis(pageNumber) ? pageNumber : prettify(pageNumber); + export const keyForPage = (pageNumber: NumberOrEllipsis, index: number) => !pageIsEllipsis(pageNumber) ? `${pageNumber}` : `${pageNumber}_${index}`; From fc9341f63134c10902f15a4501fbb4b224d99640 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 13 Sep 2020 11:11:17 +0200 Subject: [PATCH 2/6] Added number formatting to visits line chart --- src/visits/helpers/LineChartCard.tsx | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/visits/helpers/LineChartCard.tsx b/src/visits/helpers/LineChartCard.tsx index a600d364..42713bdf 100644 --- a/src/visits/helpers/LineChartCard.tsx +++ b/src/visits/helpers/LineChartCard.tsx @@ -11,13 +11,14 @@ import { import { Line } from 'react-chartjs-2'; import { always, cond, reverse } from 'ramda'; import moment from 'moment'; -import { ChartData, ChartDataSets } from 'chart.js'; +import { ChartData, ChartDataSets, ChartTooltipItem, 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 './LineChartCard.scss'; +import { prettify } from '../../utils/helpers/numbers'; interface LineChartCardProps { title: string; @@ -137,13 +138,18 @@ const LineChartCard = ({ title, visits, highlightedVisits, highlightedLabel = 'S highlightedVisits.length > 0 && generateDataset(groupedHighlighted, highlightedLabel, '#F77F28'), ].filter(Boolean) as ChartDataSets[], }; - const options = { + const options: ChartOptions = { maintainAspectRatio: false, legend: { display: false }, scales: { yAxes: [ { - ticks: { beginAtZero: true, precision: 0 }, + ticks: { + beginAtZero: true, + // @ts-expect-error + precision: 0, + callback: prettify, + }, }, ], xAxes: [ @@ -154,7 +160,15 @@ const LineChartCard = ({ title, visits, highlightedVisits, highlightedLabel = 'S }, tooltips: { intersect: false, + // @ts-expect-error axis: 'x', + callbacks: { + label({ datasetIndex, yLabel }: ChartTooltipItem, data: ChartData) { + const datasetLabel = datasetIndex !== undefined && data.datasets?.[datasetIndex]?.label || ''; + + return `${datasetLabel}: ${prettify(Number(yLabel))}`; + }, + }, }, }; From 67495fa3022572808a0abcc0dcd52d42751cd930 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 15 Sep 2020 22:22:56 +0200 Subject: [PATCH 3/6] Added number formatting to charts --- src/visits/helpers/DefaultChart.tsx | 31 +++++++++++++++++++++++++--- src/visits/helpers/LineChartCard.tsx | 2 +- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/visits/helpers/DefaultChart.tsx b/src/visits/helpers/DefaultChart.tsx index 2d8f1094..48a33f78 100644 --- a/src/visits/helpers/DefaultChart.tsx +++ b/src/visits/helpers/DefaultChart.tsx @@ -2,10 +2,11 @@ import React, { ChangeEvent, useRef } from 'react'; import { Doughnut, HorizontalBar } from 'react-chartjs-2'; import { keys, values } from 'ramda'; import classNames from 'classnames'; -import Chart, { ChartData, ChartDataSets, ChartOptions } from 'chart.js'; +import Chart, { ChartData, ChartDataSets, ChartOptions, ChartTooltipItem } from 'chart.js'; import { fillTheGaps } from '../../utils/helpers/visits'; import { Stats } from '../types'; import './DefaultChart.scss'; +import { prettify } from '../../utils/helpers/numbers'; export interface DefaultChartProps { title: Function | string; @@ -124,7 +125,13 @@ const DefaultChart = ( scales: !isBarChart ? undefined : { xAxes: [ { - ticks: { beginAtZero: true, precision: 0, max } as any, + ticks: { + beginAtZero: true, + // @ts-expect-error + precision: 0, + callback: prettify, + max, + }, stacked: true, }, ], @@ -132,9 +139,27 @@ const DefaultChart = ( }, tooltips: { intersect: !isBarChart, - // Do not show tooltip on items with empty label when in a bar chart filter: ({ yLabel }) => !isBarChart || yLabel !== '', + callbacks: { + ...isBarChart ? {} : { + label({ 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))}`; + }, + }, + ...!isBarChart ? {} : { + label({ datasetIndex, xLabel }: ChartTooltipItem, { datasets }: ChartData) { + const datasetLabel = datasetIndex !== undefined && datasets?.[datasetIndex]?.label || ''; + + return `${datasetLabel}: ${prettify(Number(xLabel))}`; + }, + }, + }, }, onHover: !isBarChart ? undefined : ((e: ChangeEvent, chartElement: HorizontalBar[] | Doughnut[]) => { const { target } = e; diff --git a/src/visits/helpers/LineChartCard.tsx b/src/visits/helpers/LineChartCard.tsx index 42713bdf..bdbcaec8 100644 --- a/src/visits/helpers/LineChartCard.tsx +++ b/src/visits/helpers/LineChartCard.tsx @@ -17,8 +17,8 @@ import { fillTheGaps } from '../../utils/helpers/visits'; import { useToggle } from '../../utils/helpers/hooks'; import { rangeOf } from '../../utils/utils'; import ToggleSwitch from '../../utils/ToggleSwitch'; -import './LineChartCard.scss'; import { prettify } from '../../utils/helpers/numbers'; +import './LineChartCard.scss'; interface LineChartCardProps { title: string; From 871868f0a49f4e59b2be89e3af0c90631672825d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 15 Sep 2020 22:27:01 +0200 Subject: [PATCH 4/6] Moved common rendering chart labels code to external module for reuse --- src/utils/helpers/visits.ts | 25 +++++++++++++++++++++++++ src/visits/helpers/DefaultChart.tsx | 24 ++++-------------------- src/visits/helpers/LineChartCard.tsx | 10 +++------- 3 files changed, 32 insertions(+), 27 deletions(-) diff --git a/src/utils/helpers/visits.ts b/src/utils/helpers/visits.ts index ea45421a..7a2bd7e2 100644 --- a/src/utils/helpers/visits.ts +++ b/src/utils/helpers/visits.ts @@ -1,7 +1,9 @@ import bowser from 'bowser'; import { zipObj } from 'ramda'; +import { ChartData, ChartTooltipItem } from 'chart.js'; import { Empty, hasValue } from '../utils'; import { Stats, UserAgent } from '../../visits/types'; +import { prettify } from './numbers'; const DEFAULT = 'Others'; const BROWSERS_WHITELIST = [ @@ -40,3 +42,26 @@ export const extractDomain = (url: string | Empty): string => { export const fillTheGaps = (stats: Stats, labels: string[]): number[] => Object.values({ ...zipObj(labels, labels.map(() => 0)), ...stats }); + +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))}`; +}; + +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))}`; +}; diff --git a/src/visits/helpers/DefaultChart.tsx b/src/visits/helpers/DefaultChart.tsx index 48a33f78..01a38fb2 100644 --- a/src/visits/helpers/DefaultChart.tsx +++ b/src/visits/helpers/DefaultChart.tsx @@ -2,11 +2,11 @@ import React, { ChangeEvent, useRef } from 'react'; import { Doughnut, HorizontalBar } from 'react-chartjs-2'; import { keys, values } from 'ramda'; import classNames from 'classnames'; -import Chart, { ChartData, ChartDataSets, ChartOptions, ChartTooltipItem } from 'chart.js'; -import { fillTheGaps } from '../../utils/helpers/visits'; +import Chart, { ChartData, ChartDataSets, ChartOptions } from 'chart.js'; +import { fillTheGaps, renderDoughnutChartLabel, renderNonDoughnutChartLabel } from '../../utils/helpers/visits'; import { Stats } from '../types'; -import './DefaultChart.scss'; import { prettify } from '../../utils/helpers/numbers'; +import './DefaultChart.scss'; export interface DefaultChartProps { title: Function | string; @@ -142,23 +142,7 @@ const DefaultChart = ( // Do not show tooltip on items with empty label when in a bar chart filter: ({ yLabel }) => !isBarChart || yLabel !== '', callbacks: { - ...isBarChart ? {} : { - label({ 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))}`; - }, - }, - ...!isBarChart ? {} : { - label({ datasetIndex, xLabel }: ChartTooltipItem, { datasets }: ChartData) { - const datasetLabel = datasetIndex !== undefined && datasets?.[datasetIndex]?.label || ''; - - return `${datasetLabel}: ${prettify(Number(xLabel))}`; - }, - }, + label: isBarChart ? renderNonDoughnutChartLabel('xLabel') : renderDoughnutChartLabel, }, }, onHover: !isBarChart ? undefined : ((e: ChangeEvent, chartElement: HorizontalBar[] | Doughnut[]) => { diff --git a/src/visits/helpers/LineChartCard.tsx b/src/visits/helpers/LineChartCard.tsx index bdbcaec8..bcac1b41 100644 --- a/src/visits/helpers/LineChartCard.tsx +++ b/src/visits/helpers/LineChartCard.tsx @@ -11,9 +11,9 @@ import { import { Line } from 'react-chartjs-2'; import { always, cond, reverse } from 'ramda'; import moment from 'moment'; -import { ChartData, ChartDataSets, ChartTooltipItem, ChartOptions } from 'chart.js'; +import { ChartData, ChartDataSets, ChartOptions } from 'chart.js'; import { NormalizedVisit, Stats } from '../types'; -import { fillTheGaps } from '../../utils/helpers/visits'; +import { fillTheGaps, renderNonDoughnutChartLabel } from '../../utils/helpers/visits'; import { useToggle } from '../../utils/helpers/hooks'; import { rangeOf } from '../../utils/utils'; import ToggleSwitch from '../../utils/ToggleSwitch'; @@ -163,11 +163,7 @@ const LineChartCard = ({ title, visits, highlightedVisits, highlightedLabel = 'S // @ts-expect-error axis: 'x', callbacks: { - label({ datasetIndex, yLabel }: ChartTooltipItem, data: ChartData) { - const datasetLabel = datasetIndex !== undefined && data.datasets?.[datasetIndex]?.label || ''; - - return `${datasetLabel}: ${prettify(Number(yLabel))}`; - }, + label: renderNonDoughnutChartLabel('yLabel'), }, }, }; From 51556d76acb5ba3b53273b2aa08992eba8ae6350 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 17 Sep 2020 18:05:26 +0200 Subject: [PATCH 5/6] Fixed tests --- src/utils/helpers/visits.ts | 4 +--- test/visits/helpers/DefaultChart.test.tsx | 3 ++- test/visits/helpers/LineChartCard.test.tsx | 21 +++++++++++---------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/utils/helpers/visits.ts b/src/utils/helpers/visits.ts index 7a2bd7e2..89dc1d57 100644 --- a/src/utils/helpers/visits.ts +++ b/src/utils/helpers/visits.ts @@ -35,9 +35,7 @@ export const extractDomain = (url: string | Empty): string => { return 'Direct'; } - const domain = url.includes('://') ? url.split('/')[2] : url.split('/')[0]; - - return domain.split(':')[0]; + return url.split('/')[url.includes('://') ? 2 : 0]?.split(':')[0] ?? ''; }; export const fillTheGaps = (stats: Stats, labels: string[]): number[] => diff --git a/test/visits/helpers/DefaultChart.test.tsx b/test/visits/helpers/DefaultChart.test.tsx index 43312cec..4ac0056f 100644 --- a/test/visits/helpers/DefaultChart.test.tsx +++ b/test/visits/helpers/DefaultChart.test.tsx @@ -3,6 +3,7 @@ import { shallow, ShallowWrapper } from 'enzyme'; import { Doughnut, HorizontalBar } from 'react-chartjs-2'; import { keys, values } from 'ramda'; import DefaultChart from '../../../src/visits/helpers/DefaultChart'; +import { prettify } from '../../../src/utils/helpers/numbers'; describe('', () => { let wrapper: ShallowWrapper; @@ -69,7 +70,7 @@ describe('', () => { expect(scales).toEqual({ xAxes: [ { - ticks: { beginAtZero: true, precision: 0 }, + ticks: { beginAtZero: true, precision: 0, callback: prettify }, stacked: true, }, ], diff --git a/test/visits/helpers/LineChartCard.test.tsx b/test/visits/helpers/LineChartCard.test.tsx index 1fc0113a..8aaf2a48 100644 --- a/test/visits/helpers/LineChartCard.test.tsx +++ b/test/visits/helpers/LineChartCard.test.tsx @@ -6,11 +6,12 @@ import moment from 'moment'; import { Mock } from 'ts-mockery'; import LineChartCard from '../../../src/visits/helpers/LineChartCard'; import ToggleSwitch from '../../../src/utils/ToggleSwitch'; -import { Visit } from '../../../src/visits/types'; +import { NormalizedVisit } from '../../../src/visits/types'; +import { prettify } from '../../../src/utils/helpers/numbers'; describe('', () => { let wrapper: ShallowWrapper; - const createWrapper = (visits: Visit[] = [], highlightedVisits: Visit[] = []) => { + const createWrapper = (visits: NormalizedVisit[] = [], highlightedVisits: NormalizedVisit[] = []) => { wrapper = shallow(); return wrapper; @@ -34,7 +35,7 @@ describe('', () => { [[{ date: moment().subtract(7, 'month').format() }], 'monthly' ], [[{ date: moment().subtract(1, 'year').format() }], 'monthly' ], ])('renders group menu and selects proper grouping item based on visits dates', (visits, expectedActiveItem) => { - const wrapper = createWrapper(visits.map((visit) => Mock.of(visit))); + const wrapper = createWrapper(visits.map((visit) => Mock.of(visit))); const items = wrapper.find(DropdownItem); expect(items).toHaveLength(4); @@ -58,7 +59,7 @@ describe('', () => { scales: { yAxes: [ { - ticks: { beginAtZero: true, precision: 0 }, + ticks: { beginAtZero: true, precision: 0, callback: prettify }, }, ], xAxes: [ @@ -67,16 +68,16 @@ describe('', () => { }, ], }, - tooltips: { + tooltips: expect.objectContaining({ intersect: false, axis: 'x', - }, + }), }); }); it.each([ - [[ Mock.of({}) ], [], 1 ], - [[ Mock.of({}) ], [ Mock.of({}) ], 2 ], + [[ Mock.of({}) ], [], 1 ], + [[ Mock.of({}) ], [ Mock.of({}) ], 2 ], ])('renders chart with expected data', (visits, highlightedVisits, expectedLines) => { const wrapper = createWrapper(visits, highlightedVisits); const chart = wrapper.find(Line); @@ -87,8 +88,8 @@ describe('', () => { it('includes stats for visits with no dates if selected', () => { const wrapper = createWrapper([ - Mock.of({ date: '2016-04-01' }), - Mock.of({ date: '2016-01-01' }), + Mock.of({ date: '2016-04-01' }), + Mock.of({ date: '2016-01-01' }), ]); expect((wrapper.find(Line).prop('data') as any).labels).toHaveLength(2); From b52e40edd3e71cdec0223cd804a1dbe3a4b31707 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 17 Sep 2020 18:11:03 +0200 Subject: [PATCH 6/6] Updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d90721a..3391cf17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), #### Changed * [#150](https://github.com/shlinkio/shlink-web-client/issues/150) The list of short URLs is now ordered by the creation date, showing newest results first. +* [#248](https://github.com/shlinkio/shlink-web-client/issues/248) Numbers displayed application-wide are now prettified. * [#40](https://github.com/shlinkio/shlink-web-client/issues/40) Migrated project to TypeScript. #### Deprecated