From 83704ca4b5e914078f20560f46c6a8cb29d96d2f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 9 Mar 2019 12:19:33 +0100 Subject: [PATCH 01/10] Created rangeOf helper function which does a range + map --- src/index.scss | 4 ++++ src/short-urls/Paginator.js | 4 ++-- src/utils/services/ColorGenerator.js | 8 ++----- src/utils/utils.js | 3 +++ test/tags/TagsList.test.js | 5 +++-- test/utils/utils.test.js | 32 +++++++++++++++++++++++++++- 6 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/index.scss b/src/index.scss index cc361862..1d07ba5d 100644 --- a/src/index.scss +++ b/src/index.scss @@ -51,3 +51,7 @@ body, margin: 0 auto !important; } } + +.pagination .page-link { + cursor: pointer; +} diff --git a/src/short-urls/Paginator.js b/src/short-urls/Paginator.js index 3890c27b..c1458c02 100644 --- a/src/short-urls/Paginator.js +++ b/src/short-urls/Paginator.js @@ -2,7 +2,7 @@ import React from 'react'; import { Link } from 'react-router-dom'; import { Pagination, PaginationItem, PaginationLink } from 'reactstrap'; import PropTypes from 'prop-types'; -import { range } from 'ramda'; +import { rangeOf } from '../utils/utils'; const propTypes = { serverId: PropTypes.string.isRequired, @@ -20,7 +20,7 @@ export default function Paginator({ paginator = {}, serverId }) { } const renderPages = () => - range(1, pagesCount + 1).map((pageNumber) => ( + rangeOf(pagesCount, (pageNumber) => ( - `#${ - range(0, HEX_COLOR_LENGTH) - .map(() => letters[floor(random() * letters.length)]) - .join('') - }`; + `#${rangeOf(HEX_COLOR_LENGTH, () => letters[floor(random() * letters.length)]).join('')}`; const normalizeKey = (key) => key.toLowerCase().trim(); export default class ColorGenerator { diff --git a/src/utils/utils.js b/src/utils/utils.js index e9d60fbd..60f33c16 100644 --- a/src/utils/utils.js +++ b/src/utils/utils.js @@ -2,6 +2,7 @@ import L from 'leaflet'; import marker2x from 'leaflet/dist/images/marker-icon-2x.png'; import marker from 'leaflet/dist/images/marker-icon.png'; import markerShadow from 'leaflet/dist/images/marker-shadow.png'; +import { range } from 'ramda'; const DEFAULT_TIMEOUT_DELAY = 2000; @@ -37,3 +38,5 @@ export const fixLeafletIcons = () => { shadowUrl: markerShadow, }); }; + +export const rangeOf = (size, mappingFn, startAt = 1) => range(startAt, size + 1).map(mappingFn); diff --git a/test/tags/TagsList.test.js b/test/tags/TagsList.test.js index 0098eee9..df914f7b 100644 --- a/test/tags/TagsList.test.js +++ b/test/tags/TagsList.test.js @@ -1,10 +1,11 @@ import React from 'react'; import { shallow } from 'enzyme'; -import { identity, range } from 'ramda'; +import { identity } from 'ramda'; import * as sinon from 'sinon'; import createTagsList from '../../src/tags/TagsList'; import MuttedMessage from '../../src/utils/MuttedMessage'; import SearchField from '../../src/utils/SearchField'; +import { rangeOf } from '../../src/utils/utils'; describe('', () => { let wrapper; @@ -53,7 +54,7 @@ describe('', () => { it('renders the proper amount of groups and cards based on the amount of tags', () => { const amountOfTags = 10; const amountOfGroups = 4; - const wrapper = createWrapper({ filteredTags: range(0, amountOfTags).map((i) => `tag_${i}`) }); + const wrapper = createWrapper({ filteredTags: rangeOf(amountOfTags, (i) => `tag_${i}`) }); const cards = wrapper.find(TagCard); const groups = wrapper.find('.col-md-6'); diff --git a/test/utils/utils.test.js b/test/utils/utils.test.js index 94a40578..e4c47a90 100644 --- a/test/utils/utils.test.js +++ b/test/utils/utils.test.js @@ -3,7 +3,12 @@ import L from 'leaflet'; import marker2x from 'leaflet/dist/images/marker-icon-2x.png'; import marker from 'leaflet/dist/images/marker-icon.png'; import markerShadow from 'leaflet/dist/images/marker-shadow.png'; -import { stateFlagTimeout as stateFlagTimeoutFactory, determineOrderDir, fixLeafletIcons } from '../../src/utils/utils'; +import { + stateFlagTimeout as stateFlagTimeoutFactory, + determineOrderDir, + fixLeafletIcons, + rangeOf, +} from '../../src/utils/utils'; describe('utils', () => { describe('stateFlagTimeout', () => { @@ -57,4 +62,29 @@ describe('utils', () => { expect(shadowUrl).toEqual(markerShadow); }); }); + + describe('rangeOf', () => { + const func = (i) => `result_${i}`; + const size = 5; + + it('builds a range of specified size invike provided function', () => { + expect(rangeOf(size, func)).toEqual([ + 'result_1', + 'result_2', + 'result_3', + 'result_4', + 'result_5', + ]); + }); + + it('builds a range starting at provided pos', () => { + const startAt = 3; + + expect(rangeOf(size, func, startAt)).toEqual([ + 'result_3', + 'result_4', + 'result_5', + ]); + }); + }); }); From c094a27c979b59e4298a9599c29322b310b246b8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 9 Mar 2019 13:20:43 +0100 Subject: [PATCH 02/10] Created PaginationDropdown component --- src/index.scss | 4 ++++ src/utils/PaginationDropdown.js | 33 +++++++++++++++++++++++++++++++++ src/utils/SortingDropdown.js | 2 +- src/utils/SortingDropdown.scss | 4 ---- 4 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 src/utils/PaginationDropdown.js diff --git a/src/index.scss b/src/index.scss index 1d07ba5d..1f4cda05 100644 --- a/src/index.scss +++ b/src/index.scss @@ -55,3 +55,7 @@ body, .pagination .page-link { cursor: pointer; } + +.paddingless { + padding: 0; +} diff --git a/src/utils/PaginationDropdown.js b/src/utils/PaginationDropdown.js new file mode 100644 index 00000000..0789e9bf --- /dev/null +++ b/src/utils/PaginationDropdown.js @@ -0,0 +1,33 @@ +import React from 'react'; +import { DropdownItem, DropdownMenu, DropdownToggle, UncontrolledDropdown } from 'reactstrap'; +import * as PropTypes from 'prop-types'; + +const propTypes = { + toggleClassName: PropTypes.string, + ranges: PropTypes.arrayOf(PropTypes.number).isRequired, + value: PropTypes.number.isRequired, + setValue: PropTypes.func.isRequired, +}; + +const PaginationDropdown = ({ toggleClassName, ranges, value, setValue }) => ( + + + Paginate + + + {ranges.map((itemsPerPage) => ( + setValue(itemsPerPage)}> + {itemsPerPage} items per page + + ))} + + setValue(Infinity)}> + Clear pagination + + + +); + +PaginationDropdown.propTypes = propTypes; + +export default PaginationDropdown; diff --git a/src/utils/SortingDropdown.js b/src/utils/SortingDropdown.js index 5cc5ecf1..e4018ddd 100644 --- a/src/utils/SortingDropdown.js +++ b/src/utils/SortingDropdown.js @@ -33,7 +33,7 @@ const SortingDropdown = ({ items, orderField, orderDir, onChange, isButton, righ Order by diff --git a/src/utils/SortingDropdown.scss b/src/utils/SortingDropdown.scss index 5f9c624e..3d2ae507 100644 --- a/src/utils/SortingDropdown.scss +++ b/src/utils/SortingDropdown.scss @@ -10,7 +10,3 @@ margin: 3.5px 0 0; float: right; } - -.sorting-dropdown__paddingless.sorting-dropdown__paddingless { - padding: 0; -} From 61480abd2ecbaddb5e7765d50efae8144cfdf13e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 10 Mar 2019 08:28:14 +0100 Subject: [PATCH 03/10] Updated charts to allow optional pagination --- .eslintrc | 1 + package.json | 4 +- src/utils/utils.js | 4 + src/visits/GraphCard.js | 60 +++------ src/visits/ShortUrlVisits.js | 7 +- src/visits/SortableBarGraph.js | 126 +++++++++++++++---- test/short-urls/helpers/ShortUrlsRow.test.js | 11 +- test/utils/utils.test.js | 18 +++ test/visits/SortableBarGraph.test.js | 19 ++- yarn.lock | 26 ++-- 10 files changed, 180 insertions(+), 96 deletions(-) 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" From 1ad4290487933671b517d357fbfc70a14d0b69a7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 10 Mar 2019 09:54:40 +0100 Subject: [PATCH 04/10] Applied some naming improvements --- src/visits/ShortUrlVisits.js | 2 +- src/visits/SortableBarGraph.js | 25 +++++++++++++------------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js index 0d880d9f..820956d3 100644 --- a/src/visits/ShortUrlVisits.js +++ b/src/visits/ShortUrlVisits.js @@ -94,7 +94,7 @@ const ShortUrlVisits = ({ processStatsFromVisits }) => class ShortUrlVisits exte
{title} @@ -107,14 +109,14 @@ export default class SortableBarGraph extends React.Component { onChange={(orderField, orderDir) => this.setState({ orderField, orderDir, currentPage: 1 })} />
- {supportPagination && ( + {withPagination && keys(stats).length > 50 && (
{ - this.redraw = true; + this.redrawChart = true; this.setState({ itemsPerPage, currentPage: 1 }); }} /> @@ -123,7 +125,6 @@ export default class SortableBarGraph extends React.Component { {extraHeaderContent &&
{extraHeaderContent}
} ); - const { currentPageStats, pagination, max } = this.determineStats(stats, sortingItems); return ( ); } From b6f6b1ae9dbaf510d3f05f3cae4482b8adac1fcd Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 10 Mar 2019 10:08:42 +0100 Subject: [PATCH 05/10] Enabled stickiness on footer --- src/visits/GraphCard.js | 3 ++- src/visits/GraphCard.scss | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 src/visits/GraphCard.scss diff --git a/src/visits/GraphCard.js b/src/visits/GraphCard.js index 44facee7..b8d8d94c 100644 --- a/src/visits/GraphCard.js +++ b/src/visits/GraphCard.js @@ -3,6 +3,7 @@ import { Doughnut, HorizontalBar } from 'react-chartjs-2'; import PropTypes from 'prop-types'; import React from 'react'; import { keys, values } from 'ramda'; +import './GraphCard.scss'; const propTypes = { title: PropTypes.oneOfType([ PropTypes.string, PropTypes.node ]), @@ -66,7 +67,7 @@ const GraphCard = ({ title, footer, isBarChart, stats, max, redraw = false }) => {title} {renderGraph(title, isBarChart, stats, max, redraw)} - {footer && {footer}} + {footer && {footer}} ); diff --git a/src/visits/GraphCard.scss b/src/visits/GraphCard.scss new file mode 100644 index 00000000..d429901c --- /dev/null +++ b/src/visits/GraphCard.scss @@ -0,0 +1,4 @@ +.graph-card__footer--sticky { + position: sticky; + bottom: 0; +} From 478ee59bb05959ecbd858ba544d63e999e8e33cb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 10 Mar 2019 10:56:36 +0100 Subject: [PATCH 06/10] Updated cities chart so that map shows locations from current page when result set is paginated --- src/visits/ShortUrlVisits.js | 12 +++++++++--- src/visits/SortableBarGraph.js | 13 +++++++++++-- src/visits/helpers/OpenMapModalBtn.js | 8 +++++--- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js index 820956d3..9c74356f 100644 --- a/src/visits/ShortUrlVisits.js +++ b/src/visits/ShortUrlVisits.js @@ -116,9 +116,15 @@ const ShortUrlVisits = ({ processStatsFromVisits }) => class ShortUrlVisits exte 0 && - )} + extraHeaderContent={(filterLocations) => + mapLocations.length > 0 && ( + + ) + } sortingItems={{ name: 'City name', amount: 'Visits amount', diff --git a/src/visits/SortableBarGraph.js b/src/visits/SortableBarGraph.js index 70b309e5..2d0d5e60 100644 --- a/src/visits/SortableBarGraph.js +++ b/src/visits/SortableBarGraph.js @@ -16,7 +16,7 @@ export default class SortableBarGraph extends React.Component { stats: PropTypes.object.isRequired, title: PropTypes.string.isRequired, sortingItems: PropTypes.object.isRequired, - extraHeaderContent: PropTypes.node, + extraHeaderContent: PropTypes.func, withPagination: PropTypes.bool, }; @@ -96,6 +96,11 @@ export default class SortableBarGraph extends React.Component { render() { const { stats, sortingItems, title, extraHeaderContent, withPagination = true } = this.props; const { currentPageStats, pagination, max } = this.determineStats(stats, sortingItems); + const filterLocations = (locations) => { + const validCities = keys(currentPageStats); + + return locations.filter(({ cityName }) => validCities.includes(cityName)); + }; const computedTitle = ( {title} @@ -122,7 +127,11 @@ export default class SortableBarGraph extends React.Component { />
)} - {extraHeaderContent &&
{extraHeaderContent}
} + {extraHeaderContent && ( +
+ {extraHeaderContent(pagination ? filterLocations : undefined)} +
+ )} ); diff --git a/src/visits/helpers/OpenMapModalBtn.js b/src/visits/helpers/OpenMapModalBtn.js index afe00d1c..18a88f4f 100644 --- a/src/visits/helpers/OpenMapModalBtn.js +++ b/src/visits/helpers/OpenMapModalBtn.js @@ -10,22 +10,24 @@ export default class OpenMapModalBtn extends React.Component { static propTypes = { modalTitle: PropTypes.string.isRequired, locations: PropTypes.arrayOf(PropTypes.object), + filterLocations: PropTypes.func, }; state = { mapIsOpened: false }; render() { - const { modalTitle, locations = [] } = this.props; + const { modalTitle, locations = [], filterLocations } = this.props; const toggleMap = () => this.setState(({ mapIsOpened }) => ({ mapIsOpened: !mapIsOpened })); const buttonRef = React.createRef(); + const filteredLocations = filterLocations ? filterLocations(locations) : locations; return ( - buttonRef.current}>Show in map - + buttonRef.current}>Show in map + ); } From 5233f5a07b43043a4f1b81c4e05979f3aeb44131 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 10 Mar 2019 12:09:54 +0100 Subject: [PATCH 07/10] Updated OpenMapModalBtn so that it allows showing only active cities --- src/visits/ShortUrlVisits.js | 11 ++-- src/visits/SortableBarGraph.js | 8 +-- src/visits/helpers/MapModal.js | 4 ++ src/visits/helpers/OpenMapModalBtn.js | 77 ++++++++++++++++++--------- 4 files changed, 62 insertions(+), 38 deletions(-) diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js index 9c74356f..1de3c183 100644 --- a/src/visits/ShortUrlVisits.js +++ b/src/visits/ShortUrlVisits.js @@ -116,14 +116,9 @@ const ShortUrlVisits = ({ processStatsFromVisits }) => class ShortUrlVisits exte - mapLocations.length > 0 && ( - - ) + extraHeaderContent={(activeCities) => + mapLocations.length > 0 && + } sortingItems={{ name: 'City name', diff --git a/src/visits/SortableBarGraph.js b/src/visits/SortableBarGraph.js index 2d0d5e60..4096467a 100644 --- a/src/visits/SortableBarGraph.js +++ b/src/visits/SortableBarGraph.js @@ -96,11 +96,7 @@ export default class SortableBarGraph extends React.Component { render() { const { stats, sortingItems, title, extraHeaderContent, withPagination = true } = this.props; const { currentPageStats, pagination, max } = this.determineStats(stats, sortingItems); - const filterLocations = (locations) => { - const validCities = keys(currentPageStats); - - return locations.filter(({ cityName }) => validCities.includes(cityName)); - }; + const activeCities = keys(currentPageStats); const computedTitle = ( {title} @@ -129,7 +125,7 @@ export default class SortableBarGraph extends React.Component { )} {extraHeaderContent && (
- {extraHeaderContent(pagination ? filterLocations : undefined)} + {extraHeaderContent(pagination ? activeCities : undefined)}
)} diff --git a/src/visits/helpers/MapModal.js b/src/visits/helpers/MapModal.js index 6a616bca..45cc7304 100644 --- a/src/visits/helpers/MapModal.js +++ b/src/visits/helpers/MapModal.js @@ -27,6 +27,10 @@ const OpenStreetMapTile = () => ( ); const calculateMapProps = (locations) => { + if (locations.length === 0) { + return {}; + } + if (locations.length > 1) { return { bounds: locations.map(prop('latLong')) }; } diff --git a/src/visits/helpers/OpenMapModalBtn.js b/src/visits/helpers/OpenMapModalBtn.js index 18a88f4f..2289ee06 100644 --- a/src/visits/helpers/OpenMapModalBtn.js +++ b/src/visits/helpers/OpenMapModalBtn.js @@ -1,34 +1,63 @@ -import React from 'react'; +import React, { useState } from 'react'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { faMapMarkedAlt as mapIcon } from '@fortawesome/free-solid-svg-icons'; -import { UncontrolledTooltip } from 'reactstrap'; +import { Dropdown, DropdownItem, DropdownMenu, UncontrolledTooltip } from 'reactstrap'; import * as PropTypes from 'prop-types'; import MapModal from './MapModal'; import './OpenMapModalBtn.scss'; -export default class OpenMapModalBtn extends React.Component { - static propTypes = { - modalTitle: PropTypes.string.isRequired, - locations: PropTypes.arrayOf(PropTypes.object), - filterLocations: PropTypes.func, +const propTypes = { + modalTitle: PropTypes.string.isRequired, + locations: PropTypes.arrayOf(PropTypes.object), + activeCities: PropTypes.arrayOf(PropTypes.string), +}; + +const OpenMapModalBtn = ({ modalTitle, locations = [], activeCities }) => { + const [ mapIsOpened, setMapIsOpened ] = useState(false); + const [ dropdownIsOpened, setDropdownIsOpened ] = useState(false); + const [ locationsToShow, setLocationsToShow ] = useState([]); + + const buttonRef = React.createRef(); + const filterLocations = (locations) => locations.filter(({ cityName }) => activeCities.includes(cityName)); + const toggleMap = () => setMapIsOpened(!mapIsOpened); + const onClick = () => { + if (mapIsOpened) { + setMapIsOpened(false); + + return; + } + + if (!activeCities) { + setLocationsToShow(locations); + setMapIsOpened(true); + + return; + } + + setDropdownIsOpened(true); + }; + const openMapWithLocations = (filtered) => () => { + setLocationsToShow(filtered ? filterLocations(locations) : locations); + setMapIsOpened(true); }; - state = { mapIsOpened: false }; + return ( + + + buttonRef.current}>Show in map + setDropdownIsOpened(!dropdownIsOpened)} inNavbar> + + Show all locations + Show locations in current page + + + + + ); +}; - render() { - const { modalTitle, locations = [], filterLocations } = this.props; - const toggleMap = () => this.setState(({ mapIsOpened }) => ({ mapIsOpened: !mapIsOpened })); - const buttonRef = React.createRef(); - const filteredLocations = filterLocations ? filterLocations(locations) : locations; +OpenMapModalBtn.propTypes = propTypes; - return ( - - - buttonRef.current}>Show in map - - - ); - } -} +export default OpenMapModalBtn; From 87dc24e8a229f6d68784faca22eb846246e0f130 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 10 Mar 2019 13:05:20 +0100 Subject: [PATCH 08/10] Fixed and improved OpenMapModalBtn and ShortUrlVisit components tests --- src/visits/ShortUrlVisits.js | 6 +- src/visits/helpers/OpenMapModalBtn.js | 77 ++++++++--------- src/visits/services/provideServices.js | 6 +- test/visits/ShortUrlVisits.test.js | 2 +- test/visits/helpers/OpenMapModalBtn.test.js | 93 ++++++++++++++++----- 5 files changed, 121 insertions(+), 63 deletions(-) diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js index 1de3c183..c5b4ef99 100644 --- a/src/visits/ShortUrlVisits.js +++ b/src/visits/ShortUrlVisits.js @@ -12,9 +12,11 @@ import VisitsHeader from './VisitsHeader'; import GraphCard from './GraphCard'; import { shortUrlDetailType } from './reducers/shortUrlDetail'; import './ShortUrlVisits.scss'; -import OpenMapModalBtn from './helpers/OpenMapModalBtn'; -const ShortUrlVisits = ({ processStatsFromVisits }) => class ShortUrlVisits extends React.PureComponent { +const ShortUrlVisits = ( + { processStatsFromVisits }, + OpenMapModalBtn +) => class ShortUrlVisits extends React.PureComponent { static propTypes = { match: PropTypes.shape({ params: PropTypes.object, diff --git a/src/visits/helpers/OpenMapModalBtn.js b/src/visits/helpers/OpenMapModalBtn.js index 2289ee06..5d3ec58b 100644 --- a/src/visits/helpers/OpenMapModalBtn.js +++ b/src/visits/helpers/OpenMapModalBtn.js @@ -3,7 +3,6 @@ import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { faMapMarkedAlt as mapIcon } from '@fortawesome/free-solid-svg-icons'; import { Dropdown, DropdownItem, DropdownMenu, UncontrolledTooltip } from 'reactstrap'; import * as PropTypes from 'prop-types'; -import MapModal from './MapModal'; import './OpenMapModalBtn.scss'; const propTypes = { @@ -12,52 +11,50 @@ const propTypes = { activeCities: PropTypes.arrayOf(PropTypes.string), }; -const OpenMapModalBtn = ({ modalTitle, locations = [], activeCities }) => { - const [ mapIsOpened, setMapIsOpened ] = useState(false); - const [ dropdownIsOpened, setDropdownIsOpened ] = useState(false); - const [ locationsToShow, setLocationsToShow ] = useState([]); +const OpenMapModalBtn = (MapModal) => { + const OpenMapModalBtn = ({ modalTitle, locations = [], activeCities }) => { + const [ mapIsOpened, setMapIsOpened ] = useState(false); + const [ dropdownIsOpened, setDropdownIsOpened ] = useState(false); + const [ locationsToShow, setLocationsToShow ] = useState([]); - const buttonRef = React.createRef(); - const filterLocations = (locations) => locations.filter(({ cityName }) => activeCities.includes(cityName)); - const toggleMap = () => setMapIsOpened(!mapIsOpened); - const onClick = () => { - if (mapIsOpened) { - setMapIsOpened(false); + const buttonRef = React.createRef(); + const filterLocations = (locations) => locations.filter(({ cityName }) => activeCities.includes(cityName)); + const toggleMap = () => setMapIsOpened(!mapIsOpened); + const onClick = () => { + if (!activeCities) { + setLocationsToShow(locations); + setMapIsOpened(true); - return; - } + return; + } - if (!activeCities) { - setLocationsToShow(locations); + setDropdownIsOpened(true); + }; + const openMapWithLocations = (filtered) => () => { + setLocationsToShow(filtered ? filterLocations(locations) : locations); setMapIsOpened(true); + }; - return; - } - - setDropdownIsOpened(true); - }; - const openMapWithLocations = (filtered) => () => { - setLocationsToShow(filtered ? filterLocations(locations) : locations); - setMapIsOpened(true); + return ( + + + buttonRef.current}>Show in map + setDropdownIsOpened(!dropdownIsOpened)} inNavbar> + + Show all locations + Show locations in current page + + + + + ); }; - return ( - - - buttonRef.current}>Show in map - setDropdownIsOpened(!dropdownIsOpened)} inNavbar> - - Show all locations - Show locations in current page - - - - - ); + OpenMapModalBtn.propTypes = propTypes; + + return OpenMapModalBtn; }; -OpenMapModalBtn.propTypes = propTypes; - export default OpenMapModalBtn; diff --git a/src/visits/services/provideServices.js b/src/visits/services/provideServices.js index 8dcc498a..6258adda 100644 --- a/src/visits/services/provideServices.js +++ b/src/visits/services/provideServices.js @@ -1,11 +1,15 @@ import ShortUrlVisits from '../ShortUrlVisits'; import { cancelGetShortUrlVisits, getShortUrlVisits } from '../reducers/shortUrlVisits'; import { getShortUrlDetail } from '../reducers/shortUrlDetail'; +import OpenMapModalBtn from '../helpers/OpenMapModalBtn'; +import MapModal from '../helpers/MapModal'; import * as visitsParser from './VisitsParser'; const provideServices = (bottle, connect) => { // Components - bottle.serviceFactory('ShortUrlVisits', ShortUrlVisits, 'VisitsParser'); + bottle.serviceFactory('OpenMapModalBtn', OpenMapModalBtn, 'MapModal'); + bottle.serviceFactory('MapModal', () => MapModal); + bottle.serviceFactory('ShortUrlVisits', ShortUrlVisits, 'VisitsParser', 'OpenMapModalBtn'); bottle.decorator('ShortUrlVisits', connect( [ 'shortUrlVisits', 'shortUrlDetail' ], [ 'getShortUrlVisits', 'getShortUrlDetail', 'cancelGetShortUrlVisits' ] diff --git a/test/visits/ShortUrlVisits.test.js b/test/visits/ShortUrlVisits.test.js index d485a984..29e1e932 100644 --- a/test/visits/ShortUrlVisits.test.js +++ b/test/visits/ShortUrlVisits.test.js @@ -104,6 +104,6 @@ describe('', () => { const extraHeaderContent = citiesGraph.prop('extraHeaderContent'); expect(extraHeaderContent).toHaveLength(1); - expect(typeof extraHeaderContent[0]).toEqual('function'); + expect(typeof extraHeaderContent).toEqual('function'); }); }); diff --git a/test/visits/helpers/OpenMapModalBtn.test.js b/test/visits/helpers/OpenMapModalBtn.test.js index 3548992b..895d9310 100644 --- a/test/visits/helpers/OpenMapModalBtn.test.js +++ b/test/visits/helpers/OpenMapModalBtn.test.js @@ -1,42 +1,97 @@ import React from 'react'; -import { shallow } from 'enzyme'; -import { UncontrolledTooltip } from 'reactstrap'; -import OpenMapModalBtn from '../../../src/visits/helpers/OpenMapModalBtn'; -import MapModal from '../../../src/visits/helpers/MapModal'; +import { mount } from 'enzyme'; +import { Dropdown, DropdownItem, UncontrolledTooltip } from 'reactstrap'; +import createOpenMapModalBtn from '../../../src/visits/helpers/OpenMapModalBtn'; describe('', () => { let wrapper; const title = 'Foo'; - const locations = []; + const locations = [ + { + cityName: 'foo', + count: 30, + }, + { + cityName: 'bar', + count: 45, + }, + ]; + const MapModal = () => ''; + const OpenMapModalBtn = createOpenMapModalBtn(MapModal); + const createWrapper = (activeCities) => { + wrapper = mount(); - beforeEach(() => { - wrapper = shallow(); - }); + return wrapper; + }; - afterEach(() => wrapper.unmount()); + afterEach(() => wrapper && wrapper.unmount()); - it('Renders expected content', () => { + it('renders expected content', () => { + const wrapper = createWrapper(); const button = wrapper.find('.open-map-modal-btn__btn'); const tooltip = wrapper.find(UncontrolledTooltip); + const dropdown = wrapper.find(Dropdown); const modal = wrapper.find(MapModal); expect(button).toHaveLength(1); expect(tooltip).toHaveLength(1); + expect(dropdown).toHaveLength(1); expect(modal).toHaveLength(1); }); - it('changes modal visibility when toggled', () => { - const modal = wrapper.find(MapModal); + it('sets provided props to the map', (done) => { + const wrapper = createWrapper(); + const button = wrapper.find('.open-map-modal-btn__btn'); - expect(wrapper.state('mapIsOpened')).toEqual(false); - modal.prop('toggle')(); - expect(wrapper.state('mapIsOpened')).toEqual(true); + button.simulate('click'); + setImmediate(() => { + const modal = wrapper.find(MapModal); + + expect(modal.prop('title')).toEqual(title); + expect(modal.prop('locations')).toEqual(locations); + expect(modal.prop('isOpen')).toEqual(true); + done(); + }); }); - it('sets provided props to the map', () => { - const modal = wrapper.find(MapModal); + it('opens dropdown instead of modal when a list of active cities has been provided', (done) => { + const wrapper = createWrapper([ 'bar' ]); + const button = wrapper.find('.open-map-modal-btn__btn'); - expect(modal.prop('title')).toEqual(title); - expect(modal.prop('locations')).toEqual(locations); + button.simulate('click'); + + setImmediate(() => { + const dropdown = wrapper.find(Dropdown); + const modal = wrapper.find(MapModal); + + expect(dropdown.prop('isOpen')).toEqual(true); + expect(modal.prop('isOpen')).toEqual(false); + done(); + }); + }); + + it('filters out non-active cities from list of locations', (done) => { + const wrapper = createWrapper([ 'bar' ]); + const button = wrapper.find('.open-map-modal-btn__btn'); + + button.simulate('click'); + setImmediate(() => { + const dropdown = wrapper.find(Dropdown); + const item = dropdown.find(DropdownItem).at(1); + + item.simulate('click'); + setImmediate(() => { + const modal = wrapper.find(MapModal); + + expect(modal.prop('title')).toEqual(title); + expect(modal.prop('locations')).toEqual([ + { + cityName: 'bar', + count: 45, + }, + ]); + done(); + }); + }); }); }); From e0db6d5a578c14b07e3ffc1fcb917500e11ea5c8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 10 Mar 2019 17:55:02 +0100 Subject: [PATCH 09/10] Improved SortableBarGraph test --- src/visits/GraphCard.js | 4 +- src/visits/SortableBarGraph.js | 4 +- test/visits/SortableBarGraph.test.js | 74 +++++++++++++++++++++++----- 3 files changed, 65 insertions(+), 17 deletions(-) diff --git a/src/visits/GraphCard.js b/src/visits/GraphCard.js index b8d8d94c..0b2c5c0b 100644 --- a/src/visits/GraphCard.js +++ b/src/visits/GraphCard.js @@ -6,7 +6,7 @@ import { keys, values } from 'ramda'; import './GraphCard.scss'; const propTypes = { - title: PropTypes.oneOfType([ PropTypes.string, PropTypes.node ]), + title: PropTypes.oneOfType([ PropTypes.string, PropTypes.func ]), footer: PropTypes.oneOfType([ PropTypes.string, PropTypes.node ]), isBarChart: PropTypes.bool, stats: PropTypes.object, @@ -65,7 +65,7 @@ const renderGraph = (title, isBarChart, stats, max, redraw) => { const GraphCard = ({ title, footer, isBarChart, stats, max, redraw = false }) => ( - {title} + {typeof title === 'function' ? title() : title} {renderGraph(title, isBarChart, stats, max, redraw)} {footer && {footer}} diff --git a/src/visits/SortableBarGraph.js b/src/visits/SortableBarGraph.js index 4096467a..2d3c6146 100644 --- a/src/visits/SortableBarGraph.js +++ b/src/visits/SortableBarGraph.js @@ -97,7 +97,7 @@ export default class SortableBarGraph extends React.Component { const { stats, sortingItems, title, extraHeaderContent, withPagination = true } = this.props; const { currentPageStats, pagination, max } = this.determineStats(stats, sortingItems); const activeCities = keys(currentPageStats); - const computedTitle = ( + const computeTitle = () => ( {title}
@@ -134,7 +134,7 @@ export default class SortableBarGraph extends React.Component { return ( ', () => { let wrapper; @@ -15,9 +17,14 @@ describe('', () => { Foo: 100, Bar: 50, }; - const createWrapper = (extraHeaderContent) => { + const createWrapper = (withPagination = false, extraStats = {}) => { wrapper = shallow( - + ); return wrapper; @@ -37,7 +44,7 @@ describe('', () => { beforeEach(() => { const wrapper = createWrapper(); - const dropdown = wrapper.find(SortingDropdown); + const dropdown = wrapper.renderProp('title')().find(SortingDropdown); assert = (sortName, sortDir, expectedKeys, expectedValues, done) => { dropdown.prop('onChange')(sortName, sortDir); @@ -59,15 +66,56 @@ describe('', () => { it('value - DESC', (done) => assert('value', 'DESC', [ 'Foo', 'Bar' ], [ 100, 50 ], done)); }); - it('renders extra header functions', () => { - const wrapper = createWrapper(( - - Foo - Bar - - )); + describe('renders properly paginated stats when pagination is set', () => { + let assert; - expect(wrapper.find('.foo-span')).toHaveLength(1); - expect(wrapper.find('.bar-span')).toHaveLength(1); + beforeEach(() => { + const wrapper = createWrapper(true, range(1, 159).reduce((accum, value) => { + accum[`key_${value}`] = value; + + return accum; + }, {})); + const dropdown = wrapper.renderProp('title')().find(PaginationDropdown); + + assert = (itemsPerPage, expectedStats, done) => { + dropdown.prop('setValue')(itemsPerPage); + setImmediate(() => { + const graphCard = wrapper.find(GraphCard); + const statsKeys = keys(graphCard.prop('stats')); + + expect(statsKeys).toEqual(expectedStats); + done(); + }); + }; + }); + + const buildExpected = (size) => [ 'Foo', 'Bar', ...rangeOf(size - 2, (i) => `key_${i}`) ]; + + it('50 items per page', (done) => assert(50, buildExpected(50), done)); + it('100 items per page', (done) => assert(100, buildExpected(100), done)); + it('200 items per page', (done) => assert(200, buildExpected(160), done)); + it('500 items per page', (done) => assert(500, buildExpected(160), done)); + }); + + it('renders extra header content', () => { + wrapper = shallow( + + ( + + Foo + Bar + + )} + /> + + ).find(SortableBarGraph); + const header = wrapper.renderProp('extraHeaderContent')(); + + expect(header.find('.foo-span')).toHaveLength(1); + expect(header.find('.bar-span')).toHaveLength(1); }); }); From 391424d8a18357b5643d5f0dd552f1f9e51948ba Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 16 Mar 2019 09:02:10 +0100 Subject: [PATCH 10/10] Ensured bar charts are regenerated when their height changes --- CHANGELOG.md | 3 ++- src/visits/GraphCard.js | 16 ++++++++-------- src/visits/SortableBarGraph.js | 25 ++----------------------- test/visits/GraphCard.test.js | 2 +- 4 files changed, 13 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 284f23e7..bd7161c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org). -## [Unreleased] +## 2.0.3 - 2019-03-16 #### Added @@ -26,6 +26,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * [#120](https://github.com/shlinkio/shlink-web-client/issues/120) Fixed crash when visits page is loaded and there are no visits with known cities. * [#113](https://github.com/shlinkio/shlink-web-client/issues/113) Ensured visits loading is cancelled when the visits page is unmounted. Requests on flight will still finish. +* [#118](https://github.com/shlinkio/shlink-web-client/issues/118) Fixed chart crashing when trying to render lots of bars by adding pagination. ## 2.0.2 - 2019-03-04 diff --git a/src/visits/GraphCard.js b/src/visits/GraphCard.js index 0b2c5c0b..253f4011 100644 --- a/src/visits/GraphCard.js +++ b/src/visits/GraphCard.js @@ -11,7 +11,6 @@ const propTypes = { isBarChart: PropTypes.bool, stats: PropTypes.object, max: PropTypes.number, - redraw: PropTypes.bool, }; const generateGraphData = (title, isBarChart, labels, data) => ({ @@ -37,19 +36,19 @@ const generateGraphData = (title, isBarChart, labels, data) => ({ const dropLabelIfHidden = (label) => label.startsWith('hidden') ? '' : label; -const renderGraph = (title, isBarChart, stats, max, redraw) => { +const renderGraph = (title, isBarChart, stats, max) => { const Component = isBarChart ? HorizontalBar : Doughnut; const labels = keys(stats).map(dropLabelIfHidden); const data = values(stats); const options = { legend: isBarChart ? { display: false } : { position: 'right' }, - scales: isBarChart ? { + scales: isBarChart && { xAxes: [ { ticks: { beginAtZero: true, max }, }, ], - } : null, + }, tooltips: { intersect: !isBarChart, @@ -58,15 +57,16 @@ const renderGraph = (title, isBarChart, stats, max, redraw) => { }, }; const graphData = generateGraphData(title, isBarChart, labels, data); - const height = labels.length < 20 ? null : labels.length * 8; + const height = isBarChart && labels.length > 20 ? labels.length * 8 : null; - return ; + // Provide a key based on the height, so that every time the dataset changes, a new graph is rendered + return ; }; -const GraphCard = ({ title, footer, isBarChart, stats, max, redraw = false }) => ( +const GraphCard = ({ title, footer, isBarChart, stats, max }) => ( {typeof title === 'function' ? title() : title} - {renderGraph(title, isBarChart, stats, max, redraw)} + {renderGraph(title, isBarChart, stats, max)} {footer && {footer}} ); diff --git a/src/visits/SortableBarGraph.js b/src/visits/SortableBarGraph.js index 2d3c6146..c51245f0 100644 --- a/src/visits/SortableBarGraph.js +++ b/src/visits/SortableBarGraph.js @@ -26,15 +26,6 @@ export default class SortableBarGraph extends React.Component { currentPage: 1, itemsPerPage: Infinity, }; - redrawChart = false; - - doRedrawChart() { - const prev = this.redrawChart; - - this.redrawChart = false; - - return prev; - } determineStats(stats, sortingItems) { const pairs = toPairs(stats); @@ -116,10 +107,7 @@ export default class SortableBarGraph extends React.Component { toggleClassName="btn-sm paddingless mr-3" ranges={[ 50, 100, 200, 500 ]} value={this.state.itemsPerPage} - setValue={(itemsPerPage) => { - this.redrawChart = true; - this.setState({ itemsPerPage, currentPage: 1 }); - }} + setValue={(itemsPerPage) => this.setState({ itemsPerPage, currentPage: 1 })} />
)} @@ -131,15 +119,6 @@ export default class SortableBarGraph extends React.Component { ); - return ( - - ); + return ; } } diff --git a/test/visits/GraphCard.test.js b/test/visits/GraphCard.test.js index d6820008..2b7d2645 100644 --- a/test/visits/GraphCard.test.js +++ b/test/visits/GraphCard.test.js @@ -39,7 +39,7 @@ describe('', () => { ]); expect(borderColor).toEqual('white'); expect(legend).toEqual({ position: 'right' }); - expect(scales).toBeNull(); + expect(scales).toBeUndefined(); }); it('renders HorizontalBar when is not a bar chart', () => {