From 812e391e3411976c7a735e2d4d9a0fb5a1d24c4c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 19 Oct 2018 19:04:22 +0200 Subject: [PATCH 1/2] Moved helper functions in GraphCard outside of component function --- src/visits/GraphCard.js | 83 +++++++++++++++--------------- src/visits/ShortUrlVisits.js | 2 +- test/visits/GraphCard.test.js | 2 +- test/visits/ShortUrlVisits.test.js | 2 +- 4 files changed, 45 insertions(+), 44 deletions(-) diff --git a/src/visits/GraphCard.js b/src/visits/GraphCard.js index e1a814da..f0c5f49b 100644 --- a/src/visits/GraphCard.js +++ b/src/visits/GraphCard.js @@ -10,49 +10,50 @@ const propTypes = { stats: PropTypes.object, }; -export function GraphCard({ title, isBarChart, stats }) { - const generateGraphData = (stats) => ({ - labels: keys(stats), - datasets: [ - { - title, - data: values(stats), - backgroundColor: isBarChart ? 'rgba(70, 150, 229, 0.4)' : [ - '#97BBCD', - '#DCDCDC', - '#F7464A', - '#46BFBD', - '#FDB45C', - '#949FB1', - '#4D5360', - ], - borderColor: isBarChart ? 'rgba(70, 150, 229, 1)' : 'white', - borderWidth: 2, - }, - ], - }); - const renderGraph = () => { - const Component = isBarChart ? HorizontalBar : Doughnut; - const options = { - legend: isBarChart ? { display: false } : { position: 'right' }, - scales: isBarChart ? { - xAxes: [ - { - ticks: { beginAtZero: true }, - }, - ], - } : null, - }; +const generateGraphData = (title, isBarChart, stats) => ({ + labels: keys(stats), + datasets: [ + { + title, + data: values(stats), + backgroundColor: isBarChart ? 'rgba(70, 150, 229, 0.4)' : [ + '#97BBCD', + '#DCDCDC', + '#F7464A', + '#46BFBD', + '#FDB45C', + '#949FB1', + '#4D5360', + ], + borderColor: isBarChart ? 'rgba(70, 150, 229, 1)' : 'white', + borderWidth: 2, + }, + ], +}); - return ; +const renderGraph = (title, isBarChart, stats) => { + const Component = isBarChart ? HorizontalBar : Doughnut; + const options = { + legend: isBarChart ? { display: false } : { position: 'right' }, + scales: isBarChart ? { + xAxes: [ + { + ticks: { beginAtZero: true }, + }, + ], + } : null, }; - return ( - - {title} - {renderGraph()} - - ); -} + return ; +}; + +const GraphCard = ({ title, isBarChart, stats }) => ( + + {title} + {renderGraph(title, isBarChart, stats)} + +); GraphCard.propTypes = propTypes; + +export default GraphCard; diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js index 11256c0e..72d162cb 100644 --- a/src/visits/ShortUrlVisits.js +++ b/src/visits/ShortUrlVisits.js @@ -15,7 +15,7 @@ import { processReferrersStats, } from './services/VisitsParser'; import { VisitsHeader } from './VisitsHeader'; -import { GraphCard } from './GraphCard'; +import GraphCard from './GraphCard'; import { getShortUrlDetail, shortUrlDetailType } from './reducers/shortUrlDetail'; import './ShortUrlVisits.scss'; diff --git a/test/visits/GraphCard.test.js b/test/visits/GraphCard.test.js index 50d2ae14..61fda396 100644 --- a/test/visits/GraphCard.test.js +++ b/test/visits/GraphCard.test.js @@ -2,7 +2,7 @@ import React from 'react'; import { shallow } from 'enzyme'; import { Doughnut, HorizontalBar } from 'react-chartjs-2'; import { keys, values } from 'ramda'; -import { GraphCard } from '../../src/visits/GraphCard'; +import GraphCard from '../../src/visits/GraphCard'; describe('', () => { let wrapper; diff --git a/test/visits/ShortUrlVisits.test.js b/test/visits/ShortUrlVisits.test.js index d7d2a3e8..56837968 100644 --- a/test/visits/ShortUrlVisits.test.js +++ b/test/visits/ShortUrlVisits.test.js @@ -5,7 +5,7 @@ import { Card } from 'reactstrap'; import * as sinon from 'sinon'; import { ShortUrlsVisitsComponent as ShortUrlsVisits } from '../../src/visits/ShortUrlVisits'; import MutedMessage from '../../src/utils/MuttedMessage'; -import { GraphCard } from '../../src/visits/GraphCard'; +import GraphCard from '../../src/visits/GraphCard'; import DateInput from '../../src/common/DateInput'; describe('', () => { From 0e8631ae9dc2b83b6ce0ca40d2962c7e2c943c21 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 19 Oct 2018 20:27:25 +0200 Subject: [PATCH 2/2] Updated GraphCard so that it automatically calculates the proper aspect ration for bar chart graphs --- src/visits/GraphCard.js | 48 ++++++++++++++++++++++++++++++----- test/visits/GraphCard.test.js | 5 ++-- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/src/visits/GraphCard.js b/src/visits/GraphCard.js index f0c5f49b..4351fefa 100644 --- a/src/visits/GraphCard.js +++ b/src/visits/GraphCard.js @@ -8,14 +8,18 @@ const propTypes = { title: PropTypes.string, isBarChart: PropTypes.bool, stats: PropTypes.object, + matchMedia: PropTypes.func, +}; +const defaultProps = { + matchMedia: global.window ? global.window.matchMedia : () => {}, }; -const generateGraphData = (title, isBarChart, stats) => ({ - labels: keys(stats), +const generateGraphData = (title, isBarChart, labels, data) => ({ + labels, datasets: [ { title, - data: values(stats), + data, backgroundColor: isBarChart ? 'rgba(70, 150, 229, 0.4)' : [ '#97BBCD', '#DCDCDC', @@ -31,9 +35,38 @@ const generateGraphData = (title, isBarChart, stats) => ({ ], }); -const renderGraph = (title, isBarChart, stats) => { +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 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 Component = isBarChart ? HorizontalBar : Doughnut; + const labels = keys(stats); + const data = values(stats); + const aspectRatio = determineGraphAspectRatio(labels.length, isBarChart, matchMedia); const options = { + aspectRatio, legend: isBarChart ? { display: false } : { position: 'right' }, scales: isBarChart ? { xAxes: [ @@ -44,16 +77,17 @@ const renderGraph = (title, isBarChart, stats) => { } : null, }; - return ; + return ; }; -const GraphCard = ({ title, isBarChart, stats }) => ( +const GraphCard = ({ title, isBarChart, stats, matchMedia }) => ( {title} - {renderGraph(title, isBarChart, stats)} + {renderGraph(title, isBarChart, stats, matchMedia)} ); GraphCard.propTypes = propTypes; +GraphCard.defaultProps = defaultProps; export default GraphCard; diff --git a/test/visits/GraphCard.test.js b/test/visits/GraphCard.test.js index 61fda396..0525efd0 100644 --- a/test/visits/GraphCard.test.js +++ b/test/visits/GraphCard.test.js @@ -10,6 +10,7 @@ describe('', () => { foo: 123, bar: 456, }; + const matchMedia = () => ({ matches: false }); afterEach(() => { if (wrapper) { @@ -18,7 +19,7 @@ describe('', () => { }); it('renders Doughnut when is not a bar chart', () => { - wrapper = shallow(); + wrapper = shallow(); const doughnut = wrapper.find(Doughnut); const horizontal = wrapper.find(HorizontalBar); @@ -46,7 +47,7 @@ describe('', () => { }); it('renders HorizontalBar when is not a bar chart', () => { - wrapper = shallow(); + wrapper = shallow(); const doughnut = wrapper.find(Doughnut); const horizontal = wrapper.find(HorizontalBar);