Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Canvas] XY. Step 1. Remove all specific logic, related to aggType. #111876

Merged
merged 33 commits into from
Sep 22, 2021
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
9dfab8a
Added comments for places I wished to refactor.
Kuznietsov Aug 12, 2021
76b1f5d
Merge branch 'master' into xy_chart
Kuznietsov Aug 23, 2021
8a6b502
Fixed comments.
Kuznietsov Aug 23, 2021
f74fc28
Removed logic, related to BUCKET_TYPES.
Kuznietsov Sep 1, 2021
87fd22f
removed aggType connected code.
Kuznietsov Sep 1, 2021
df2e3db
Added `id` to `xy_dimension` to avoid direct comparison with aggId
Kuznietsov Sep 2, 2021
6093e96
Removed all checks of seriesParams at chart.
Kuznietsov Sep 2, 2021
80eb59f
removed aggId and aggType from chart
Kuznietsov Sep 2, 2021
74867c6
Merge branch 'master' into xy_chart
kibanamachine Sep 2, 2021
807a8eb
removed all aggId/aggTypes from tests/mocks.
Kuznietsov Sep 2, 2021
c803841
Fixed comment.
Kuznietsov Sep 2, 2021
ce5c1d4
moved `get_agg_id.ts` util to the vislib
Kuznietsov Sep 2, 2021
8276a66
clearified the code, related to isSimpleField logic.
Kuznietsov Sep 2, 2021
ef3dcef
added comment.
Kuznietsov Sep 2, 2021
9909715
Fixed error at percentile agg.
Kuznietsov Sep 2, 2021
4b34965
Fixed render_all_series failure of tests.
Kuznietsov Sep 2, 2021
7dd186a
Added tests for new behavior.
Kuznietsov Sep 2, 2021
a325fe4
changed the way of handling `enableHistogramMode`.
Kuznietsov Sep 3, 2021
6a70424
updated snapshots.
Kuznietsov Sep 3, 2021
defbf4f
Merge branch 'master' into xy_chart
Kuznietsov Sep 3, 2021
4fe713e
remove not used lib.
Kuznietsov Sep 3, 2021
33dc89f
Merge branch 'presentation/feature-xy' into xy_chart
kibanamachine Sep 10, 2021
aa6e158
Merge branch 'presentation/feature-xy' into xy_chart
kibanamachine Sep 10, 2021
84fe9a9
Merge branch 'presentation/feature-xy' into xy_chart
kibanamachine Sep 10, 2021
fa9bc9d
Merge branch 'master' into xy_chart
Kuznietsov Sep 13, 2021
308a84e
Merge branch 'master' into xy_chart
kibanamachine Sep 13, 2021
e0aca67
Merge branch 'master' into xy_chart
kibanamachine Sep 13, 2021
bbe4077
Merge branch 'master' into xy_chart
kibanamachine Sep 14, 2021
7714247
Fixed shard_delay test.
Kuznietsov Sep 14, 2021
9d06227
Merge branch 'presentation/feature-xy' into xy_chart
Kuznietsov Sep 20, 2021
2390697
removed all todos from xy.
Kuznietsov Sep 21, 2021
d00c6a9
removed latest todos.
Kuznietsov Sep 21, 2021
884bc38
Changed the type of tickFormatter.
Kuznietsov Sep 21, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { getAggId } from '../../../../../xy/public';
import type { Dimension } from '../../../../../xy/public';

import { getAggId } from './get_agg_id';
import { Point } from './_get_point';

export interface Serie {
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/vis_types/xy/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"version": "kibana",
"ui": true,
"server": false,
"requiredPlugins": ["charts", "data", "expressions", "visualizations", "usageCollection"],
"requiredPlugins": ["charts", "data", "expressions", "visualizations", "usageCollection", "fieldFormats"],
"requiredBundles": ["kibanaUtils", "visDefaultEditor"],
"extraPublicDirs": ["common/index"],
"owner": {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ export const aspects = {
pattern: 'YYYY-MM-DD HH:mm',
},
},
aggType: 'date_histogram',
aggId: '3',
id: '3',
params: {
date: true,
intervalESUnit: 'h',
Expand All @@ -35,8 +34,7 @@ export const aspects = {
format: {
id: 'number',
},
aggType: 'count',
aggId: '1',
id: '1',
params: {},
},
],
Expand All @@ -53,8 +51,7 @@ export const aspectsWithSplitColumn = {
pattern: 'YYYY-MM-DD HH:mm',
},
},
aggType: 'date_histogram',
aggId: '3',
id: '3',
params: {
date: true,
intervalESUnit: 'h',
Expand All @@ -71,8 +68,7 @@ export const aspectsWithSplitColumn = {
format: {
id: 'number',
},
aggType: 'count',
aggId: '1',
id: '1',
params: {},
},
],
Expand All @@ -88,8 +84,7 @@ export const aspectsWithSplitColumn = {
missingBucketLabel: 'Missing',
},
},
aggType: 'terms',
aggId: '4',
id: '4',
params: {},
},
};
Expand All @@ -105,8 +100,7 @@ export const aspectsWithSplitRow = {
pattern: 'YYYY-MM-DD HH:mm',
},
},
aggType: 'date_histogram',
aggId: '3',
id: '3',
params: {
date: true,
intervalESUnit: 'h',
Expand All @@ -123,8 +117,7 @@ export const aspectsWithSplitRow = {
format: {
id: 'number',
},
aggType: 'count',
aggId: '1',
id: '1',
params: {},
},
],
Expand All @@ -140,8 +133,7 @@ export const aspectsWithSplitRow = {
missingBucketLabel: 'Missing',
},
},
aggType: 'terms',
aggId: '4',
id: '4',
params: {},
},
};
Expand Down
14 changes: 9 additions & 5 deletions src/plugins/vis_types/xy/public/components/detailed_tooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@ import {
} from '@elastic/charts';

import { Aspects } from '../types';
import {
applyFormatterIfSimpleField,
applyFormatter,
COMPLEX_SPLIT_ACCESSOR,
} from '../utils/accessors';

import './_detailed_tooltip.scss';
import { COMPLEX_SPLIT_ACCESSOR, isRangeAggType } from '../utils/accessors';

interface TooltipData {
label: string;
Expand All @@ -34,10 +38,10 @@ export const getTooltipData = (
const data: TooltipData[] = [];

if (header) {
const xFormatter = isRangeAggType(aspects.x.aggType) ? null : aspects.x.formatter;
data.push({
label: aspects.x.title,
value: xFormatter ? xFormatter(header.value) : `${header.value}`,
// already formatted while executing accessor on such a complex field type as `*_range`
value: `${applyFormatterIfSimpleField(aspects.x, header.value)}`,
});
}

Expand All @@ -47,14 +51,14 @@ export const getTooltipData = (
if (yAccessor) {
data.push({
label: yAccessor.title,
value: yAccessor.formatter ? yAccessor.formatter(value.value) : `${value.value}`,
value: `${applyFormatter(yAccessor, value.value)}`,
});
}

if (aspects.z && !isNil(value.markValue)) {
data.push({
label: aspects.z.title,
value: aspects.z.formatter ? aspects.z.formatter(value.markValue) : `${value.markValue}`,
value: `${applyFormatterIfSimpleField(aspects.z, value.markValue)}`,
});
}

Expand Down
8 changes: 2 additions & 6 deletions src/plugins/vis_types/xy/public/config/get_aspects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,10 @@ import { DatatableColumn } from '../../../../expressions/public';

import { Aspect, Dimension, Aspects, Dimensions } from '../types';
import { getFormatService } from '../services';
import { getAggId } from './get_agg_id';

export function getEmptyAspect(): Aspect {
return {
accessor: null,
aggId: null,
aggType: null,
title: i18n.translate('visTypeXy.aggResponse.allDocsTitle', {
defaultMessage: 'All docs',
}),
Expand Down Expand Up @@ -76,14 +73,13 @@ function getAspectsFromDimension(

const getAspect = (
{ id: accessor, name: title }: DatatableColumn,
{ accessor: column, format, params, aggType }: Dimension
{ accessor: column, format, params, id }: Dimension
): Aspect => ({
accessor,
column,
title,
format,
aggType,
aggId: getAggId(accessor),
formatter: (value: any) => getFormatService().deserialize(format).convert(value),
params,
id,
});
22 changes: 11 additions & 11 deletions src/plugins/vis_types/xy/public/config/get_axis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,9 @@
* Side Public License, v 1.
*/

import { identity, isNil } from 'lodash';

import { isNil } from 'lodash';
import { AxisSpec, TickFormatter, YDomainRange, ScaleType as ECScaleType } from '@elastic/charts';

import { LabelRotation } from '../../../../charts/public';
import { BUCKET_TYPES } from '../../../../data/public';

import {
Aspect,
CategoryAxis,
Expand All @@ -27,13 +23,15 @@ import {
YScaleType,
SeriesParam,
} from '../types';
import { isSimpleField } from '../utils/accessors';

export function getAxis<S extends XScaleType | YScaleType>(
{ type, title: axisTitle, labels, scale: axisScale, ...axis }: CategoryAxis,
{ categoryLines, valueAxis }: Grid,
{ params, format, formatter, title: fallbackTitle = '', aggType }: Aspect,
{ params, format, formatter, title: fallbackTitle = '', accessor }: Aspect,
seriesParams: SeriesParam[],
isDateHistogram = false
isDateHistogram = false,
shouldApplyFormatter = false
): AxisConfig<S> {
const isCategoryAxis = type === AxisType.Category;
// Hide unassigned axis, not supported in elastic charts
Expand All @@ -50,9 +48,10 @@ export function getAxis<S extends XScaleType | YScaleType>(
: {
show: valueAxis === axis.id,
};
// Date range formatter applied on xAccessor
const tickFormatter =
aggType === BUCKET_TYPES.DATE_RANGE || aggType === BUCKET_TYPES.RANGE ? identity : formatter;

const tickFormatter = (v: any) =>
Copy link
Contributor

@stratoula stratoula Sep 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checking here. Can we omit using any here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done)

isSimpleField(format) || shouldApplyFormatter ? formatter?.(v) ?? v : v;

const ticks: TickOptions = {
formatter: tickFormatter,
labelFormatter: getLabelFormatter(labels.truncate, tickFormatter),
Expand All @@ -61,6 +60,7 @@ export function getAxis<S extends XScaleType | YScaleType>(
showOverlappingLabels: !labels.filter,
showDuplicates: !labels.filter,
};

const scale = getScale<S>(axisScale, params, format, isCategoryAxis);
const title = axisTitle.text || fallbackTitle;
const fallbackRotation =
Expand All @@ -76,7 +76,7 @@ export function getAxis<S extends XScaleType | YScaleType>(
scale,
style: getAxisStyle(ticks, title, fallbackRotation),
domain: getAxisDomain(scale, isCategoryAxis),
integersOnly: aggType === 'count',
integersOnly: params?.integersOnly ?? false,
};
}

Expand Down
39 changes: 20 additions & 19 deletions src/plugins/vis_types/xy/public/config/get_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@
import { ScaleContinuousType } from '@elastic/charts';

import { Datatable } from '../../../../expressions/public';
import { BUCKET_TYPES } from '../../../../data/public';
import { DateHistogramParams } from '../../../../visualizations/public';

import { KBN_FIELD_TYPES } from '../../../../data/public';
import {
Aspect,
AxisConfig,
AxisMode,
ChartMode,
SeriesParam,
VisConfig,
VisParams,
Expand Down Expand Up @@ -42,23 +42,28 @@ export function getConfig(table: Datatable, params: VisParams): VisConfig {
fillOpacity,
} = params;
const aspects = getAspects(table.columns, params.dimensions);
const yAxes = params.valueAxes.map((a) =>
// shouldApplyFormatter = true, because no formatter was applied to this axis values before
// and will be not applied in the future
getAxis<YScaleType>(a, params.grid, aspects.y[0], params.seriesParams, false, true)
);

const enableHistogramMode =
(params.enableHistogramMode ?? false) &&
shouldEnableHistogramMode(params.seriesParams, aspects.y, yAxes);

const timeChartFieldTypes: string[] = [KBN_FIELD_TYPES.DATE, KBN_FIELD_TYPES.DATE_RANGE];
const isTimeChart = timeChartFieldTypes.includes(aspects.x.format?.id ?? '');

const xAxis = getAxis<XScaleType>(
params.categoryAxes[0],
params.grid,
aspects.x,
params.seriesParams,
params.dimensions.x?.aggType === BUCKET_TYPES.DATE_HISTOGRAM
aspects.x.format?.id === KBN_FIELD_TYPES.DATE
);

const tooltip = getTooltip(aspects, params);
const yAxes = params.valueAxes.map((a) =>
// uses first y aspect in array for formatting axis
getAxis<YScaleType>(a, params.grid, aspects.y[0], params.seriesParams)
);
const enableHistogramMode =
(params.dimensions.x?.aggType === BUCKET_TYPES.DATE_HISTOGRAM ||
params.dimensions.x?.aggType === BUCKET_TYPES.HISTOGRAM) &&
shouldEnableHistogramMode(params.seriesParams, aspects.y, yAxes);
const isTimeChart = (aspects.x.params as DateHistogramParams).date ?? false;

return {
// NOTE: downscale ratio to match current vislib implementation
Expand Down Expand Up @@ -94,11 +99,7 @@ const shouldEnableHistogramMode = (
yAspects: Aspect[],
yAxes: Array<AxisConfig<ScaleContinuousType>>
): boolean => {
const bars = seriesParams.filter(({ type, data: { id: paramId } }) => {
return (
type === ChartType.Histogram && yAspects.find(({ aggId }) => aggId === paramId) !== undefined
);
});
const bars = seriesParams.filter(({ type }) => type === ChartType.Histogram);

const groupIds = [
...bars.reduce<Set<string>>((acc, { valueAxis: groupId, mode }) => {
Expand All @@ -114,6 +115,6 @@ const shouldEnableHistogramMode = (
return bars.every(({ valueAxis: groupId, mode }) => {
const yAxisScale = yAxes.find(({ groupId: axisGroupId }) => axisGroupId === groupId)?.scale;

return mode === 'stacked' || yAxisScale?.mode === 'percentage';
return mode === ChartMode.Stacked || yAxisScale?.mode === AxisMode.Percentage;
});
};
1 change: 0 additions & 1 deletion src/plugins/vis_types/xy/public/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,3 @@
*/

export { getConfig } from './get_config';
export { getAggId } from './get_agg_id';
Loading