From bbf25108c7fa64a66b173d67005996dea9f190f1 Mon Sep 17 00:00:00 2001 From: Neel Mehta Date: Wed, 29 Apr 2020 22:39:17 -0400 Subject: [PATCH 1/8] fix config for on time rate row --- frontend/src/components/RouteSummary.jsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frontend/src/components/RouteSummary.jsx b/frontend/src/components/RouteSummary.jsx index 7904dc78..0e4ed1a6 100644 --- a/frontend/src/components/RouteSummary.jsx +++ b/frontend/src/components/RouteSummary.jsx @@ -150,6 +150,9 @@ function RouteSummary(props) { scheduled="" units="%" precision={0} + positiveDiffDesc="higher" + negativeDiffDesc="lower" + goodDiffDirection={1} infoContent={ <> This is the percentage of scheduled departure times where a From db80487e62c580ce433a7f1a81332c90728ea821 Mon Sep 17 00:00:00 2001 From: Neel Mehta Date: Wed, 29 Apr 2020 22:43:06 -0400 Subject: [PATCH 2/8] refactor color selection code --- frontend/src/components/SummaryRow.jsx | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/SummaryRow.jsx b/frontend/src/components/SummaryRow.jsx index 9c27e0db..beb11863 100644 --- a/frontend/src/components/SummaryRow.jsx +++ b/frontend/src/components/SummaryRow.jsx @@ -109,14 +109,19 @@ export default function SummaryRow(props) { fontSize: 16, }; - let comparisonCellColor = 'green'; + const COMPARISON_GOOD_COLOR = 'green'; + const COMPARISON_BAD_COLOR = '#f07d02'; + + let comparisonCellColor; if ( goodDiffDirection != null && diff != null && firstColumnText !== secondColumnText && goodDiffDirection * diff < 0 ) { - comparisonCellColor = '#f07d02'; + comparisonCellColor = COMPARISON_BAD_COLOR; + } else { + comparisonCellColor = COMPARISON_GOOD_COLOR; } let comparisonText = null; From 942bf43876843ba957a39175ef67bf8de91f89ab Mon Sep 17 00:00:00 2001 From: Neel Mehta Date: Wed, 29 Apr 2020 22:56:50 -0400 Subject: [PATCH 3/8] refactor less-than-1 rendering code --- frontend/src/components/SummaryRow.jsx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/frontend/src/components/SummaryRow.jsx b/frontend/src/components/SummaryRow.jsx index beb11863..6d0b4fe3 100644 --- a/frontend/src/components/SummaryRow.jsx +++ b/frontend/src/components/SummaryRow.jsx @@ -125,12 +125,11 @@ export default function SummaryRow(props) { } let comparisonText = null; - if (diff != null && firstColumnText !== secondColumnText) { + console.log(diff, firstColumnText, secondColumnText); + if (diff != null) { const absDiff = Math.abs(diff); - let diffStr = absDiff.toFixed(precision); - if (diffStr === '0') { - diffStr = '< 1'; - } + const diffStr = absDiff < 1 ? '< 1' : absDiff.toFixed(precision); + comparisonText = `${diffStr}${unitsSuffix} ${ diff > 0 ? positiveDiffDesc : negativeDiffDesc }`; // ${diffPercentStr}`; From 913706957edc0cfad2609fb5d0c91e1ec569bce8 Mon Sep 17 00:00:00 2001 From: Neel Mehta Date: Wed, 29 Apr 2020 22:58:22 -0400 Subject: [PATCH 4/8] add comment --- frontend/src/components/SummaryRow.jsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frontend/src/components/SummaryRow.jsx b/frontend/src/components/SummaryRow.jsx index 6d0b4fe3..fb7ee044 100644 --- a/frontend/src/components/SummaryRow.jsx +++ b/frontend/src/components/SummaryRow.jsx @@ -128,6 +128,8 @@ export default function SummaryRow(props) { console.log(diff, firstColumnText, secondColumnText); if (diff != null) { const absDiff = Math.abs(diff); + // Not sure if this should be <1 or <0.5. A number like 0.6 rounds to 1, + // but it's also less than 1, so both "1" and "<1" could apply. const diffStr = absDiff < 1 ? '< 1' : absDiff.toFixed(precision); comparisonText = `${diffStr}${unitsSuffix} ${ From 3a1c3b9ef0311ecffa8cfa35d4bcab8b16c98fbd Mon Sep 17 00:00:00 2001 From: Neel Mehta Date: Wed, 6 May 2020 21:34:04 -0400 Subject: [PATCH 5/8] add comment and change cutoff --- frontend/src/components/SummaryRow.jsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/frontend/src/components/SummaryRow.jsx b/frontend/src/components/SummaryRow.jsx index fb7ee044..e4e91630 100644 --- a/frontend/src/components/SummaryRow.jsx +++ b/frontend/src/components/SummaryRow.jsx @@ -128,13 +128,14 @@ export default function SummaryRow(props) { console.log(diff, firstColumnText, secondColumnText); if (diff != null) { const absDiff = Math.abs(diff); - // Not sure if this should be <1 or <0.5. A number like 0.6 rounds to 1, - // but it's also less than 1, so both "1" and "<1" could apply. - const diffStr = absDiff < 1 ? '< 1' : absDiff.toFixed(precision); + // DESIGN DECISION: + // Not sure if this cutoff should be <1 or <0.5. A number like 0.6 + // rounds to 1, but it's also less than 1, so both "1" and "<1" could apply. + const diffStr = absDiff < 0.5 ? '< 1' : absDiff.toFixed(precision); comparisonText = `${diffStr}${unitsSuffix} ${ diff > 0 ? positiveDiffDesc : negativeDiffDesc - }`; // ${diffPercentStr}`; + }`; } return ( From 1f90ad8da4fe1f3489987a0df9ad6b78a8f833b8 Mon Sep 17 00:00:00 2001 From: Neel Mehta Date: Wed, 6 May 2020 21:56:07 -0400 Subject: [PATCH 6/8] refactor and fix regression --- frontend/src/components/SummaryRow.jsx | 40 +++++++++++++++----------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/frontend/src/components/SummaryRow.jsx b/frontend/src/components/SummaryRow.jsx index e4e91630..db9efa1d 100644 --- a/frontend/src/components/SummaryRow.jsx +++ b/frontend/src/components/SummaryRow.jsx @@ -6,6 +6,8 @@ import { makeStyles } from '@material-ui/core/styles'; import IconButton from '@material-ui/core/IconButton'; import Popover from '@material-ui/core/Popover'; import InfoIcon from '@material-ui/icons/InfoOutlined'; +import green from '@material-ui/core/colors/green'; +import red from '@material-ui/core/colors/red'; /* * Renders a row of a table that renders two columns of data @@ -109,24 +111,18 @@ export default function SummaryRow(props) { fontSize: 16, }; - const COMPARISON_GOOD_COLOR = 'green'; - const COMPARISON_BAD_COLOR = '#f07d02'; - - let comparisonCellColor; - if ( - goodDiffDirection != null && - diff != null && - firstColumnText !== secondColumnText && - goodDiffDirection * diff < 0 - ) { - comparisonCellColor = COMPARISON_BAD_COLOR; - } else { - comparisonCellColor = COMPARISON_GOOD_COLOR; - } - + // Determine what to write as the comparison (which one is better and by + // how much) let comparisonText = null; - console.log(diff, firstColumnText, secondColumnText); - if (diff != null) { + let comparisonCellColor = null; + + if (firstColumnText === secondColumnText) { + // the two routes' stats will appear identical, so don't write anything + comparisonText = null; + } else if (diff != null) { + // write something like "X mph higher" + + // Determine text const absDiff = Math.abs(diff); // DESIGN DECISION: // Not sure if this cutoff should be <1 or <0.5. A number like 0.6 @@ -136,6 +132,16 @@ export default function SummaryRow(props) { comparisonText = `${diffStr}${unitsSuffix} ${ diff > 0 ? positiveDiffDesc : negativeDiffDesc }`; + + // Determine color + // const COMPARISON_EVEN_COLOR = grey[700]; + const COMPARISON_GOOD_COLOR = green[700]; + const COMPARISON_BAD_COLOR = red[700]; + if (goodDiffDirection != null && goodDiffDirection * diff < 0) { + comparisonCellColor = COMPARISON_BAD_COLOR; + } else { + comparisonCellColor = COMPARISON_GOOD_COLOR; + } } return ( From 6e16cfa8670e76cffb9a1f743605aede37c35e63 Mon Sep 17 00:00:00 2001 From: Neel Mehta Date: Wed, 6 May 2020 22:00:57 -0400 Subject: [PATCH 7/8] improve logic for showing nothing --- frontend/src/components/SummaryRow.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/SummaryRow.jsx b/frontend/src/components/SummaryRow.jsx index db9efa1d..c25d34d9 100644 --- a/frontend/src/components/SummaryRow.jsx +++ b/frontend/src/components/SummaryRow.jsx @@ -116,8 +116,8 @@ export default function SummaryRow(props) { let comparisonText = null; let comparisonCellColor = null; - if (firstColumnText === secondColumnText) { - // the two routes' stats will appear identical, so don't write anything + if (diff === 0) { + // the two routes' stats are identical, so don't write anything comparisonText = null; } else if (diff != null) { // write something like "X mph higher" From 00f9a6b0e274e21f4a4e2a1c6570f92bf0bd17af Mon Sep 17 00:00:00 2001 From: Neel Mehta Date: Wed, 6 May 2020 22:05:36 -0400 Subject: [PATCH 8/8] better variables --- frontend/src/components/SummaryRow.jsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/SummaryRow.jsx b/frontend/src/components/SummaryRow.jsx index c25d34d9..0cac90ed 100644 --- a/frontend/src/components/SummaryRow.jsx +++ b/frontend/src/components/SummaryRow.jsx @@ -127,14 +127,15 @@ export default function SummaryRow(props) { // DESIGN DECISION: // Not sure if this cutoff should be <1 or <0.5. A number like 0.6 // rounds to 1, but it's also less than 1, so both "1" and "<1" could apply. - const diffStr = absDiff < 0.5 ? '< 1' : absDiff.toFixed(precision); + const NEGLIGIBLE_CUTOFF = 0.5; + const diffStr = + absDiff < NEGLIGIBLE_CUTOFF ? '< 1' : absDiff.toFixed(precision); comparisonText = `${diffStr}${unitsSuffix} ${ diff > 0 ? positiveDiffDesc : negativeDiffDesc }`; // Determine color - // const COMPARISON_EVEN_COLOR = grey[700]; const COMPARISON_GOOD_COLOR = green[700]; const COMPARISON_BAD_COLOR = red[700]; if (goodDiffDirection != null && goodDiffDirection * diff < 0) {