Commit b5363b2c authored by tom goriunov's avatar tom goriunov Committed by GitHub

Better labels for graphs (#1348)

* demo for etc instance

* manage different ticks with the same label

* refactoring

* update screenshots
parent ec765edb
...@@ -51,8 +51,8 @@ frontend: ...@@ -51,8 +51,8 @@ frontend:
NEXT_PUBLIC_FEATURED_NETWORKS: https://raw.githubusercontent.com/blockscout/frontend-configs/dev/configs/featured-networks/eth-goerli.json NEXT_PUBLIC_FEATURED_NETWORKS: https://raw.githubusercontent.com/blockscout/frontend-configs/dev/configs/featured-networks/eth-goerli.json
NEXT_PUBLIC_NETWORK_LOGO: https://raw.githubusercontent.com/blockscout/frontend-configs/main/configs/network-logos/goerli.svg NEXT_PUBLIC_NETWORK_LOGO: https://raw.githubusercontent.com/blockscout/frontend-configs/main/configs/network-logos/goerli.svg
NEXT_PUBLIC_NETWORK_ICON: https://raw.githubusercontent.com/blockscout/frontend-configs/main/configs/network-icons/goerli.svg NEXT_PUBLIC_NETWORK_ICON: https://raw.githubusercontent.com/blockscout/frontend-configs/main/configs/network-icons/goerli.svg
NEXT_PUBLIC_API_HOST: blockscout-main.k8s-dev.blockscout.com NEXT_PUBLIC_API_HOST: etc.blockscout.com
NEXT_PUBLIC_STATS_API_HOST: https://stats-test.k8s-dev.blockscout.com/ NEXT_PUBLIC_STATS_API_HOST: https://stats-etc.k8s.blockscout.com/
NEXT_PUBLIC_VISUALIZE_API_HOST: http://visualizer-svc.visualizer-testing.svc.cluster.local/ NEXT_PUBLIC_VISUALIZE_API_HOST: http://visualizer-svc.visualizer-testing.svc.cluster.local/
NEXT_PUBLIC_CONTRACT_INFO_API_HOST: https://contracts-info-test.k8s-dev.blockscout.com NEXT_PUBLIC_CONTRACT_INFO_API_HOST: https://contracts-info-test.k8s-dev.blockscout.com
NEXT_PUBLIC_ADMIN_SERVICE_API_HOST: https://admin-rs-test.k8s-dev.blockscout.com NEXT_PUBLIC_ADMIN_SERVICE_API_HOST: https://admin-rs-test.k8s-dev.blockscout.com
......
...@@ -3,13 +3,11 @@ import React from 'react'; ...@@ -3,13 +3,11 @@ import React from 'react';
import type { TimeChartData } from 'ui/shared/chart/types'; import type { TimeChartData } from 'ui/shared/chart/types';
import useClientRect from 'lib/hooks/useClientRect';
import ChartArea from 'ui/shared/chart/ChartArea'; import ChartArea from 'ui/shared/chart/ChartArea';
import ChartLine from 'ui/shared/chart/ChartLine'; import ChartLine from 'ui/shared/chart/ChartLine';
import ChartOverlay from 'ui/shared/chart/ChartOverlay'; import ChartOverlay from 'ui/shared/chart/ChartOverlay';
import ChartTooltip from 'ui/shared/chart/ChartTooltip'; import ChartTooltip from 'ui/shared/chart/ChartTooltip';
import useTimeChartController from 'ui/shared/chart/useTimeChartController'; import useTimeChartController from 'ui/shared/chart/useTimeChartController';
import calculateInnerSize from 'ui/shared/chart/utils/calculateInnerSize';
interface Props { interface Props {
data: TimeChartData; data: TimeChartData;
...@@ -22,12 +20,17 @@ const ChainIndicatorChart = ({ data }: Props) => { ...@@ -22,12 +20,17 @@ const ChainIndicatorChart = ({ data }: Props) => {
const overlayRef = React.useRef<SVGRectElement>(null); const overlayRef = React.useRef<SVGRectElement>(null);
const lineColor = useToken('colors', 'blue.500'); const lineColor = useToken('colors', 'blue.500');
const [ rect, ref ] = useClientRect<SVGSVGElement>(); const axesConfig = React.useMemo(() => {
const { innerWidth, innerHeight } = calculateInnerSize(rect, CHART_MARGIN); return {
const { xScale, yScale } = useTimeChartController({ x: { ticks: 4 },
y: { ticks: 3, nice: true },
};
}, [ ]);
const { rect, ref, axis, innerWidth, innerHeight } = useTimeChartController({
data, data,
width: innerWidth, margin: CHART_MARGIN,
height: innerHeight, axesConfig,
}); });
return ( return (
...@@ -35,13 +38,13 @@ const ChainIndicatorChart = ({ data }: Props) => { ...@@ -35,13 +38,13 @@ const ChainIndicatorChart = ({ data }: Props) => {
<g transform={ `translate(${ CHART_MARGIN?.left || 0 },${ CHART_MARGIN?.top || 0 })` } opacity={ rect ? 1 : 0 }> <g transform={ `translate(${ CHART_MARGIN?.left || 0 },${ CHART_MARGIN?.top || 0 })` } opacity={ rect ? 1 : 0 }>
<ChartArea <ChartArea
data={ data[0].items } data={ data[0].items }
xScale={ xScale } xScale={ axis.x.scale }
yScale={ yScale } yScale={ axis.y.scale }
/> />
<ChartLine <ChartLine
data={ data[0].items } data={ data[0].items }
xScale={ xScale } xScale={ axis.x.scale }
yScale={ yScale } yScale={ axis.y.scale }
stroke={ lineColor } stroke={ lineColor }
animation="left" animation="left"
strokeWidth={ 3 } strokeWidth={ 3 }
...@@ -51,8 +54,8 @@ const ChainIndicatorChart = ({ data }: Props) => { ...@@ -51,8 +54,8 @@ const ChainIndicatorChart = ({ data }: Props) => {
anchorEl={ overlayRef.current } anchorEl={ overlayRef.current }
width={ innerWidth } width={ innerWidth }
height={ innerHeight } height={ innerHeight }
xScale={ xScale } xScale={ axis.x.scale }
yScale={ yScale } yScale={ axis.y.scale }
data={ data } data={ data }
/> />
</ChartOverlay> </ChartOverlay>
......
import { useToken } from '@chakra-ui/react'; import { useToken } from '@chakra-ui/react';
import * as d3 from 'd3'; import * as d3 from 'd3';
import React, { useEffect, useMemo } from 'react'; import React from 'react';
import type { ChartMargin, TimeChartItem } from 'ui/shared/chart/types'; import type { ChartMargin, TimeChartItem } from 'ui/shared/chart/types';
import dayjs from 'lib/date/dayjs'; import dayjs from 'lib/date/dayjs';
import useClientRect from 'lib/hooks/useClientRect';
import useIsMobile from 'lib/hooks/useIsMobile'; import useIsMobile from 'lib/hooks/useIsMobile';
import ChartArea from 'ui/shared/chart/ChartArea'; import ChartArea from 'ui/shared/chart/ChartArea';
import ChartAxis from 'ui/shared/chart/ChartAxis'; import ChartAxis from 'ui/shared/chart/ChartAxis';
...@@ -15,7 +14,6 @@ import ChartOverlay from 'ui/shared/chart/ChartOverlay'; ...@@ -15,7 +14,6 @@ import ChartOverlay from 'ui/shared/chart/ChartOverlay';
import ChartSelectionX from 'ui/shared/chart/ChartSelectionX'; import ChartSelectionX from 'ui/shared/chart/ChartSelectionX';
import ChartTooltip from 'ui/shared/chart/ChartTooltip'; import ChartTooltip from 'ui/shared/chart/ChartTooltip';
import useTimeChartController from 'ui/shared/chart/useTimeChartController'; import useTimeChartController from 'ui/shared/chart/useTimeChartController';
import calculateInnerSize from 'ui/shared/chart/utils/calculateInnerSize';
interface Props { interface Props {
isEnlarged?: boolean; isEnlarged?: boolean;
...@@ -31,36 +29,54 @@ interface Props { ...@@ -31,36 +29,54 @@ interface Props {
const MAX_SHOW_ITEMS = 100_000_000_000; const MAX_SHOW_ITEMS = 100_000_000_000;
const DEFAULT_CHART_MARGIN = { bottom: 20, left: 40, right: 20, top: 10 }; const DEFAULT_CHART_MARGIN = { bottom: 20, left: 40, right: 20, top: 10 };
const ChartWidgetGraph = ({ isEnlarged, items, onZoom, isZoomResetInitial, title, margin, units }: Props) => { const ChartWidgetGraph = ({ isEnlarged, items, onZoom, isZoomResetInitial, title, margin: marginProps, units }: Props) => {
const isMobile = useIsMobile(); const isMobile = useIsMobile();
const color = useToken('colors', 'blue.200'); const color = useToken('colors', 'blue.200');
const overlayRef = React.useRef<SVGRectElement>(null); const chartId = `chart-${ title.split(' ').join('') }-${ isEnlarged ? 'fullscreen' : 'small' }`;
const [ rect, ref ] = useClientRect<SVGSVGElement>(); const overlayRef = React.useRef<SVGRectElement>(null);
const chartMargin = { ...DEFAULT_CHART_MARGIN, ...margin };
const { innerWidth, innerHeight } = calculateInnerSize(rect, chartMargin);
const chartId = `chart-${ title.split(' ').join('') }-${ isEnlarged ? 'fullscreen' : 'small' }`;
const [ range, setRange ] = React.useState<[ Date, Date ]>([ items[0].date, items[items.length - 1].date ]); const [ range, setRange ] = React.useState<[ Date, Date ]>([ items[0].date, items[items.length - 1].date ]);
const rangedItems = useMemo(() => const rangedItems = React.useMemo(() =>
items.filter((item) => item.date >= range[0] && item.date <= range[1]), items.filter((item) => item.date >= range[0] && item.date <= range[1]),
[ items, range ]); [ items, range ]);
const isGroupedValues = rangedItems.length > MAX_SHOW_ITEMS; const isGroupedValues = rangedItems.length > MAX_SHOW_ITEMS;
const displayedData = useMemo(() => { const displayedData = React.useMemo(() => {
if (isGroupedValues) { if (isGroupedValues) {
return groupChartItemsByWeekNumber(rangedItems); return groupChartItemsByWeekNumber(rangedItems);
} else { } else {
return rangedItems; return rangedItems;
} }
}, [ isGroupedValues, rangedItems ]); }, [ isGroupedValues, rangedItems ]);
const chartData = React.useMemo(() => ([ { items: displayedData, name: 'Value', color, units } ]), [ color, displayedData, units ]); const chartData = React.useMemo(() => ([ { items: displayedData, name: 'Value', color, units } ]), [ color, displayedData, units ]);
const { xTickFormat, yTickFormat, xScale, yScale } = useTimeChartController({ const margin: ChartMargin = React.useMemo(() => ({ ...DEFAULT_CHART_MARGIN, ...marginProps }), [ marginProps ]);
data: [ { items: displayedData, name: title, color } ], const axesConfig = React.useMemo(() => {
width: innerWidth, return {
height: innerHeight, x: {
ticks: isEnlarged ? 8 : 4,
},
y: {
ticks: isEnlarged ? 6 : 3,
nice: true,
},
};
}, [ isEnlarged ]);
const {
ref,
rect,
innerWidth,
innerHeight,
chartMargin,
axis,
} = useTimeChartController({
data: chartData,
margin,
axesConfig,
}); });
const handleRangeSelect = React.useCallback((nextRange: [ Date, Date ]) => { const handleRangeSelect = React.useCallback((nextRange: [ Date, Date ]) => {
...@@ -68,7 +84,7 @@ const ChartWidgetGraph = ({ isEnlarged, items, onZoom, isZoomResetInitial, title ...@@ -68,7 +84,7 @@ const ChartWidgetGraph = ({ isEnlarged, items, onZoom, isZoomResetInitial, title
onZoom(); onZoom();
}, [ onZoom ]); }, [ onZoom ]);
useEffect(() => { React.useEffect(() => {
if (isZoomResetInitial) { if (isZoomResetInitial) {
setRange([ items[0].date, items[items.length - 1].date ]); setRange([ items[0].date, items[items.length - 1].date ]);
} }
...@@ -80,8 +96,8 @@ const ChartWidgetGraph = ({ isEnlarged, items, onZoom, isZoomResetInitial, title ...@@ -80,8 +96,8 @@ const ChartWidgetGraph = ({ isEnlarged, items, onZoom, isZoomResetInitial, title
<g transform={ `translate(${ chartMargin?.left || 0 },${ chartMargin?.top || 0 })` }> <g transform={ `translate(${ chartMargin?.left || 0 },${ chartMargin?.top || 0 })` }>
<ChartGridLine <ChartGridLine
type="horizontal" type="horizontal"
scale={ yScale } scale={ axis.y.scale }
ticks={ isEnlarged ? 6 : 3 } ticks={ axesConfig.y.ticks }
size={ innerWidth } size={ innerWidth }
disableAnimation disableAnimation
/> />
...@@ -90,14 +106,14 @@ const ChartWidgetGraph = ({ isEnlarged, items, onZoom, isZoomResetInitial, title ...@@ -90,14 +106,14 @@ const ChartWidgetGraph = ({ isEnlarged, items, onZoom, isZoomResetInitial, title
id={ chartId } id={ chartId }
data={ displayedData } data={ displayedData }
color={ color } color={ color }
xScale={ xScale } xScale={ axis.x.scale }
yScale={ yScale } yScale={ axis.y.scale }
/> />
<ChartLine <ChartLine
data={ displayedData } data={ displayedData }
xScale={ xScale } xScale={ axis.x.scale }
yScale={ yScale } yScale={ axis.y.scale }
stroke={ color } stroke={ color }
animation="none" animation="none"
strokeWidth={ isMobile ? 1 : 2 } strokeWidth={ isMobile ? 1 : 2 }
...@@ -105,19 +121,19 @@ const ChartWidgetGraph = ({ isEnlarged, items, onZoom, isZoomResetInitial, title ...@@ -105,19 +121,19 @@ const ChartWidgetGraph = ({ isEnlarged, items, onZoom, isZoomResetInitial, title
<ChartAxis <ChartAxis
type="left" type="left"
scale={ yScale } scale={ axis.y.scale }
ticks={ isEnlarged ? 6 : 3 } ticks={ axesConfig.y.ticks }
tickFormatGenerator={ yTickFormat } tickFormatGenerator={ axis.y.tickFormatter }
disableAnimation disableAnimation
/> />
<ChartAxis <ChartAxis
type="bottom" type="bottom"
scale={ xScale } scale={ axis.x.scale }
transform={ `translate(0, ${ innerHeight })` } transform={ `translate(0, ${ innerHeight })` }
ticks={ isMobile ? 1 : 4 } ticks={ axesConfig.x.ticks }
anchorEl={ overlayRef.current } anchorEl={ overlayRef.current }
tickFormatGenerator={ xTickFormat } tickFormatGenerator={ axis.x.tickFormatter }
disableAnimation disableAnimation
/> />
...@@ -127,15 +143,15 @@ const ChartWidgetGraph = ({ isEnlarged, items, onZoom, isZoomResetInitial, title ...@@ -127,15 +143,15 @@ const ChartWidgetGraph = ({ isEnlarged, items, onZoom, isZoomResetInitial, title
width={ innerWidth } width={ innerWidth }
tooltipWidth={ isGroupedValues ? 280 : 200 } tooltipWidth={ isGroupedValues ? 280 : 200 }
height={ innerHeight } height={ innerHeight }
xScale={ xScale } xScale={ axis.x.scale }
yScale={ yScale } yScale={ axis.y.scale }
data={ chartData } data={ chartData }
/> />
<ChartSelectionX <ChartSelectionX
anchorEl={ overlayRef.current } anchorEl={ overlayRef.current }
height={ innerHeight } height={ innerHeight }
scale={ xScale } scale={ axis.x.scale }
data={ chartData } data={ chartData }
onSelect={ handleRangeSelect } onSelect={ handleRangeSelect }
/> />
......
...@@ -25,3 +25,13 @@ export interface TimeChartDataItem { ...@@ -25,3 +25,13 @@ export interface TimeChartDataItem {
} }
export type TimeChartData = Array<TimeChartDataItem>; export type TimeChartData = Array<TimeChartDataItem>;
export interface AxisConfig {
ticks?: number;
nice?: boolean;
}
export interface AxesConfig {
x?: AxisConfig;
y?: AxisConfig;
}
import * as d3 from 'd3'; import React from 'react';
import { useMemo } from 'react';
import type { TimeChartData } from 'ui/shared/chart/types'; import type { AxesConfig, ChartMargin, TimeChartData } from 'ui/shared/chart/types';
import { WEEK, MONTH, YEAR } from 'lib/consts'; import useClientRect from 'lib/hooks/useClientRect';
import calculateInnerSize from './utils/calculateInnerSize';
import { getAxisParams, DEFAULT_MAXIMUM_SIGNIFICANT_DIGITS } from './utils/timeChartAxis';
interface Props { interface Props {
data: TimeChartData; data: TimeChartData;
width: number; margin?: ChartMargin;
height: number; axesConfig?: AxesConfig;
} }
export default function useTimeChartController({ data, width, height }: Props) { export default function useTimeChartController({ data, margin, axesConfig }: Props) {
const xMin = useMemo( const [ rect, ref ] = useClientRect<SVGSVGElement>();
() => d3.min(data, ({ items }) => d3.min(items, ({ date }) => date)) || new Date(),
[ data ], // we need to recalculate the axis scale whenever the rect width changes
); // eslint-disable-next-line react-hooks/exhaustive-deps
const axisParams = React.useMemo(() => getAxisParams(data, axesConfig), [ data, axesConfig, rect?.width ]);
const xMax = useMemo(
() => d3.max(data, ({ items }) => d3.max(items, ({ date }) => date)) || new Date(), const chartMargin = React.useMemo(() => {
[ data ], const exceedingDigits = (axisParams.y.labelFormatParams.maximumSignificantDigits ?? DEFAULT_MAXIMUM_SIGNIFICANT_DIGITS) -
); DEFAULT_MAXIMUM_SIGNIFICANT_DIGITS;
const PIXELS_PER_DIGIT = 7;
const xScale = useMemo( const leftShift = PIXELS_PER_DIGIT * exceedingDigits;
() => d3.scaleTime().domain([ xMin, xMax ]).range([ 0, width ]),
[ xMin, xMax, width ], return {
); ...margin,
left: (margin?.left ?? 0) + leftShift,
const yMin = useMemo( };
() => d3.min(data, ({ items }) => d3.min(items, ({ value }) => value)) || 0, }, [ axisParams.y.labelFormatParams.maximumSignificantDigits, margin ]);
[ data ],
); const { innerWidth, innerHeight } = calculateInnerSize(rect, chartMargin);
const yMax = useMemo( const xScale = React.useMemo(() => {
() => d3.max(data, ({ items }) => d3.max(items, ({ value }) => value)) || 0, return axisParams.x.scale.range([ 0, innerWidth ]);
[ data ], }, [ axisParams.x.scale, innerWidth ]);
);
const yScale = React.useMemo(() => {
const yScale = useMemo(() => { return axisParams.y.scale.range([ innerHeight, 0 ]);
const indention = (yMax - yMin) * 0.15; }, [ axisParams.y.scale, innerHeight ]);
return d3.scaleLinear() return React.useMemo(() => {
.domain([ yMin >= 0 && yMin - indention <= 0 ? 0 : yMin - indention, yMax + indention ]) return {
.range([ height, 0 ]); rect,
}, [ height, yMin, yMax ]); ref,
chartMargin,
const yScaleForAxis = useMemo( innerWidth,
() => d3.scaleBand().domain([ String(yMin), String(yMax) ]).range([ height, 0 ]), innerHeight,
[ height, yMin, yMax ], axis: {
); x: {
tickFormatter: axisParams.x.tickFormatter,
const xTickFormat = (axis: d3.Axis<d3.NumberValue>) => (d: d3.AxisDomain) => { scale: xScale,
let format: (date: Date) => string; },
const scale = axis.scale(); y: {
const extent = scale.domain(); tickFormatter: axisParams.y.tickFormatter,
scale: yScale,
const span = Number(extent[1]) - Number(extent[0]); },
},
if (span > YEAR) { };
format = d3.timeFormat('%Y'); }, [ axisParams.x.tickFormatter, axisParams.y.tickFormatter, chartMargin, innerHeight, innerWidth, rect, ref, xScale, yScale ]);
} else if (span > 2 * MONTH) {
format = d3.timeFormat('%b');
} else if (span > WEEK) {
format = d3.timeFormat('%b %d');
} else {
format = d3.timeFormat('%a %d');
}
return format(d as Date);
};
const yTickFormat = () => (d: d3.AxisDomain) => {
const num = Number(d);
const maximumFractionDigits = (() => {
if (num < 1) {
return 3;
}
if (num < 10) {
return 2;
}
if (num < 100) {
return 1;
}
return 0;
})();
return Number(d).toLocaleString(undefined, { maximumFractionDigits, notation: 'compact' });
};
return {
xTickFormat,
yTickFormat,
xScale,
yScale,
yScaleForAxis,
};
} }
import * as d3 from 'd3';
import _unique from 'lodash/uniq';
import type { AxesConfig, AxisConfig, TimeChartData } from '../types';
import { WEEK, MONTH, YEAR } from 'lib/consts';
export const DEFAULT_MAXIMUM_SIGNIFICANT_DIGITS = 2;
export const DEFAULT_MAXIMUM_FRACTION_DIGITS = 3;
export const MAXIMUM_SIGNIFICANT_DIGITS_LIMIT = 8;
export function getAxisParams(data: TimeChartData, axesConfig?: AxesConfig) {
const { labelFormatParams: labelFormatParamsY, scale: yScale } = getAxisParamsY(data, axesConfig?.y);
return {
x: {
scale: getAxisParamsX(data).scale,
tickFormatter: tickFormatterX,
},
y: {
scale: yScale,
labelFormatParams: labelFormatParamsY,
tickFormatter: getTickFormatterY(labelFormatParamsY),
},
};
}
function getAxisParamsX(data: TimeChartData) {
const min = d3.min(data, ({ items }) => d3.min(items, ({ date }) => date)) ?? new Date();
const max = d3.max(data, ({ items }) => d3.max(items, ({ date }) => date)) ?? new Date();
const scale = d3.scaleTime().domain([ min, max ]);
return { min, max, scale };
}
const tickFormatterX = (axis: d3.Axis<d3.NumberValue>) => (d: d3.AxisDomain) => {
let format: (date: Date) => string;
const scale = axis.scale();
const extent = scale.domain();
const span = Number(extent[1]) - Number(extent[0]);
if (span > YEAR) {
format = d3.timeFormat('%Y');
} else if (span > 2 * MONTH) {
format = d3.timeFormat('%b');
} else if (span > WEEK) {
format = d3.timeFormat('%b %d');
} else {
format = d3.timeFormat('%a %d');
}
return format(d as Date);
};
function getAxisParamsY(data: TimeChartData, config?: AxisConfig) {
const DEFAULT_TICKS_NUM = 3;
const min = d3.min(data, ({ items }) => d3.min(items, ({ value }) => value)) ?? 0;
const max = d3.max(data, ({ items }) => d3.max(items, ({ value }) => value)) ?? 0;
const scale = config?.nice ?
d3.scaleLinear()
.domain([ min, max ])
.nice(config?.ticks ?? DEFAULT_TICKS_NUM) :
d3.scaleLinear()
.domain([ min, max ]);
const ticks = scale.ticks(config?.ticks ?? DEFAULT_TICKS_NUM);
const labelFormatParams = getYLabelFormatParams(ticks);
return { min, max, scale, labelFormatParams };
}
const getTickFormatterY = (params: Intl.NumberFormatOptions) => () => (d: d3.AxisDomain) => {
const num = Number(d);
if (num < 1) {
// for small number there are no algorithm to format label right now
// so we set it to 3 digits after dot maximum
return num.toLocaleString(undefined, { maximumFractionDigits: 3 });
}
return num.toLocaleString(undefined, params);
};
function getYLabelFormatParams(ticks: Array<number>, maximumSignificantDigits = DEFAULT_MAXIMUM_SIGNIFICANT_DIGITS): Intl.NumberFormatOptions {
const params = {
maximumFractionDigits: 3,
maximumSignificantDigits,
notation: 'compact' as const,
};
const uniqTicksStr = _unique(ticks.map((tick) => tick.toLocaleString(undefined, params)));
if (uniqTicksStr.length === ticks.length || maximumSignificantDigits === MAXIMUM_SIGNIFICANT_DIGITS_LIMIT) {
return params;
}
return getYLabelFormatParams(ticks, maximumSignificantDigits + 1);
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment