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

[XY] Migrate vis type xy to new unified xy expression #136475

Merged
merged 38 commits into from
Aug 11, 2022
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
6d1ba64
Migrate vis type xy to new unified xy expression
VladLasitsa Jul 15, 2022
ce23d34
Add legend toggle and color picker. Some fixes
VladLasitsa Jul 15, 2022
b5d27c8
Fix snapshots
VladLasitsa Jul 15, 2022
c89335d
Fix tests
VladLasitsa Jul 18, 2022
1aa9577
Fix some tests
VladLasitsa Jul 18, 2022
dfc353d
Fix snapshots
VladLasitsa Jul 18, 2022
5b039ac
Fix tests
VladLasitsa Jul 19, 2022
c66c4ac
Merge branch 'main' into vis_type_xy_to_new_expression
flash1293 Jul 19, 2022
e533a2c
Fix some tests
VladLasitsa Jul 19, 2022
ebfdd1c
Fix some tests
VladLasitsa Jul 20, 2022
2d46b7e
Fix some more tests
VladLasitsa Jul 21, 2022
61d5149
Update snapshot for area chart
VladLasitsa Jul 21, 2022
f1829af
Merge branch 'main' into vis_type_xy_to_new_expression
kibanamachine Jul 21, 2022
1ad23d9
Fix dashboards tests
VladLasitsa Jul 22, 2022
61f9418
Fix test
VladLasitsa Jul 22, 2022
f0ced01
Merge remote-tracking branch 'upstream/main' into vis_type_xy_to_new_…
VladLasitsa Jul 25, 2022
57abcc4
Merge remote-tracking branch 'upstream/main' into vis_type_xy_to_new_…
VladLasitsa Jul 25, 2022
2c38d2c
Fix some remarks
VladLasitsa Jul 25, 2022
d8067fd
Fix tests
VladLasitsa Jul 25, 2022
8da7181
Fix test
VladLasitsa Jul 26, 2022
72d8c03
Merge remote-tracking branch 'upstream/main' into vis_type_xy_to_new_…
VladLasitsa Jul 26, 2022
a85f845
Remove useAdjustedInterval arg
VladLasitsa Jul 26, 2022
7a6faa4
Fix remarks
VladLasitsa Jul 27, 2022
526bb0e
Fix CI checks
VladLasitsa Jul 27, 2022
06d3ccd
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine Jul 27, 2022
d20b31f
Fix CI
VladLasitsa Jul 28, 2022
159c923
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Jul 28, 2022
fb63186
Merge branch 'main' into vis_type_xy_to_new_expression
kibanamachine Jul 28, 2022
dd78ebd
Merge branch 'main' into vis_type_xy_to_new_expression
flash1293 Aug 2, 2022
8a756f8
Fix all remarks
VladLasitsa Aug 3, 2022
4b4544d
Remove unused code
VladLasitsa Aug 3, 2022
bddd5aa
Merge branch 'main' into vis_type_xy_to_new_expression
kibanamachine Aug 3, 2022
1a1249b
Merge remote-tracking branch 'upstream/main' into vis_type_xy_to_new_…
VladLasitsa Aug 4, 2022
4a5d053
Merge branch 'main' into vis_type_xy_to_new_expression
kibanamachine Aug 8, 2022
99ae23f
Fix Percentile aggragtion
VladLasitsa Aug 9, 2022
c44b153
Merge branch 'main' into vis_type_xy_to_new_expression
stratoula Aug 10, 2022
f0e1bf2
Fix problems with several series
VladLasitsa Aug 10, 2022
32b29f5
Fix problems with hidden series
VladLasitsa Aug 10, 2022
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
2 changes: 1 addition & 1 deletion packages/kbn-optimizer/limits.yml
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,6 @@ pageLoadAssetSize:
eventAnnotation: 19334
screenshotting: 22870
synthetics: 40958
expressionXY: 34000
expressionXY: 35000
kibanaUsageCollection: 16463
kubernetesSecurity: 77234
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ export const FittingFunctions = {
LINEAR: 'Linear',
CARRY: 'Carry',
LOOKAHEAD: 'Lookahead',
AVERAGE: 'Average',
NEAREST: 'Nearest',
} as const;

export const EndValues = {
Expand Down Expand Up @@ -98,6 +100,7 @@ export const XScaleTypes = {
export const XYCurveTypes = {
LINEAR: 'LINEAR',
CURVE_MONOTONE_X: 'CURVE_MONOTONE_X',
CURVE_STEP_AFTER: 'CURVE_STEP_AFTER',
} as const;

export const ValueLabelModes = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import { ArgumentType } from '@kbn/expressions-plugin/common';
import { SeriesTypes, XScaleTypes, DATA_DECORATION_CONFIG } from '../constants';
import { SeriesTypes, XScaleTypes, DATA_DECORATION_CONFIG, XYCurveTypes } from '../constants';
import { strings } from '../i18n';
import { DataLayerArgs, ExtendedDataLayerArgs } from '../types';

Expand Down Expand Up @@ -58,6 +58,12 @@ export const commonDataLayerArgs: Omit<
default: false,
help: strings.getIsHorizontalHelp(),
},
curveType: {
types: ['string'],
options: [...Object.values(XYCurveTypes)],
help: strings.getCurveTypeHelp(),
strict: true,
},
lineWidth: {
types: ['number'],
help: strings.getLineWidthHelp(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
FittingFunctions,
LEGEND_CONFIG,
ValueLabelModes,
XYCurveTypes,
X_AXIS_CONFIG,
Y_AXIS_CONFIG,
} from '../constants';
Expand Down Expand Up @@ -50,12 +49,6 @@ export const commonXYArgs: CommonXYFn['args'] = {
strict: true,
default: ValueLabelModes.HIDE,
},
curveType: {
types: ['string'],
options: [...Object.values(XYCurveTypes)],
help: strings.getCurveTypeHelp(),
strict: true,
},
fillOpacity: {
types: ['number'],
help: strings.getFillOpacityHelp(),
Expand Down Expand Up @@ -110,4 +103,14 @@ export const commonXYArgs: CommonXYFn['args'] = {
types: ['string'],
help: strings.getMinTimeBarIntervalHelp(),
},
handleEmptyXAccessor: {
types: ['boolean'],
default: false,
help: strings.getHandleEmptyXAccessorHelp(),
},
useAdjustedInterval: {
types: ['boolean'],
default: false,
help: strings.getUseAdjustedIntervalHelp(),
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -20,27 +20,23 @@ export const extendedDataLayerFunction: ExtendedDataLayerFn = {
args: {
...commonDataLayerArgs,
xAccessor: {
types: ['string'],
types: ['vis_dimension', 'string'],
help: strings.getXAccessorHelp(),
},
splitAccessors: {
types: ['string'],
types: ['vis_dimension', 'string'],
help: strings.getSplitAccessorHelp(),
multi: true,
},
accessors: {
types: ['string'],
types: ['vis_dimension', 'string'],
help: strings.getAccessorsHelp(),
multi: true,
},
markSizeAccessor: {
types: ['string'],
types: ['vis_dimension', 'string'],
help: strings.getMarkSizeAccessorHelp(),
},
table: {
types: ['datatable'],
help: strings.getTableHelp(),
},
layerId: {
types: ['string'],
help: strings.getLayerIdHelp(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Side Public License, v 1.
*/

import { ExpressionValueVisDimension } from '@kbn/visualizations-plugin/common';
import { validateAccessor } from '@kbn/visualizations-plugin/common/utils';
import { ExtendedDataLayerArgs, ExtendedDataLayerFn } from '../types';
import { EXTENDED_DATA_LAYER, LayerTypes } from '../constants';
Expand All @@ -19,8 +20,11 @@ import {
} from './validate';

export const extendedDataLayerFn: ExtendedDataLayerFn['fn'] = async (data, args, context) => {
const table = args.table ?? data;
const accessors = getAccessors<string, ExtendedDataLayerArgs>(args, table);
const table = data;
const accessors = getAccessors<string | ExpressionValueVisDimension, ExtendedDataLayerArgs>(
args,
table
);

validateAccessor(accessors.xAccessor, table.columns);
accessors.splitAccessors?.forEach((accessor) => validateAccessor(accessor, table.columns));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
REFERENCE_LINE_LAYER,
LAYERED_XY_VIS,
EXTENDED_ANNOTATION_LAYER,
REFERENCE_LINE,
} from '../constants';
import { commonXYArgs } from './common_xy_args';
import { strings } from '../i18n';
Expand All @@ -25,12 +26,20 @@ export const layeredXyVisFunction: LayeredXyVisFn = {
args: {
...commonXYArgs,
layers: {
types: [EXTENDED_DATA_LAYER, REFERENCE_LINE_LAYER, EXTENDED_ANNOTATION_LAYER],
types: [EXTENDED_DATA_LAYER, REFERENCE_LINE_LAYER, EXTENDED_ANNOTATION_LAYER, REFERENCE_LINE],
help: i18n.translate('expressionXY.layeredXyVis.layers.help', {
defaultMessage: 'Layers of visual series',
}),
multi: true,
},
splitColumnAccessor: {
types: ['vis_dimension', 'string'],
help: strings.getSplitColumnAccessorHelp(),
},
splitRowAccessor: {
types: ['vis_dimension', 'string'],
help: strings.getSplitRowAccessorHelp(),
},
},
async fn(data, args, handlers) {
const { layeredXyVisFn } = await import('./layered_xy_vis_fn');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import { XY_VIS_RENDERER } from '../constants';
import { LayeredXyVisFn } from '../types';
import { logDatatables } from '../utils';
import { logDatatables, logDatatable } from '../utils';
import {
validateMarkSizeRatioLimits,
validateAddTimeMarker,
Expand All @@ -22,7 +22,12 @@ import { appendLayerIds, getDataLayers } from '../helpers';
export const layeredXyVisFn: LayeredXyVisFn['fn'] = async (data, args, handlers) => {
const layers = appendLayerIds(args.layers ?? [], 'layers');

logDatatables(layers, handlers);
// for visialize we should log one datable for all layers
if (handlers.getExecutionContext()?.name === 'visualize') {
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid checking for visualize here, let's instead check whether the tables are the same:

const sameTable = layers.every(l => l.table === layers[0].table)

Copy link
Contributor Author

@VladLasitsa VladLasitsa Jul 27, 2022

Choose a reason for hiding this comment

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

@flash1293 Unless in Lens there can be a situation that there will be an identical table in two layers?

Copy link
Contributor

Choose a reason for hiding this comment

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

@VladLasitsa This can't happen in Lens - layers always have their separate data fetching

Copy link
Contributor

Choose a reason for hiding this comment

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

And even if it would happen IMHO it makes sense to report it as a single table in the inspector if it is virtually the same object.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@flash1293 I have checked this more deeply and found out that we should use my approach because for lens we should log datatable using layer id as key, but when we log only one datatable we use "default" as key. It leads to some problems with getting correct datatable from inspector which we use after as active data in many places.

logDatatable(data, layers, handlers, args.splitColumnAccessor, args.splitRowAccessor);
} else {
logDatatables(layers, handlers, args.splitColumnAccessor, args.splitRowAccessor);
}

const dataLayers = getDataLayers(layers);
const hasBar = hasBarLayer(dataLayers);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,12 @@
* Side Public License, v 1.
*/

import {
Dimension,
prepareLogTable,
validateAccessor,
} from '@kbn/visualizations-plugin/common/utils';
import { validateAccessor } from '@kbn/visualizations-plugin/common/utils';
import type { Datatable } from '@kbn/expressions-plugin/common';
import { ExpressionValueVisDimension } from '@kbn/visualizations-plugin/common/expression_functions';
import { LayerTypes, XY_VIS_RENDERER, DATA_LAYER, REFERENCE_LINE } from '../constants';
import { LayerTypes, XY_VIS_RENDERER, DATA_LAYER } from '../constants';
import { appendLayerIds, getAccessors, getShowLines, normalizeTable } from '../helpers';
import { DataLayerConfigResult, XYLayerConfig, XyVisFn, XYArgs } from '../types';
import { getLayerDimensions } from '../utils';
import {
hasAreaLayer,
hasBarLayer,
Expand All @@ -35,6 +30,7 @@ import {
validateLinesVisibilityForChartType,
validateAxes,
} from './validate';
import { logDatatable } from '../utils';

const createDataLayer = (args: XYArgs, table: Datatable): DataLayerConfigResult => {
const accessors = getAccessors<string | ExpressionValueVisDimension, XYArgs>(args, table);
Expand Down Expand Up @@ -108,21 +104,7 @@ export const xyVisFn: XyVisFn['fn'] = async (data, args, handlers) => {
...appendLayerIds(annotationLayers, 'annotationLayers'),
];

if (handlers.inspectorAdapters.tables) {
handlers.inspectorAdapters.tables.reset();
handlers.inspectorAdapters.tables.allowCsvExport = true;

const layerDimensions = layers.reduce<Dimension[]>((dimensions, layer) => {
if (layer.layerType === LayerTypes.ANNOTATIONS || layer.type === REFERENCE_LINE) {
return dimensions;
}

return [...dimensions, ...getLayerDimensions(layer)];
}, []);

const logTable = prepareLogTable(data, layerDimensions, true);
handlers.inspectorAdapters.tables.logDatatable('default', logTable);
}
logDatatable(data, layers, handlers, args.splitColumnAccessor, args.splitRowAccessor);

const hasBar = hasBarLayer(dataLayers);
const hasArea = hasAreaLayer(dataLayers);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,18 @@ export const strings = {
i18n.translate('expressionXY.xyVis.logDatatable.breakDown', {
defaultMessage: 'Break down by',
}),
getSplitRowHelp: () =>
i18n.translate('expressionXY.xyVis.logDatatable.splitRow', {
defaultMessage: 'Split rows by',
}),
getSplitColumnHelp: () =>
i18n.translate('expressionXY.xyVis.logDatatable.splitColumn', {
defaultMessage: 'Split columns by',
}),
getMarkSizeHelp: () =>
i18n.translate('expressionXY.xyVis.logDatatable.markSize', {
defaultMessage: 'Mark size',
}),
getReferenceLineHelp: () =>
i18n.translate('expressionXY.xyVis.logDatatable.breakDown', {
defaultMessage: 'Break down by',
Expand Down Expand Up @@ -117,6 +129,14 @@ export const strings = {
i18n.translate('expressionXY.xyVis.splitRowAccessor.help', {
defaultMessage: 'Specifies split row of the xy chart',
}),
getHandleEmptyXAccessorHelp: () =>
i18n.translate('expressionXY.xyVis.handleEmptyXAccessor.help', {
defaultMessage: 'Allow handling empty x accessor',
}),
getUseAdjustedIntervalHelp: () =>
i18n.translate('expressionXY.xyVis.useAdjustedInterval.help', {
defaultMessage: 'Use adjusted interval fox x domain',
}),
getLayersHelp: () =>
i18n.translate('expressionXY.layeredXyVis.layers.help', {
defaultMessage: 'Layers of visual series',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ export interface DataLayerArgs {
isHorizontal: boolean;
palette: PaletteOutput;
decorations?: DataDecorationConfigResult[];
curveType?: XYCurveType;
}

export interface ValidLayer extends DataLayerConfigResult {
Expand All @@ -138,12 +139,12 @@ export interface ValidLayer extends DataLayerConfigResult {

export interface ExtendedDataLayerArgs {
layerId?: string;
accessors: string[];
accessors: Array<ExpressionValueVisDimension | string>;
seriesType: SeriesType;
xAccessor?: string;
xAccessor?: string | ExpressionValueVisDimension;
simpleView?: boolean;
splitAccessors?: string[];
markSizeAccessor?: string;
splitAccessors?: Array<ExpressionValueVisDimension | string>;
markSizeAccessor?: string | ExpressionValueVisDimension;
lineWidth?: number;
showPoints?: boolean;
showLines?: boolean;
Expand All @@ -157,7 +158,7 @@ export interface ExtendedDataLayerArgs {
palette: PaletteOutput;
// palette will always be set on the expression
decorations?: DataDecorationConfigResult[];
table?: Datatable;
curveType?: XYCurveType;
}

export interface LegendConfig {
Expand Down Expand Up @@ -215,7 +216,6 @@ export interface XYArgs extends DataLayerArgs {
referenceLines: ReferenceLineConfigResult[];
annotationLayers: AnnotationLayerConfigResult[];
fittingFunction?: FittingFunction;
curveType?: XYCurveType;
fillOpacity?: number;
hideEndzones?: boolean;
valuesInLegend?: boolean;
Expand All @@ -230,6 +230,8 @@ export interface XYArgs extends DataLayerArgs {
detailedTooltip?: boolean;
orderBucketsBySum?: boolean;
showTooltip: boolean;
handleEmptyXAccessor?: boolean;
useAdjustedInterval?: boolean;
}

export interface LayeredXYArgs {
Expand All @@ -239,7 +241,6 @@ export interface LayeredXYArgs {
valueLabels: ValueLabelMode;
layers?: XYExtendedLayerConfigResult[];
fittingFunction?: FittingFunction;
curveType?: XYCurveType;
fillOpacity?: number;
hideEndzones?: boolean;
valuesInLegend?: boolean;
Expand All @@ -252,6 +253,10 @@ export interface LayeredXYArgs {
minTimeBarInterval?: string;
orderBucketsBySum?: boolean;
showTooltip: boolean;
splitRowAccessor?: ExpressionValueVisDimension | string;
splitColumnAccessor?: ExpressionValueVisDimension | string;
handleEmptyXAccessor?: boolean;
useAdjustedInterval?: boolean;
}

export interface XYProps {
Expand All @@ -261,7 +266,6 @@ export interface XYProps {
valueLabels: ValueLabelMode;
layers: CommonXYLayerConfig[];
fittingFunction?: FittingFunction;
curveType?: XYCurveType;
fillOpacity?: number;
hideEndzones?: boolean;
valuesInLegend?: boolean;
Expand All @@ -276,6 +280,8 @@ export interface XYProps {
detailedTooltip?: boolean;
orderBucketsBySum?: boolean;
showTooltip: boolean;
handleEmptyXAccessor?: boolean;
useAdjustedInterval?: boolean;
}

export interface AnnotationLayerArgs {
Expand Down Expand Up @@ -317,12 +323,14 @@ export type XYLayerConfig = DataLayerConfig | ReferenceLineConfig | AnnotationLa
export type XYExtendedLayerConfig =
| ExtendedDataLayerConfig
| ReferenceLineLayerConfig
| ExtendedAnnotationLayerConfig;
| ExtendedAnnotationLayerConfig
| ReferenceLineConfig;

export type XYExtendedLayerConfigResult =
| ExtendedDataLayerConfigResult
| ReferenceLineLayerConfigResult
| ExtendedAnnotationLayerConfigResult;
| ExtendedAnnotationLayerConfigResult
| ReferenceLineConfigResult;

export interface ExtendedReferenceLineDecorationConfig extends ReferenceLineArgs {
type: typeof EXTENDED_REFERENCE_LINE_DECORATION_CONFIG;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
* Side Public License, v 1.
*/

export { logDatatables, getLayerDimensions } from './log_datatables';
export { logDatatables, logDatatable, getLayerDimensions } from './log_datatables';
Loading