From 8f42e65ccd91cd48b4c59edea806250f0f66d2eb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 10 Apr 2020 11:59:53 +0200 Subject: [PATCH 1/9] Allowed visits to be selected on charts so that they get highlighted on the rest of the charts --- src/visits/GraphCard.js | 34 ++++++++++--- src/visits/ShortUrlVisits.js | 54 ++++++++++---------- src/visits/SortableBarGraph.js | 5 +- src/visits/VisitsTable.js | 38 +++++++------- src/visits/services/VisitsParser.js | 15 ++++-- test/visits/ShortUrlVisits.test.js | 2 +- test/visits/VisitsTable.test.js | 60 +++++++++++++---------- test/visits/services/VisitsParser.test.js | 51 ++++++++++++++++++- 8 files changed, 169 insertions(+), 90 deletions(-) diff --git a/src/visits/GraphCard.js b/src/visits/GraphCard.js index 5814739e..695be8fb 100644 --- a/src/visits/GraphCard.js +++ b/src/visits/GraphCard.js @@ -12,6 +12,7 @@ const propTypes = { stats: PropTypes.object, max: PropTypes.number, highlightedStats: PropTypes.object, + onClick: PropTypes.func, }; const generateGraphData = (title, isBarChart, labels, data, highlightedData) => ({ @@ -19,6 +20,7 @@ const generateGraphData = (title, isBarChart, labels, data, highlightedData) => datasets: [ { title, + label: '', data, backgroundColor: isBarChart ? 'rgba(70, 150, 229, 0.4)' : [ '#97BBCD', @@ -45,17 +47,20 @@ const generateGraphData = (title, isBarChart, labels, data, highlightedData) => const dropLabelIfHidden = (label) => label.startsWith('hidden') ? '' : label; -const renderGraph = (title, isBarChart, stats, max, highlightedStats) => { +const renderGraph = (title, isBarChart, stats, max, highlightedStats, onClick) => { + const hasHighlightedStats = highlightedStats && Object.keys(highlightedStats).length > 0; const Component = isBarChart ? HorizontalBar : Doughnut; const labels = keys(stats).map(dropLabelIfHidden); - const data = values(!highlightedStats ? stats : keys(highlightedStats).reduce((acc, highlightedKey) => { + const data = values(!hasHighlightedStats ? stats : keys(highlightedStats).reduce((acc, highlightedKey) => { if (acc[highlightedKey]) { acc[highlightedKey] -= highlightedStats[highlightedKey]; } return acc; }, { ...stats })); - const highlightedData = highlightedStats && values({ ...zipObj(labels, labels.map(() => 0)), ...highlightedStats }); + const highlightedData = hasHighlightedStats && values( + { ...zipObj(labels, labels.map(() => 0)), ...highlightedStats } + ); const options = { legend: isBarChart ? { display: false } : { position: 'right' }, @@ -79,13 +84,30 @@ const renderGraph = (title, isBarChart, stats, max, highlightedStats) => { const height = isBarChart && labels.length > 20 ? labels.length * 8 : null; // Provide a key based on the height, so that every time the dataset changes, a new graph is rendered - return ; + return ( + { + if (!onClick || !chart) { + return; + } + + const { _index, _chart: { data } } = chart; + const { labels } = data; + + onClick(labels[_index]); + }} + /> + ); }; -const GraphCard = ({ title, footer, isBarChart, stats, max, highlightedStats }) => ( +const GraphCard = ({ title, footer, isBarChart, stats, max, highlightedStats, onClick }) => ( {typeof title === 'function' ? title() : title} - {renderGraph(title, isBarChart, stats, max, highlightedStats)} + {renderGraph(title, isBarChart, stats, max, highlightedStats, onClick)} {footer && {footer}} ); diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js index fd6026e2..5c233bb9 100644 --- a/src/visits/ShortUrlVisits.js +++ b/src/visits/ShortUrlVisits.js @@ -1,5 +1,5 @@ -import { isEmpty, values } from 'ramda'; -import React, { useState, useEffect } from 'react'; +import { isEmpty, propEq, values } from 'ramda'; +import React, { useState, useEffect, useMemo } from 'react'; import { Button, Card, Collapse } from 'reactstrap'; import PropTypes from 'prop-types'; import qs from 'qs'; @@ -41,10 +41,8 @@ const highlightedVisitsToStats = (highlightedVisits, prop) => highlightedVisits. return acc; }, {}); const format = formatDate(); -let memoizationId; -let timeWhenMounted; -const ShortUrlVisits = ({ processStatsFromVisits }, OpenMapModalBtn) => { +const ShortUrlVisits = ({ processStatsFromVisits, normalizeVisits }, OpenMapModalBtn) => { const ShortUrlVisitsComp = ({ match, location, @@ -62,23 +60,28 @@ const ShortUrlVisits = ({ processStatsFromVisits }, OpenMapModalBtn) => { const [ highlightedVisits, setHighlightedVisits ] = useState([]); const [ isMobileDevice, setIsMobileDevice ] = useState(false); const determineIsMobileDevice = () => setIsMobileDevice(matchMedia('(max-width: 991px)').matches); + const highlightVisitsForProp = (prop) => (value) => setHighlightedVisits( + highlightedVisits.length === 0 ? normalizedVisits.filter(propEq(prop, value)) : [] + ); const { params } = match; const { shortCode } = params; const { search } = location; const { domain } = qs.parse(search, { ignoreQueryPrefix: true }); - const loadVisits = () => { - const start = format(startDate); - const end = format(endDate); + const { visits, loading, loadingLarge, error } = shortUrlVisits; + const showTableControls = !loading && visits.length > 0; + const normalizedVisits = useMemo(() => normalizeVisits(visits), [ visits ]); + const { os, browsers, referrers, countries, cities, citiesForMap } = useMemo( + () => processStatsFromVisits(visits), + [ visits ] + ); + const mapLocations = values(citiesForMap); - // While the "page" is loaded, use the timestamp + filtering dates as memoization IDs for stats calculations - memoizationId = `${timeWhenMounted}_${shortCode}_${start}_${end}`; - getShortUrlVisits(shortCode, { startDate: start, endDate: end, domain }); - }; + const loadVisits = () => + getShortUrlVisits(shortCode, { startDate: format(startDate), endDate: format(endDate), domain }); useEffect(() => { - timeWhenMounted = new Date().getTime(); getShortUrlDetail(shortCode, domain); determineIsMobileDevice(); window.addEventListener('resize', determineIsMobileDevice); @@ -92,9 +95,6 @@ const ShortUrlVisits = ({ processStatsFromVisits }, OpenMapModalBtn) => { loadVisits(); }, [ startDate, endDate ]); - const { visits, loading, loadingLarge, error } = shortUrlVisits; - const showTableControls = !loading && visits.length > 0; - const renderVisitsContent = () => { if (loading) { const message = loadingLarge ? 'This is going to take a while... :S' : 'Loading...'; @@ -114,11 +114,6 @@ const ShortUrlVisits = ({ processStatsFromVisits }, OpenMapModalBtn) => { return There are no visits matching current filter :(; } - const { os, browsers, referrers, countries, cities, citiesForMap } = processStatsFromVisits( - { id: memoizationId, visits } - ); - const mapLocations = values(citiesForMap); - return (
@@ -137,6 +132,7 @@ const ShortUrlVisits = ({ processStatsFromVisits }, OpenMapModalBtn) => { name: 'Referrer name', amount: 'Visits amount', }} + onClick={highlightVisitsForProp('referer')} />
@@ -148,6 +144,7 @@ const ShortUrlVisits = ({ processStatsFromVisits }, OpenMapModalBtn) => { name: 'Country name', amount: 'Visits amount', }} + onClick={highlightVisitsForProp('country')} />
@@ -163,6 +160,7 @@ const ShortUrlVisits = ({ processStatsFromVisits }, OpenMapModalBtn) => { name: 'City name', amount: 'Visits amount', }} + onClick={highlightVisitsForProp('city')} />
@@ -185,11 +183,7 @@ const ShortUrlVisits = ({ processStatsFromVisits }, OpenMapModalBtn) => {
{showTableControls && ( - @@ -201,12 +195,16 @@ const ShortUrlVisits = ({ processStatsFromVisits }, OpenMapModalBtn) => { {showTableControls && ( - + )} diff --git a/src/visits/SortableBarGraph.js b/src/visits/SortableBarGraph.js index 54a1bd9b..aecca4e7 100644 --- a/src/visits/SortableBarGraph.js +++ b/src/visits/SortableBarGraph.js @@ -20,6 +20,7 @@ export default class SortableBarGraph extends React.Component { sortingItems: PropTypes.object.isRequired, extraHeaderContent: PropTypes.func, withPagination: PropTypes.bool, + onClick: PropTypes.func, }; state = { @@ -74,7 +75,7 @@ export default class SortableBarGraph extends React.Component { } render() { - const { stats, sortingItems, title, extraHeaderContent, highlightedStats, withPagination = true } = this.props; + const { stats, sortingItems, title, extraHeaderContent, withPagination = true, ...rest } = this.props; const { currentPageStats, pagination, max } = this.determineStats(stats, sortingItems); const activeCities = keys(currentPageStats); const computeTitle = () => ( @@ -115,7 +116,7 @@ export default class SortableBarGraph extends React.Component { stats={currentPageStats} footer={pagination} max={max} - highlightedStats={highlightedStats} + {...rest} /> ); } diff --git a/src/visits/VisitsTable.js b/src/visits/VisitsTable.js index bd4dcaf1..6c5e082f 100644 --- a/src/visits/VisitsTable.js +++ b/src/visits/VisitsTable.js @@ -2,7 +2,7 @@ import React, { useEffect, useMemo, useState } from 'react'; import PropTypes from 'prop-types'; import Moment from 'react-moment'; import classNames from 'classnames'; -import { map, min, splitEvery } from 'ramda'; +import { min, splitEvery } from 'ramda'; import { faCaretDown as caretDownIcon, faCaretUp as caretUpIcon, @@ -11,15 +11,18 @@ import { import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import SimplePaginator from '../common/SimplePaginator'; import SearchField from '../utils/SearchField'; -import { browserFromUserAgent, extractDomain, osFromUserAgent } from '../utils/helpers/visits'; import { determineOrderDir } from '../utils/utils'; import { prettify } from '../utils/helpers/numbers'; -import { visitType } from './reducers/shortUrlVisits'; import './VisitsTable.scss'; +const NormalizedVisitType = PropTypes.shape({ + +}); + const propTypes = { - visits: PropTypes.arrayOf(visitType).isRequired, - onVisitsSelected: PropTypes.func, + visits: PropTypes.arrayOf(NormalizedVisitType).isRequired, + selectedVisits: PropTypes.arrayOf(NormalizedVisitType), + setSelectedVisits: PropTypes.func.isRequired, isSticky: PropTypes.bool, matchMedia: PropTypes.func, }; @@ -35,34 +38,30 @@ const sortVisits = ({ field, dir }, visits) => visits.sort((a, b) => { return a[field] > b[field] ? greaterThan : smallerThan; }); const calculateVisits = (allVisits, searchTerm, order) => { - const filteredVisits = searchTerm ? searchVisits(searchTerm, allVisits) : allVisits; + const filteredVisits = searchTerm ? searchVisits(searchTerm, allVisits) : [ ...allVisits ]; const sortedVisits = order.dir ? sortVisits(order, filteredVisits) : filteredVisits; const total = sortedVisits.length; const visitsGroups = splitEvery(PAGE_SIZE, sortedVisits); return { visitsGroups, total }; }; -const normalizeVisits = map(({ userAgent, date, referer, visitLocation }) => ({ - date, - browser: browserFromUserAgent(userAgent), - os: osFromUserAgent(userAgent), - referer: extractDomain(referer), - country: (visitLocation && visitLocation.countryName) || 'Unknown', - city: (visitLocation && visitLocation.cityName) || 'Unknown', -})); -const VisitsTable = ({ visits, onVisitsSelected, isSticky = false, matchMedia = window.matchMedia }) => { - const allVisits = normalizeVisits(visits); +const VisitsTable = ({ + visits, + selectedVisits = [], + setSelectedVisits, + isSticky = false, + matchMedia = window.matchMedia, +}) => { const headerCellsClass = classNames('visits-table__header-cell', { 'visits-table__sticky': isSticky, }); const matchMobile = () => matchMedia('(max-width: 767px)').matches; - const [ selectedVisits, setSelectedVisits ] = useState([]); const [ isMobileDevice, setIsMobileDevice ] = useState(matchMobile()); const [ searchTerm, setSearchTerm ] = useState(undefined); const [ order, setOrder ] = useState({ field: undefined, dir: undefined }); - const resultSet = useMemo(() => calculateVisits(allVisits, searchTerm, order), [ searchTerm, order ]); + const resultSet = useMemo(() => calculateVisits(visits, searchTerm, order), [ searchTerm, order ]); const [ page, setPage ] = useState(1); const end = page * PAGE_SIZE; @@ -76,9 +75,6 @@ const VisitsTable = ({ visits, onVisitsSelected, isSticky = false, matchMedia = /> ); - useEffect(() => { - onVisitsSelected && onVisitsSelected(selectedVisits); - }, [ selectedVisits ]); useEffect(() => { const listener = () => setIsMobileDevice(matchMobile()); diff --git a/src/visits/services/VisitsParser.js b/src/visits/services/VisitsParser.js index 32cdf29d..25db20d8 100644 --- a/src/visits/services/VisitsParser.js +++ b/src/visits/services/VisitsParser.js @@ -1,4 +1,4 @@ -import { isEmpty, isNil, memoizeWith, prop } from 'ramda'; +import { isEmpty, isNil, map } from 'ramda'; import { browserFromUserAgent, extractDomain, osFromUserAgent } from '../../utils/helpers/visits'; const visitLocationHasProperty = (visitLocation, propertyName) => @@ -51,7 +51,7 @@ const updateCitiesForMapForVisit = (citiesForMapStats, { visitLocation }) => { citiesForMapStats[cityName] = currentCity; }; -export const processStatsFromVisits = memoizeWith(prop('id'), ({ visits }) => +export const processStatsFromVisits = (visits) => visits.reduce( (stats, visit) => { // We mutate the original object because it has a big side effect when large data sets are processed @@ -65,4 +65,13 @@ export const processStatsFromVisits = memoizeWith(prop('id'), ({ visits }) => return stats; }, { os: {}, browsers: {}, referrers: {}, countries: {}, cities: {}, citiesForMap: {} } - )); + ); + +export const normalizeVisits = map(({ userAgent, date, referer, visitLocation }) => ({ + date, + browser: browserFromUserAgent(userAgent), + os: osFromUserAgent(userAgent), + referer: extractDomain(referer), + country: (visitLocation && visitLocation.countryName) || 'Unknown', + city: (visitLocation && visitLocation.cityName) || 'Unknown', +})); diff --git a/test/visits/ShortUrlVisits.test.js b/test/visits/ShortUrlVisits.test.js index 1adae436..ad8743a1 100644 --- a/test/visits/ShortUrlVisits.test.js +++ b/test/visits/ShortUrlVisits.test.js @@ -20,7 +20,7 @@ describe('', () => { const location = { search: '' }; const createComponent = (shortUrlVisits) => { - const ShortUrlVisits = createShortUrlVisits({ processStatsFromVisits }, () => ''); + const ShortUrlVisits = createShortUrlVisits({ processStatsFromVisits, normalizeVisits: identity }, () => ''); wrapper = shallow( ', () => { const matchMedia = () => ({ matches: false }); + const setSelectedVisits = jest.fn(); let wrapper; - const createWrapper = (visits) => { - wrapper = shallow(); + const createWrapper = (visits, selectedVisits = []) => { + wrapper = shallow( + + ); return wrapper; }; - afterEach(() => wrapper && wrapper.unmount()); + afterEach(() => { + jest.resetAllMocks(); + wrapper && wrapper.unmount(); + }); it('renders columns as expected', () => { const wrapper = createWrapper([]); @@ -44,7 +55,7 @@ describe('', () => { [ 60, 3 ], [ 115, 6 ], ])('renders the expected amount of pages', (visitsCount, expectedAmountOfPages) => { - const wrapper = createWrapper(rangeOf(visitsCount, () => ({ userAgent: '', date: '', referer: '' }))); + const wrapper = createWrapper(rangeOf(visitsCount, () => ({ browser: '', date: '', referer: '' }))); const tr = wrapper.find('tbody').find('tr'); const paginator = wrapper.find(SimplePaginator); @@ -55,7 +66,7 @@ describe('', () => { it.each( rangeOf(20, (value) => [ value ]) )('does not render footer when there is only one page to render', (visitsCount) => { - const wrapper = createWrapper(rangeOf(visitsCount, () => ({ userAgent: '', date: '', referer: '' }))); + const wrapper = createWrapper(rangeOf(visitsCount, () => ({ browser: '', date: '', referer: '' }))); const tr = wrapper.find('tbody').find('tr'); const paginator = wrapper.find(SimplePaginator); @@ -64,39 +75,34 @@ describe('', () => { }); it('selected rows are highlighted', () => { - const wrapper = createWrapper(rangeOf(10, () => ({ userAgent: '', date: '', referer: '' }))); + const visits = rangeOf(10, () => ({ browser: '', date: '', referer: '' })); + const wrapper = createWrapper( + visits, + [ visits[1], visits[2] ], + ); - expect(wrapper.find('.text-primary')).toHaveLength(0); - expect(wrapper.find('.table-primary')).toHaveLength(0); - wrapper.find('tr').at(5).simulate('click'); - expect(wrapper.find('.text-primary')).toHaveLength(2); - expect(wrapper.find('.table-primary')).toHaveLength(1); - wrapper.find('tr').at(3).simulate('click'); expect(wrapper.find('.text-primary')).toHaveLength(3); expect(wrapper.find('.table-primary')).toHaveLength(2); + + // Select one extra + wrapper.find('tr').at(5).simulate('click'); + expect(setSelectedVisits).toHaveBeenCalledWith([ visits[1], visits[2], visits[4] ]); + + // Deselect one wrapper.find('tr').at(3).simulate('click'); - expect(wrapper.find('.text-primary')).toHaveLength(2); - expect(wrapper.find('.table-primary')).toHaveLength(1); + expect(setSelectedVisits).toHaveBeenCalledWith([ visits[1] ]); // Select all wrapper.find('thead').find('th').at(0).simulate('click'); - expect(wrapper.find('.text-primary')).toHaveLength(11); - expect(wrapper.find('.table-primary')).toHaveLength(10); - - // Select none - wrapper.find('thead').find('th').at(0).simulate('click'); - expect(wrapper.find('.text-primary')).toHaveLength(0); - expect(wrapper.find('.table-primary')).toHaveLength(0); + expect(setSelectedVisits).toHaveBeenCalledWith(visits); }); it('orders visits when column is clicked', () => { const wrapper = createWrapper(rangeOf(9, (index) => ({ - userAgent: '', + browser: '', date: `${9 - index}`, referer: `${index}`, - visitLocation: { - countryName: `Country_${index}`, - }, + country: `Country_${index}`, }))); expect(wrapper.find('tbody').find('tr').at(0).find('td').at(2).text()).toContain('Country_1'); @@ -112,8 +118,8 @@ describe('', () => { it('filters list when writing in search box', () => { const wrapper = createWrapper([ - ...rangeOf(7, () => ({ userAgent: 'aaa', date: 'aaa', referer: 'aaa' })), - ...rangeOf(2, () => ({ userAgent: 'bbb', date: 'bbb', referer: 'bbb' })), + ...rangeOf(7, () => ({ browser: 'aaa', date: 'aaa', referer: 'aaa' })), + ...rangeOf(2, () => ({ browser: 'bbb', date: 'bbb', referer: 'bbb' })), ]); const searchField = wrapper.find(SearchField); diff --git a/test/visits/services/VisitsParser.test.js b/test/visits/services/VisitsParser.test.js index 01041e53..a10eef13 100644 --- a/test/visits/services/VisitsParser.test.js +++ b/test/visits/services/VisitsParser.test.js @@ -1,4 +1,4 @@ -import { processStatsFromVisits } from '../../../src/visits/services/VisitsParser'; +import { processStatsFromVisits, normalizeVisits } from '../../../src/visits/services/VisitsParser'; describe('VisitsParser', () => { const visits = [ @@ -47,7 +47,7 @@ describe('VisitsParser', () => { let stats; beforeAll(() => { - stats = processStatsFromVisits({ id: 'id', visits }); + stats = processStatsFromVisits(visits); }); it('properly parses OS stats', () => { @@ -121,4 +121,51 @@ describe('VisitsParser', () => { }); }); }); + + describe('normalizeVisits', () => { + it('properly parses the list of visits', () => { + expect(normalizeVisits(visits)).toEqual([ + { + browser: 'Firefox', + os: 'Windows', + referer: 'google.com', + country: 'Spain', + city: 'Zaragoza', + date: undefined, + }, + { + browser: 'Firefox', + os: 'MacOS', + referer: 'google.com', + country: 'United States', + city: 'New York', + date: undefined, + }, + { + browser: 'Chrome', + os: 'Linux', + referer: 'Direct', + country: 'Spain', + city: 'Unknown', + date: undefined, + }, + { + browser: 'Chrome', + os: 'Linux', + referer: 'm.facebook.com', + country: 'Spain', + city: 'Zaragoza', + date: undefined, + }, + { + browser: 'Opera', + os: 'Linux', + referer: 'Direct', + country: 'Unknown', + city: 'Unknown', + date: undefined, + }, + ]); + }); + }); }); From c67a23c988094253b971497283a3fcc32be88395 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 10 Apr 2020 12:25:06 +0200 Subject: [PATCH 2/9] Added support to disable date inputs --- src/utils/DateInput.js | 1 + src/utils/DateInput.scss | 2 ++ src/utils/DateRangeRow.js | 5 ++++- src/visits/ShortUrlVisits.js | 5 +++-- 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/utils/DateInput.js b/src/utils/DateInput.js index 382ae753..52905c62 100644 --- a/src/utils/DateInput.js +++ b/src/utils/DateInput.js @@ -12,6 +12,7 @@ const propTypes = { isClearable: PropTypes.bool, selected: PropTypes.oneOfType([ PropTypes.string, PropTypes.object ]), ref: PropTypes.object, + disabled: PropTypes.bool, }; const DateInput = (props) => { diff --git a/src/utils/DateInput.scss b/src/utils/DateInput.scss index 5f2e23ff..cbcbe996 100644 --- a/src/utils/DateInput.scss +++ b/src/utils/DateInput.scss @@ -7,6 +7,8 @@ .date-input-container__input { padding-right: 35px !important; +} +.date-input-container__input:not(:disabled) { background-color: #fff !important; } diff --git a/src/utils/DateRangeRow.js b/src/utils/DateRangeRow.js index 376ee671..ca547e4a 100644 --- a/src/utils/DateRangeRow.js +++ b/src/utils/DateRangeRow.js @@ -9,9 +9,10 @@ const propTypes = { endDate: dateType, onStartDateChange: PropTypes.func.isRequired, onEndDateChange: PropTypes.func.isRequired, + disabled: PropTypes.bool, }; -const DateRangeRow = ({ startDate, endDate, onStartDateChange, onEndDateChange }) => ( +const DateRangeRow = ({ startDate, endDate, onStartDateChange, onEndDateChange, disabled = false }) => (
@@ -29,6 +31,7 @@ const DateRangeRow = ({ startDate, endDate, onStartDateChange, onEndDateChange } placeholderText="Until" isClearable minDate={startDate} + disabled={disabled} onChange={onEndDateChange} />
diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js index 5c233bb9..9415cf77 100644 --- a/src/visits/ShortUrlVisits.js +++ b/src/visits/ShortUrlVisits.js @@ -175,6 +175,7 @@ const ShortUrlVisits = ({ processStatsFromVisits, normalizeVisits }, OpenMapModa
{showTableControls && ( )}
From 73256dcf5b165e644f6ab76879d970099542a63c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 10 Apr 2020 12:53:54 +0200 Subject: [PATCH 3/9] Handled toggling between highlighted chart bars --- CHANGELOG.md | 6 +++++- src/utils/DateInput.scss | 1 + src/visits/ShortUrlVisits.js | 20 ++++++++++++++++---- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 654cbc4d..b110552c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,10 +8,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), #### Added -* [#199](https://github.com/shlinkio/shlink-web-client/issues/199) Added table to visits page which displays the information in a pagintaed, sortable and filterable list. +* [#199](https://github.com/shlinkio/shlink-web-client/issues/199) Added table to visits page which displays the information in a paginated, sortable and filterable list. It also supports selecting multiple visits in the table which makes the corresponding data to be highlighted in the visits charts. +* [#241](https://github.com/shlinkio/shlink-web-client/issues/241) Added support to select charts bars in order to highlight related stats in other charts. + + It also selects the visits in the new table, and you can even combine a selection in the chart and in the table. + * [#213](https://github.com/shlinkio/shlink-web-client/issues/213) The versions of both shlink-web-client and currently consumed Shlink server are now displayed in the footer. * [#221](https://github.com/shlinkio/shlink-web-client/issues/221) Improved how servers are handled, displaying meaningful errors when a not-found or a not-reachable server is tried to be loaded. * [#226](https://github.com/shlinkio/shlink-web-client/issues/226) Created servers can now be edited. diff --git a/src/utils/DateInput.scss b/src/utils/DateInput.scss index cbcbe996..616e74ab 100644 --- a/src/utils/DateInput.scss +++ b/src/utils/DateInput.scss @@ -8,6 +8,7 @@ .date-input-container__input { padding-right: 35px !important; } + .date-input-container__input:not(:disabled) { background-color: #fff !important; } diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js index 9415cf77..97787234 100644 --- a/src/visits/ShortUrlVisits.js +++ b/src/visits/ShortUrlVisits.js @@ -41,6 +41,7 @@ const highlightedVisitsToStats = (highlightedVisits, prop) => highlightedVisits. return acc; }, {}); const format = formatDate(); +let selectedBar; const ShortUrlVisits = ({ processStatsFromVisits, normalizeVisits }, OpenMapModalBtn) => { const ShortUrlVisitsComp = ({ @@ -60,9 +61,17 @@ const ShortUrlVisits = ({ processStatsFromVisits, normalizeVisits }, OpenMapModa const [ highlightedVisits, setHighlightedVisits ] = useState([]); const [ isMobileDevice, setIsMobileDevice ] = useState(false); const determineIsMobileDevice = () => setIsMobileDevice(matchMedia('(max-width: 991px)').matches); - const highlightVisitsForProp = (prop) => (value) => setHighlightedVisits( - highlightedVisits.length === 0 ? normalizedVisits.filter(propEq(prop, value)) : [] - ); + const highlightVisitsForProp = (prop) => (value) => { + const newSelectedBar = `${prop}_${value}`; + + if (selectedBar === newSelectedBar) { + setHighlightedVisits([]); + selectedBar = undefined; + } else { + setHighlightedVisits(normalizedVisits.filter(propEq(prop, value))); + selectedBar = newSelectedBar; + } + }; const { params } = match; const { shortCode } = params; @@ -203,7 +212,10 @@ const ShortUrlVisits = ({ processStatsFromVisits, normalizeVisits }, OpenMapModa { + selectedBar = undefined; + setHighlightedVisits(selectedVisits); + }} isSticky={tableIsSticky} /> From ed584d19e5ad5bf251515a1659818b385eed523f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 10 Apr 2020 12:57:14 +0200 Subject: [PATCH 4/9] Ensured charts datasets have a unique label --- src/visits/GraphCard.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/visits/GraphCard.js b/src/visits/GraphCard.js index 695be8fb..b7c221a0 100644 --- a/src/visits/GraphCard.js +++ b/src/visits/GraphCard.js @@ -20,7 +20,7 @@ const generateGraphData = (title, isBarChart, labels, data, highlightedData) => datasets: [ { title, - label: '', + label: highlightedData && 'Non-selected', data, backgroundColor: isBarChart ? 'rgba(70, 150, 229, 0.4)' : [ '#97BBCD', From b863c2e19d2df008a7490c506c5ff49136f8f49f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 10 Apr 2020 13:04:39 +0200 Subject: [PATCH 5/9] Used cursor pointer in bar charts --- src/visits/GraphCard.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/visits/GraphCard.js b/src/visits/GraphCard.js index b7c221a0..7cccf30d 100644 --- a/src/visits/GraphCard.js +++ b/src/visits/GraphCard.js @@ -79,6 +79,9 @@ const renderGraph = (title, isBarChart, stats, max, highlightedStats, onClick) = // Do not show tooltip on items with empty label when in a bar chart filter: ({ yLabel }) => !isBarChart || yLabel !== '', }, + onHover: isBarChart && (({ target }, chartElement) => { + target.style.cursor = chartElement[0] ? 'pointer' : 'default'; + }), }; const graphData = generateGraphData(title, isBarChart, labels, data, highlightedData); const height = isBarChart && labels.length > 20 ? labels.length * 8 : null; From 3851342e1b9ff78589eea4a139864c3cd3745827 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 10 Apr 2020 13:27:01 +0200 Subject: [PATCH 6/9] Added button to reset visits selection --- src/visits/ShortUrlVisits.js | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js index 97787234..589dbb78 100644 --- a/src/visits/ShortUrlVisits.js +++ b/src/visits/ShortUrlVisits.js @@ -3,6 +3,7 @@ import React, { useState, useEffect, useMemo } from 'react'; import { Button, Card, Collapse } from 'reactstrap'; import PropTypes from 'prop-types'; import qs from 'qs'; +import classNames from 'classnames'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { faChevronDown as chevronDown } from '@fortawesome/free-solid-svg-icons'; import DateRangeRow from '../utils/DateRangeRow'; @@ -61,6 +62,10 @@ const ShortUrlVisits = ({ processStatsFromVisits, normalizeVisits }, OpenMapModa const [ highlightedVisits, setHighlightedVisits ] = useState([]); const [ isMobileDevice, setIsMobileDevice ] = useState(false); const determineIsMobileDevice = () => setIsMobileDevice(matchMedia('(max-width: 991px)').matches); + const setSelectedVisits = (selectedVisits) => { + selectedBar = undefined; + setHighlightedVisits(selectedVisits); + }; const highlightVisitsForProp = (prop) => (value) => { const newSelectedBar = `${prop}_${value}`; @@ -182,7 +187,7 @@ const ShortUrlVisits = ({ processStatsFromVisits, normalizeVisits }, OpenMapModa
-
+
-
+
{showTableControls && ( - + + + + + + + + )}
@@ -212,10 +231,7 @@ const ShortUrlVisits = ({ processStatsFromVisits, normalizeVisits }, OpenMapModa { - selectedBar = undefined; - setHighlightedVisits(selectedVisits); - }} + setSelectedVisits={setSelectedVisits} isSticky={tableIsSticky} /> From 9d1e48ee905e5bd9e3d16018442c0768fa9c65bc Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 10 Apr 2020 13:42:16 +0200 Subject: [PATCH 7/9] Updated main list paginator to be sticky --- src/short-urls/Paginator.js | 3 ++- src/short-urls/Paginator.scss | 7 +++++++ src/short-urls/ShortUrls.js | 6 ++++-- 3 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 src/short-urls/Paginator.scss diff --git a/src/short-urls/Paginator.js b/src/short-urls/Paginator.js index 46783dc7..dd976954 100644 --- a/src/short-urls/Paginator.js +++ b/src/short-urls/Paginator.js @@ -3,6 +3,7 @@ import { Link } from 'react-router-dom'; import { Pagination, PaginationItem, PaginationLink } from 'reactstrap'; import PropTypes from 'prop-types'; import { isPageDisabled, keyForPage, progressivePagination } from '../utils/helpers/pagination'; +import './Paginator.scss'; const propTypes = { serverId: PropTypes.string.isRequired, @@ -36,7 +37,7 @@ const Paginator = ({ paginator = {}, serverId }) => { )); return ( - + { return (
- - +
+ + +
); }; From fafe920b7bc3292da3d9d8ea7f0de726cfcc6a6a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 10 Apr 2020 14:38:31 +0200 Subject: [PATCH 8/9] Ensured highlighted stats are properly sorted and paginated on charts that support that --- src/visits/SortableBarGraph.js | 47 +++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/src/visits/SortableBarGraph.js b/src/visits/SortableBarGraph.js index aecca4e7..29f664d4 100644 --- a/src/visits/SortableBarGraph.js +++ b/src/visits/SortableBarGraph.js @@ -1,6 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; -import { fromPairs, head, keys, pipe, prop, reverse, sortBy, splitEvery, toLower, toPairs, type } from 'ramda'; +import { fromPairs, head, keys, pipe, prop, reverse, sortBy, splitEvery, toLower, toPairs, type, zipObj } from 'ramda'; import SortingDropdown from '../utils/SortingDropdown'; import PaginationDropdown from '../utils/PaginationDropdown'; import { rangeOf } from '../utils/utils'; @@ -10,6 +10,7 @@ import GraphCard from './GraphCard'; const { max } = Math; const toLowerIfString = (value) => type(value) === 'String' ? toLower(value) : value; +const pickKeyFromPair = ([ key ]) => key; const pickValueFromPair = ([ , value ]) => value; export default class SortableBarGraph extends React.Component { @@ -30,7 +31,7 @@ export default class SortableBarGraph extends React.Component { itemsPerPage: Infinity, }; - determineStats(stats, sortingItems) { + getSortedPairsForStats(stats, sortingItems) { const pairs = toPairs(stats); const sortedPairs = !this.state.orderField ? pairs : sortBy( pipe( @@ -39,18 +40,33 @@ export default class SortableBarGraph extends React.Component { ), pairs ); - const directionalPairs = !this.state.orderDir || this.state.orderDir === 'ASC' ? sortedPairs : reverse(sortedPairs); - if (directionalPairs.length <= this.state.itemsPerPage) { - return { currentPageStats: fromPairs(directionalPairs) }; + return !this.state.orderDir || this.state.orderDir === 'ASC' ? sortedPairs : reverse(sortedPairs); + } + + determineStats(stats, highlightedStats, sortingItems) { + const sortedPairs = this.getSortedPairsForStats(stats, sortingItems); + const sortedKeys = sortedPairs.map(pickKeyFromPair); + // The highlighted stats have to be ordered based on the regular stats, not on its own values + const sortedHighlightedPairs = highlightedStats && toPairs( + { ...zipObj(sortedKeys, sortedKeys.map(() => 0)), ...highlightedStats } + ); + + if (sortedPairs.length <= this.state.itemsPerPage) { + return { + currentPageStats: fromPairs(sortedPairs), + currentPageHighlightedStats: sortedHighlightedPairs && fromPairs(sortedHighlightedPairs), + }; } - const pages = splitEvery(this.state.itemsPerPage, directionalPairs); + const pages = splitEvery(this.state.itemsPerPage, sortedPairs); + const highlightedPages = sortedHighlightedPairs && splitEvery(this.state.itemsPerPage, sortedHighlightedPairs); return { currentPageStats: fromPairs(this.determineCurrentPagePairs(pages)), + currentPageHighlightedStats: highlightedPages && fromPairs(this.determineCurrentPagePairs(highlightedPages)), pagination: this.renderPagination(pages.length), - max: roundTen(max(...directionalPairs.map(pickValueFromPair))), + max: roundTen(max(...sortedPairs.map(pickValueFromPair))), }; } @@ -75,8 +91,20 @@ export default class SortableBarGraph extends React.Component { } render() { - const { stats, sortingItems, title, extraHeaderContent, withPagination = true, ...rest } = this.props; - const { currentPageStats, pagination, max } = this.determineStats(stats, sortingItems); + const { + stats, + highlightedStats, + sortingItems, + title, + extraHeaderContent, + withPagination = true, + ...rest + } = this.props; + const { currentPageStats, currentPageHighlightedStats, pagination, max } = this.determineStats( + stats, + highlightedStats && keys(highlightedStats).length > 0 ? highlightedStats : undefined, + sortingItems + ); const activeCities = keys(currentPageStats); const computeTitle = () => ( @@ -114,6 +142,7 @@ export default class SortableBarGraph extends React.Component { isBarChart title={computeTitle} stats={currentPageStats} + highlightedStats={currentPageHighlightedStats} footer={pagination} max={max} {...rest} From 52dbeb6201d6f5016ec138fa8d6f1f735caffcf1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 10 Apr 2020 14:59:12 +0200 Subject: [PATCH 9/9] Optimized visits parser to act over the normalized list of visits --- src/visits/ShortUrlVisits.js | 4 +- src/visits/services/VisitsParser.js | 50 ++++++++++------------- test/visits/services/VisitsParser.test.js | 24 +++++++---- 3 files changed, 41 insertions(+), 37 deletions(-) diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js index 589dbb78..1a057279 100644 --- a/src/visits/ShortUrlVisits.js +++ b/src/visits/ShortUrlVisits.js @@ -87,8 +87,8 @@ const ShortUrlVisits = ({ processStatsFromVisits, normalizeVisits }, OpenMapModa const showTableControls = !loading && visits.length > 0; const normalizedVisits = useMemo(() => normalizeVisits(visits), [ visits ]); const { os, browsers, referrers, countries, cities, citiesForMap } = useMemo( - () => processStatsFromVisits(visits), - [ visits ] + () => processStatsFromVisits(normalizedVisits), + [ normalizedVisits ] ); const mapLocations = values(citiesForMap); diff --git a/src/visits/services/VisitsParser.js b/src/visits/services/VisitsParser.js index 25db20d8..2ebd5262 100644 --- a/src/visits/services/VisitsParser.js +++ b/src/visits/services/VisitsParser.js @@ -1,60 +1,52 @@ -import { isEmpty, isNil, map } from 'ramda'; +import { isNil, map } from 'ramda'; import { browserFromUserAgent, extractDomain, osFromUserAgent } from '../../utils/helpers/visits'; +import { hasValue } from '../../utils/utils'; -const visitLocationHasProperty = (visitLocation, propertyName) => - !isNil(visitLocation) - && !isNil(visitLocation[propertyName]) - && !isEmpty(visitLocation[propertyName]); - -const updateOsStatsForVisit = (osStats, { userAgent }) => { - const os = osFromUserAgent(userAgent); +const visitHasProperty = (visit, propertyName) => !isNil(visit) && hasValue(visit[propertyName]); +const updateOsStatsForVisit = (osStats, { os }) => { osStats[os] = (osStats[os] || 0) + 1; }; -const updateBrowsersStatsForVisit = (browsersStats, { userAgent }) => { - const browser = browserFromUserAgent(userAgent); - +const updateBrowsersStatsForVisit = (browsersStats, { browser }) => { browsersStats[browser] = (browsersStats[browser] || 0) + 1; }; -const updateReferrersStatsForVisit = (referrersStats, { referer }) => { - const domain = extractDomain(referer); - +const updateReferrersStatsForVisit = (referrersStats, { referer: domain }) => { referrersStats[domain] = (referrersStats[domain] || 0) + 1; }; -const updateLocationsStatsForVisit = (propertyName) => (stats, { visitLocation }) => { - const hasLocationProperty = visitLocationHasProperty(visitLocation, propertyName); - const value = hasLocationProperty ? visitLocation[propertyName] : 'Unknown'; +const updateLocationsStatsForVisit = (propertyName) => (stats, visit) => { + const hasLocationProperty = visitHasProperty(visit, propertyName); + const value = hasLocationProperty ? visit[propertyName] : 'Unknown'; stats[value] = (stats[value] || 0) + 1; }; -const updateCountriesStatsForVisit = updateLocationsStatsForVisit('countryName'); -const updateCitiesStatsForVisit = updateLocationsStatsForVisit('cityName'); +const updateCountriesStatsForVisit = updateLocationsStatsForVisit('country'); +const updateCitiesStatsForVisit = updateLocationsStatsForVisit('city'); -const updateCitiesForMapForVisit = (citiesForMapStats, { visitLocation }) => { - if (!visitLocationHasProperty(visitLocation, 'cityName')) { +const updateCitiesForMapForVisit = (citiesForMapStats, visit) => { + if (!visitHasProperty(visit, 'city') || visit.city === 'Unknown') { return; } - const { cityName, latitude, longitude } = visitLocation; - const currentCity = citiesForMapStats[cityName] || { - cityName, + const { city, latitude, longitude } = visit; + const currentCity = citiesForMapStats[city] || { + cityName: city, count: 0, latLong: [ parseFloat(latitude), parseFloat(longitude) ], }; currentCity.count++; - citiesForMapStats[cityName] = currentCity; + citiesForMapStats[city] = currentCity; }; -export const processStatsFromVisits = (visits) => - visits.reduce( +export const processStatsFromVisits = (normalizedVisits) => + normalizedVisits.reduce( (stats, visit) => { - // We mutate the original object because it has a big side effect when large data sets are processed + // We mutate the original object because it has a big performance impact when large data sets are processed updateOsStatsForVisit(stats.os, visit); updateBrowsersStatsForVisit(stats.browsers, visit); updateReferrersStatsForVisit(stats.referrers, visit); @@ -74,4 +66,6 @@ export const normalizeVisits = map(({ userAgent, date, referer, visitLocation }) referer: extractDomain(referer), country: (visitLocation && visitLocation.countryName) || 'Unknown', city: (visitLocation && visitLocation.cityName) || 'Unknown', + latitude: visitLocation && visitLocation.latitude, + longitude: visitLocation && visitLocation.longitude, })); diff --git a/test/visits/services/VisitsParser.test.js b/test/visits/services/VisitsParser.test.js index a10eef13..3538be10 100644 --- a/test/visits/services/VisitsParser.test.js +++ b/test/visits/services/VisitsParser.test.js @@ -8,8 +8,8 @@ describe('VisitsParser', () => { visitLocation: { countryName: 'Spain', cityName: 'Zaragoza', - latitude: '123.45', - longitude: '-543.21', + latitude: 123.45, + longitude: -543.21, }, }, { @@ -18,8 +18,8 @@ describe('VisitsParser', () => { visitLocation: { countryName: 'United States', cityName: 'New York', - latitude: '1029', - longitude: '6758', + latitude: 1029, + longitude: 6758, }, }, { @@ -34,8 +34,8 @@ describe('VisitsParser', () => { visitLocation: { countryName: 'Spain', cityName: 'Zaragoza', - latitude: '123.45', - longitude: '-543.21', + latitude: 123.45, + longitude: -543.21, }, }, { @@ -47,7 +47,7 @@ describe('VisitsParser', () => { let stats; beforeAll(() => { - stats = processStatsFromVisits(visits); + stats = processStatsFromVisits(normalizeVisits(visits)); }); it('properly parses OS stats', () => { @@ -132,6 +132,8 @@ describe('VisitsParser', () => { country: 'Spain', city: 'Zaragoza', date: undefined, + latitude: 123.45, + longitude: -543.21, }, { browser: 'Firefox', @@ -140,6 +142,8 @@ describe('VisitsParser', () => { country: 'United States', city: 'New York', date: undefined, + latitude: 1029, + longitude: 6758, }, { browser: 'Chrome', @@ -148,6 +152,8 @@ describe('VisitsParser', () => { country: 'Spain', city: 'Unknown', date: undefined, + latitude: undefined, + longitude: undefined, }, { browser: 'Chrome', @@ -156,6 +162,8 @@ describe('VisitsParser', () => { country: 'Spain', city: 'Zaragoza', date: undefined, + latitude: 123.45, + longitude: -543.21, }, { browser: 'Opera', @@ -164,6 +172,8 @@ describe('VisitsParser', () => { country: 'Unknown', city: 'Unknown', date: undefined, + latitude: undefined, + longitude: undefined, }, ]); });