From 0db52ff977dcd8dbe51707da4f08d585b74ba60e Mon Sep 17 00:00:00 2001 From: Aviral Date: Tue, 14 Feb 2023 12:54:22 +0100 Subject: [PATCH] Add year to X axis of multi-year graph (#2607) * Add year to X axis of multi-year graph * Remove hardcoded condition for rendering year string, render if multiple years present in graph view * Apply new argument contract to dateFormatter usage in graph-util.js * Better defensive runtime type guard when deriving hasMultipleYears * Remove console log * Remove unrelated JSDoc change --------- Co-authored-by: Vini Brasil --- .../dashboard/stats/graph/date-formatter.js | 44 +++++++++++-------- assets/js/dashboard/stats/graph/graph-util.js | 10 +++-- .../js/dashboard/stats/graph/visitor-graph.js | 32 ++++++++++++-- 3 files changed, 60 insertions(+), 26 deletions(-) diff --git a/assets/js/dashboard/stats/graph/date-formatter.js b/assets/js/dashboard/stats/graph/date-formatter.js index a44beaaca..a81732140 100644 --- a/assets/js/dashboard/stats/graph/date-formatter.js +++ b/assets/js/dashboard/stats/graph/date-formatter.js @@ -1,18 +1,21 @@ -import {parseUTCDate, formatMonthYYYY, formatDay, formatDayShort} from '../../util/date' +import { parseUTCDate, formatMonthYYYY, formatDay, formatDayShort } from '../../util/date' const browserDateFormat = Intl.DateTimeFormat(navigator.language, { hour: 'numeric' }) -const is12HourClock = function() { +const is12HourClock = function () { return browserDateFormat.resolvedOptions().hour12 } -const parseISODate = function(isoDate) { +const parseISODate = function (isoDate) { const date = parseUTCDate(isoDate) const minutes = date.getMinutes(); - return { date, minutes } + const year = date.getFullYear() + return { date, minutes, year } } -const formatHours = function(isoDate) { +const getYearString = (options, year) => options.shouldShowYear ? ` ${year}` : '' + +const formatHours = function (isoDate) { const monthIndex = 1 const dateParts = isoDate.split(/[^0-9]/); dateParts[monthIndex] = dateParts[monthIndex] - 1 @@ -37,9 +40,9 @@ const weekIntervalFormatter = { const formatted = this.short(isoDate, options) return options.isBucketPartial ? `Partial week of ${formatted}` : `Week of ${formatted}` }, - short(isoDate, _options) { - const { date } = parseISODate(isoDate) - return formatDayShort(date) + short(isoDate, options) { + const { date, year } = parseISODate(isoDate) + return `${formatDayShort(date)}${getYearString(options, year)}` } } @@ -48,9 +51,9 @@ const dateIntervalFormatter = { const { date } = parseISODate(isoDate) return formatDay(date) }, - short(isoDate, _options) { - const { date } = parseISODate(isoDate) - return formatDayShort(date) + short(isoDate, options) { + const { date, year } = parseISODate(isoDate) + return `${formatDayShort(date)}${getYearString(options, year)}` } } @@ -60,7 +63,7 @@ const hourIntervalFormatter = { }, short(isoDate, _options) { const formatted = formatHours(isoDate) - + if (is12HourClock()) { return formatted.replace(' ', '').toLowerCase() } else { @@ -109,19 +112,22 @@ const factory = { * The preferred date and time format in the dashboard depends on the selected * interval and period. For example, in real-time view only the time is necessary, * while other intervals require dates to be displayed. + * @param {Object} config - Configuration object for determining formatter. * - * @param {string} interval - The interval of the query, e.g. `minute`, `hour` - * @param {boolean} longForm - Whether the formatted result should be in long or + * @param {string} config.interval - The interval of the query, e.g. `minute`, `hour` + * @param {boolean} config.longForm - Whether the formatted result should be in long or * short form. - * @param {string} period - The period of the query, e.g. `12mo`, `day` - * @param {boolean} isPeriodFull - Indicates whether the interval has been cut + * @param {string} config.period - The period of the query, e.g. `12mo`, `day` + * @param {boolean} config.isPeriodFull - Indicates whether the interval has been cut * off by the requested date range or not. If false, the returned formatted date * indicates this cut off, e.g. `Partial week of November 8`. + * @param {boolean} config.shouldShowYear - Should the year be appended to the date? + * Defaults to false. Rendering year string is a newer opt-in feature to be enabled where needed. */ -export default function dateFormatter(interval, longForm, period, isPeriodFull) { +export default function dateFormatter({ interval, longForm, period, isPeriodFull, shouldShowYear = false }) { const displayMode = longForm ? 'long' : 'short' - const options = { period: period, interval: interval, isBucketPartial: !isPeriodFull } - return function(isoDate, _index, _ticks) { + const options = { period: period, interval: interval, isBucketPartial: !isPeriodFull, shouldShowYear } + return function (isoDate, _index, _ticks) { return factory[interval][displayMode](isoDate, options) } } diff --git a/assets/js/dashboard/stats/graph/graph-util.js b/assets/js/dashboard/stats/graph/graph-util.js index da747f30b..e5d59934e 100644 --- a/assets/js/dashboard/stats/graph/graph-util.js +++ b/assets/js/dashboard/stats/graph/graph-util.js @@ -39,14 +39,18 @@ const renderBucketLabel = function(query, graphData, label, comparison = false) let isPeriodFull = graphData.full_intervals?.[label] if (comparison) isPeriodFull = true - const formattedLabel = dateFormatter(graphData.interval, true, query.period, isPeriodFull)(label) + const formattedLabel = dateFormatter({ + interval: graphData.interval, longForm: true, period: query.period, isPeriodFull, + })(label) if (query.period === 'realtime') { - return dateFormatter(graphData.interval, true, query.period)(label) + return dateFormatter({ + interval: graphData.interval, longForm: true, period: query.period, + })(label) } if (graphData.interval === 'hour' || graphData.interval == 'minute') { - const date = dateFormatter("date", true, query.period)(label) + const date = dateFormatter({ interval: "date", longForm: true, period: query.period })(label) return `${date}, ${formattedLabel}` } diff --git a/assets/js/dashboard/stats/graph/visitor-graph.js b/assets/js/dashboard/stats/graph/visitor-graph.js index 902b5ccb5..c445b8852 100644 --- a/assets/js/dashboard/stats/graph/visitor-graph.js +++ b/assets/js/dashboard/stats/graph/visitor-graph.js @@ -89,9 +89,29 @@ class LineGraph extends React.Component { ticks: { maxTicksLimit: 8, callback: function (val, _index, _ticks) { + // realtime graph labels are not date strings + const hasMultipleYears = typeof graphData.labels[0] !== 'string' ? false : + graphData.labels + // date format: 'yyyy-mm-dd'; maps to -> 'yyyy' + .map(date => date.split('-')[0]) + // reject any year that appears at a previous index, unique years only + .filter((value, index, list) => list.indexOf(value) === index) + .length > 1 + if (graphData.interval === 'hour' && query.period !== 'day') { - const date = dateFormatter("date", false, query.period)(this.getLabelForValue(val)) - const hour = dateFormatter(graphData.interval, false, query.period)(this.getLabelForValue(val)) + const date = dateFormatter({ + interval: "date", + longForm: false, + period: query.period, + shouldShowYear: hasMultipleYears, + })(this.getLabelForValue(val)) + + const hour = dateFormatter({ + interval: graphData.interval, + longForm: false, + period: query.period, + shouldShowYear: hasMultipleYears, + })(this.getLabelForValue(val)) // Returns a combination of date and hour. This is because // small intervals like hour may return multiple days @@ -100,10 +120,14 @@ class LineGraph extends React.Component { } if (graphData.interval === 'minute' && query.period !== 'realtime') { - return dateFormatter("hour", false, query.period)(this.getLabelForValue(val)) + return dateFormatter({ + interval: "hour", longForm: false, period: query.period, + })(this.getLabelForValue(val)) } - return dateFormatter(graphData.interval, false, query.period)(this.getLabelForValue(val)) + return dateFormatter({ + interval: graphData.interval, longForm: false, period: query.period, shouldShowYear: hasMultipleYears, + })(this.getLabelForValue(val)) }, color: this.props.darkTheme ? 'rgb(243, 244, 246)' : undefined }