diff --git a/.eslintrc b/.eslintrc index 7ac3f4ec..b14ea6b0 100644 --- a/.eslintrc +++ b/.eslintrc @@ -26,6 +26,7 @@ "no-console": "warn", "template-curly-spacing": ["error", "never"], "no-warning-comments": "off", + "no-magic-numbers": "off", "no-undefined": "off", "indent": ["error", 2, { "SwitchCase": 1 diff --git a/package.json b/package.json index e9c6f15e..457521b4 100644 --- a/package.json +++ b/package.json @@ -38,13 +38,13 @@ "prop-types": "^15.6.2", "qs": "^6.5.2", "ramda": "^0.26.1", - "react": "^16.7.0", + "react": "^16.8.0", "react-autosuggest": "^9.4.0", "react-chartjs-2": "^2.7.4", "react-color": "^2.14.1", "react-copy-to-clipboard": "^5.0.1", "react-datepicker": "~1.5.0", - "react-dom": "^16.7.0", + "react-dom": "^16.8.0", "react-leaflet": "^2.2.1", "react-moment": "^0.7.6", "react-redux": "^5.0.7", diff --git a/src/utils/utils.js b/src/utils/utils.js index 60f33c16..a49cc604 100644 --- a/src/utils/utils.js +++ b/src/utils/utils.js @@ -4,7 +4,9 @@ import marker from 'leaflet/dist/images/marker-icon.png'; import markerShadow from 'leaflet/dist/images/marker-shadow.png'; import { range } from 'ramda'; +const TEN_ROUNDING_NUMBER = 10; const DEFAULT_TIMEOUT_DELAY = 2000; +const { ceil } = Math; export const stateFlagTimeout = (setTimeout) => ( setState, @@ -40,3 +42,5 @@ export const fixLeafletIcons = () => { }; export const rangeOf = (size, mappingFn, startAt = 1) => range(startAt, size + 1).map(mappingFn); + +export const roundTen = (number) => ceil(number / TEN_ROUNDING_NUMBER) * TEN_ROUNDING_NUMBER; diff --git a/src/visits/GraphCard.js b/src/visits/GraphCard.js index 2cbbda75..44facee7 100644 --- a/src/visits/GraphCard.js +++ b/src/visits/GraphCard.js @@ -1,18 +1,16 @@ -import { Card, CardHeader, CardBody } from 'reactstrap'; +import { Card, CardHeader, CardBody, CardFooter } from 'reactstrap'; import { Doughnut, HorizontalBar } from 'react-chartjs-2'; import PropTypes from 'prop-types'; import React from 'react'; import { keys, values } from 'ramda'; const propTypes = { - title: PropTypes.string, - children: PropTypes.node, + title: PropTypes.oneOfType([ PropTypes.string, PropTypes.node ]), + footer: PropTypes.oneOfType([ PropTypes.string, PropTypes.node ]), isBarChart: PropTypes.bool, stats: PropTypes.object, - matchMedia: PropTypes.func, -}; -const defaultProps = { - matchMedia: global.window ? global.window.matchMedia : () => {}, + max: PropTypes.number, + redraw: PropTypes.bool, }; const generateGraphData = (title, isBarChart, labels, data) => ({ @@ -36,62 +34,42 @@ const generateGraphData = (title, isBarChart, labels, data) => ({ ], }); -const determineGraphAspectRatio = (barsCount, isBarChart, matchMedia) => { - const determineAspectRationModifier = () => { - switch (true) { - case matchMedia('(max-width: 1200px)').matches: - return 1.5; // eslint-disable-line no-magic-numbers - case matchMedia('(max-width: 992px)').matches: - return 1.75; // eslint-disable-line no-magic-numbers - case matchMedia('(max-width: 768px)').matches: - return 2; // eslint-disable-line no-magic-numbers - case matchMedia('(max-width: 576px)').matches: - return 2.25; // eslint-disable-line no-magic-numbers - default: - return 1; - } - }; +const dropLabelIfHidden = (label) => label.startsWith('hidden') ? '' : label; - const MAX_BARS_WITHOUT_HEIGHT = 20; - const DEFAULT_ASPECT_RATION = 2; - const shouldCalculateAspectRatio = isBarChart && barsCount > MAX_BARS_WITHOUT_HEIGHT; - - return shouldCalculateAspectRatio - ? MAX_BARS_WITHOUT_HEIGHT / determineAspectRationModifier() * DEFAULT_ASPECT_RATION / barsCount - : DEFAULT_ASPECT_RATION; -}; - -const renderGraph = (title, isBarChart, stats, matchMedia) => { +const renderGraph = (title, isBarChart, stats, max, redraw) => { const Component = isBarChart ? HorizontalBar : Doughnut; - const labels = keys(stats); + const labels = keys(stats).map(dropLabelIfHidden); const data = values(stats); - const aspectRatio = determineGraphAspectRatio(labels.length, isBarChart, matchMedia); const options = { - aspectRatio, legend: isBarChart ? { display: false } : { position: 'right' }, scales: isBarChart ? { xAxes: [ { - ticks: { beginAtZero: true }, + ticks: { beginAtZero: true, max }, }, ], } : null, tooltips: { intersect: !isBarChart, + + // Do not show tooltip on items with empty label when in a bar chart + filter: ({ yLabel }) => !isBarChart || yLabel !== '', }, }; + const graphData = generateGraphData(title, isBarChart, labels, data); + const height = labels.length < 20 ? null : labels.length * 8; - return ; + return ; }; -const GraphCard = ({ title, children, isBarChart, stats, matchMedia }) => ( +const GraphCard = ({ title, footer, isBarChart, stats, max, redraw = false }) => ( - {children || title} - {renderGraph(title, isBarChart, stats, matchMedia)} + {title} + {renderGraph(title, isBarChart, stats, max, redraw)} + {footer && {footer}} ); GraphCard.propTypes = propTypes; -GraphCard.defaultProps = defaultProps; export default GraphCard; diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js index 6c2f9bf8..0d880d9f 100644 --- a/src/visits/ShortUrlVisits.js +++ b/src/visits/ShortUrlVisits.js @@ -94,6 +94,7 @@ const ShortUrlVisits = ({ processStatsFromVisits }) => class ShortUrlVisits exte
class ShortUrlVisits exte mapLocations.length > 0 && ] - } + extraHeaderContent={( + mapLocations.length > 0 && + )} sortingItems={{ name: 'City name', amount: 'Visits amount', diff --git a/src/visits/SortableBarGraph.js b/src/visits/SortableBarGraph.js index 1a30218b..9e03a6a9 100644 --- a/src/visits/SortableBarGraph.js +++ b/src/visits/SortableBarGraph.js @@ -1,61 +1,139 @@ import React from 'react'; import PropTypes from 'prop-types'; -import { fromPairs, head, keys, pipe, prop, reverse, sortBy, toLower, toPairs, type } from 'ramda'; +import { fromPairs, head, keys, pipe, prop, reverse, sortBy, splitEvery, toLower, toPairs, type } from 'ramda'; +import { Pagination, PaginationItem, PaginationLink } from 'reactstrap'; import SortingDropdown from '../utils/SortingDropdown'; +import PaginationDropdown from '../utils/PaginationDropdown'; +import { rangeOf, roundTen } from '../utils/utils'; import GraphCard from './GraphCard'; +const { max } = Math; const toLowerIfString = (value) => type(value) === 'String' ? toLower(value) : value; +const pickValueFromPair = ([ , value ]) => value; export default class SortableBarGraph extends React.Component { static propTypes = { stats: PropTypes.object.isRequired, title: PropTypes.string.isRequired, sortingItems: PropTypes.object.isRequired, - extraHeaderContent: PropTypes.arrayOf(PropTypes.func), + extraHeaderContent: PropTypes.node, + supportPagination: PropTypes.bool, }; state = { orderField: undefined, orderDir: undefined, + currentPage: 1, + itemsPerPage: Infinity, }; + redraw = false; - render() { - const { stats, sortingItems, title, extraHeaderContent } = this.props; - const sortStats = () => { - if (!this.state.orderField) { - return stats; - } + doRedraw() { + const prev = this.redraw; - const sortedPairs = sortBy( - pipe( - prop(this.state.orderField === head(keys(sortingItems)) ? 0 : 1), - toLowerIfString - ), - toPairs(stats) - ); + this.redraw = false; - return fromPairs(this.state.orderDir === 'ASC' ? sortedPairs : reverse(sortedPairs)); + return prev; + } + + determineStats(stats, sortingItems) { + const sortedPairs = !this.state.orderField ? toPairs(stats) : sortBy( + pipe( + prop(this.state.orderField === head(keys(sortingItems)) ? 0 : 1), + toLowerIfString + ), + toPairs(stats) + ); + const directionalPairs = !this.state.orderDir || this.state.orderDir === 'ASC' ? sortedPairs : reverse(sortedPairs); + + if (directionalPairs.length <= this.state.itemsPerPage) { + return { currentPageStats: fromPairs(directionalPairs) }; + } + + const pages = splitEvery(this.state.itemsPerPage, directionalPairs); + + return { + currentPageStats: fromPairs(this.determineCurrentPagePairs(pages)), + pagination: this.renderPagination(pages.length), + max: roundTen(max(...directionalPairs.map(pickValueFromPair))), }; + } + + determineCurrentPagePairs(pages) { + const page = pages[this.state.currentPage - 1]; + + if (this.state.currentPage < pages.length) { + return page; + } + + const firstPageLength = pages[0].length; + + // Using the "hidden" key, the chart will just replace the label by an empty string + return [ ...page, ...rangeOf(firstPageLength - page.length, (i) => [ `hidden_${i}`, 0 ]) ]; + } + + renderPagination(pagesCount) { + const { currentPage } = this.state; return ( - + + + this.setState({ currentPage: currentPage - 1 })} /> + + {rangeOf(pagesCount, (page) => ( + + this.setState({ currentPage: page })}>{page} + + ))} + = pagesCount}> + this.setState({ currentPage: currentPage + 1 })} /> + + + ); + } + + render() { + const { stats, sortingItems, title, extraHeaderContent, supportPagination = true } = this.props; + const computedTitle = ( + {title}
this.setState({ orderField, orderDir })} + onChange={(orderField, orderDir) => this.setState({ orderField, orderDir, currentPage: 1 })} />
- {extraHeaderContent && extraHeaderContent.map((content, index) => ( -
- {content()} + {supportPagination && ( +
+ { + this.redraw = true; + this.setState({ itemsPerPage, currentPage: 1 }); + }} + />
- ))} - + )} + {extraHeaderContent &&
{extraHeaderContent}
} + + ); + const { currentPageStats, pagination, max } = this.determineStats(stats, sortingItems); + + return ( + ); } } diff --git a/test/short-urls/helpers/ShortUrlsRow.test.js b/test/short-urls/helpers/ShortUrlsRow.test.js index d8978788..48c32b5e 100644 --- a/test/short-urls/helpers/ShortUrlsRow.test.js +++ b/test/short-urls/helpers/ShortUrlsRow.test.js @@ -53,7 +53,7 @@ describe('', () => { }); it('renders long URL in third row', () => { - const col = wrapper.find('td').at(2); // eslint-disable-line no-magic-numbers + const col = wrapper.find('td').at(2); const link = col.find(ExternalLink); expect(link.prop('href')).toEqual(shortUrl.longUrl); @@ -61,7 +61,7 @@ describe('', () => { describe('renders list of tags in fourth row', () => { it('with tags', () => { - const col = wrapper.find('td').at(3); // eslint-disable-line no-magic-numbers + const col = wrapper.find('td').at(3); const tags = col.find(Tag); expect(tags).toHaveLength(shortUrl.tags.length); @@ -75,20 +75,20 @@ describe('', () => { it('without tags', () => { wrapper.setProps({ shortUrl: assoc('tags', [], shortUrl) }); - const col = wrapper.find('td').at(3); // eslint-disable-line no-magic-numbers + const col = wrapper.find('td').at(3); expect(col.text()).toContain('No tags'); }); }); it('renders visits count in fifth row', () => { - const col = wrapper.find('td').at(4); // eslint-disable-line no-magic-numbers + const col = wrapper.find('td').at(4); expect(col.text()).toEqual(toString(shortUrl.visitsCount)); }); it('updates state when copied to clipboard', () => { - const col = wrapper.find('td').at(5); // eslint-disable-line no-magic-numbers + const col = wrapper.find('td').at(5); const menu = col.find(ShortUrlsRowMenu); expect(menu).toHaveLength(1); @@ -98,7 +98,6 @@ describe('', () => { }); it('shows copy hint when state prop is true', () => { - // eslint-disable-next-line no-magic-numbers const isHidden = () => wrapper.find('td').at(5).find('.short-urls-row__copy-hint').prop('hidden'); expect(isHidden()).toEqual(true); diff --git a/test/utils/utils.test.js b/test/utils/utils.test.js index e4c47a90..3a9c507a 100644 --- a/test/utils/utils.test.js +++ b/test/utils/utils.test.js @@ -8,6 +8,7 @@ import { determineOrderDir, fixLeafletIcons, rangeOf, + roundTen, } from '../../src/utils/utils'; describe('utils', () => { @@ -87,4 +88,21 @@ describe('utils', () => { ]); }); }); + + describe('roundTen', () => { + it('rounds provided number to the next multiple of ten', () => { + const expectationsPairs = [ + [ 10, 10 ], + [ 12, 20 ], + [ 158, 160 ], + [ 5, 10 ], + [ -42, -40 ], + ]; + + expect.assertions(expectationsPairs.length); + expectationsPairs.forEach(([ number, expected ]) => { + expect(roundTen(number)).toEqual(expected); + }); + }); + }); }); diff --git a/test/visits/SortableBarGraph.test.js b/test/visits/SortableBarGraph.test.js index 3dbcc337..02090ce7 100644 --- a/test/visits/SortableBarGraph.test.js +++ b/test/visits/SortableBarGraph.test.js @@ -15,7 +15,7 @@ describe('', () => { Foo: 100, Bar: 50, }; - const createWrapper = (extraHeaderContent = []) => { + const createWrapper = (extraHeaderContent) => { wrapper = shallow( ); @@ -53,24 +53,19 @@ describe('', () => { }; }); - // eslint-disable-next-line no-magic-numbers it('name - ASC', (done) => assert('name', 'ASC', [ 'Bar', 'Foo' ], [ 50, 100 ], done)); - - // eslint-disable-next-line no-magic-numbers it('name - DESC', (done) => assert('name', 'DESC', [ 'Foo', 'Bar' ], [ 100, 50 ], done)); - - // eslint-disable-next-line no-magic-numbers it('value - ASC', (done) => assert('value', 'ASC', [ 'Bar', 'Foo' ], [ 50, 100 ], done)); - - // eslint-disable-next-line no-magic-numbers it('value - DESC', (done) => assert('value', 'DESC', [ 'Foo', 'Bar' ], [ 100, 50 ], done)); }); it('renders extra header functions', () => { - const wrapper = createWrapper([ - () => Foo, - () => Bar, - ]); + const wrapper = createWrapper(( + + Foo + Bar + + )); expect(wrapper.find('.foo-span')).toHaveLength(1); expect(wrapper.find('.bar-span')).toHaveLength(1); diff --git a/yarn.lock b/yarn.lock index 491235a6..44d72b70 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8064,14 +8064,15 @@ react-dev-utils@^7.0.1: strip-ansi "4.0.0" text-table "0.2.0" -react-dom@^16.7.0: - version "16.7.0" - resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.7.0.tgz#a17b2a7ca89ee7390bc1ed5eb81783c7461748b8" +react-dom@^16.8.0: + version "16.8.4" + resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.8.4.tgz#1061a8e01a2b3b0c8160037441c3bf00a0e3bc48" + integrity sha512-Ob2wK7XG2tUDt7ps7LtLzGYYB6DXMCLj0G5fO6WeEICtT4/HdpOi7W/xLzZnR6RCG1tYza60nMdqtxzA8FaPJQ== dependencies: loose-envify "^1.1.0" object-assign "^4.1.1" prop-types "^15.6.2" - scheduler "^0.12.0" + scheduler "^0.13.4" react-error-overlay@^5.1.2: version "5.1.2" @@ -8187,14 +8188,15 @@ react-transition-group@^2.3.1: prop-types "^15.6.2" react-lifecycles-compat "^3.0.4" -react@^16.7.0: - version "16.7.0" - resolved "https://registry.yarnpkg.com/react/-/react-16.7.0.tgz#b674ec396b0a5715873b350446f7ea0802ab6381" +react@^16.8.0: + version "16.8.4" + resolved "https://registry.yarnpkg.com/react/-/react-16.8.4.tgz#fdf7bd9ae53f03a9c4cd1a371432c206be1c4768" + integrity sha512-0GQ6gFXfUH7aZcjGVymlPOASTuSjlQL4ZtVC5YKH+3JL6bBLCVO21DknzmaPlI90LN253ojj02nsapy+j7wIjg== dependencies: loose-envify "^1.1.0" object-assign "^4.1.1" prop-types "^15.6.2" - scheduler "^0.12.0" + scheduler "^0.13.4" reactcss@^1.2.0: version "1.2.3" @@ -8731,6 +8733,14 @@ scheduler@^0.12.0: loose-envify "^1.1.0" object-assign "^4.1.1" +scheduler@^0.13.4: + version "0.13.4" + resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.13.4.tgz#8fef05e7a3580c76c0364d2df5e550e4c9140298" + integrity sha512-cvSOlRPxOHs5dAhP9yiS/6IDmVAVxmk33f0CtTJRkmUWcb1Us+t7b1wqdzoC0REw2muC9V5f1L/w5R5uKGaepA== + dependencies: + loose-envify "^1.1.0" + object-assign "^4.1.1" + schema-utils@^0.4.4: version "0.4.7" resolved "https://registry.yarnpkg.com/schema-utils/-/schema-utils-0.4.7.tgz#ba74f597d2be2ea880131746ee17d0a093c68187"