Skip to content

Commit

Permalink
[Lens] Move empty string handling into field formatter (elastic#102877)
Browse files Browse the repository at this point in the history
# Conflicts:
#	x-pack/test/functional/apps/discover/__snapshots__/reporting.snap
  • Loading branch information
flash1293 committed Jun 24, 2021
1 parent ad1dd21 commit c896e79
Show file tree
Hide file tree
Showing 18 changed files with 23 additions and 235 deletions.
7 changes: 7 additions & 0 deletions src/plugins/data/common/field_formats/converters/string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ import { FieldFormat } from '../field_format';
import { TextContextTypeConvert, FIELD_FORMAT_IDS } from '../types';
import { shortenDottedString } from '../../utils';

export const emptyLabel = i18n.translate('data.fieldFormats.string.emptyLabel', {
defaultMessage: '(empty)',
});

const TRANSFORM_OPTIONS = [
{
kind: false,
Expand Down Expand Up @@ -103,6 +107,9 @@ export class StringFormat extends FieldFormat {
}

textConvert: TextContextTypeConvert = (val) => {
if (val === '') {
return emptyLabel;
}
switch (this.param('transform')) {
case 'lower':
return String(val).toLowerCase();
Expand Down
6 changes: 0 additions & 6 deletions src/plugins/vis_type_pie/public/utils/get_layers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
* Side Public License, v 1.
*/

import { i18n } from '@kbn/i18n';
import {
Datum,
PartitionFillLabel,
Expand Down Expand Up @@ -125,11 +124,6 @@ export const getLayers = (
},
showAccessor: (d: Datum) => d !== EMPTY_SLICE,
nodeLabel: (d: unknown) => {
if (d === '') {
return i18n.translate('visTypePie.emptyLabelValue', {
defaultMessage: '(empty)',
});
}
if (col.format) {
const formattedLabel = formatter.deserialize(col.format).convert(d) ?? '';
if (visParams.labels.truncate && formattedLabel.length <= visParams.labels.truncate) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {
import { Aspects } from '../types';

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

interface TooltipData {
Expand Down Expand Up @@ -100,8 +99,7 @@ export const getTooltipData = (
return data;
};

const renderData = ({ label, value: rawValue }: TooltipData, index: number) => {
const value = fillEmptyValue(rawValue);
const renderData = ({ label, value }: TooltipData, index: number) => {
return label && value ? (
<tr key={label + value + index}>
<td className="detailedTooltip__label">
Expand Down
3 changes: 1 addition & 2 deletions src/plugins/vis_type_xy/public/components/xy_settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import { renderEndzoneTooltip } from '../../../charts/public';

import { getThemeService, getUISettings } from '../services';
import { VisConfig } from '../types';
import { fillEmptyValue } from '../utils/get_series_name_fn';

declare global {
interface Window {
Expand Down Expand Up @@ -134,7 +133,7 @@ export const XYSettings: FC<XYSettingsProps> = ({
};

const headerValueFormatter: TickFormatter<any> | undefined = xAxis.ticks?.formatter
? (value) => fillEmptyValue(xAxis.ticks?.formatter?.(value)) ?? ''
? (value) => xAxis.ticks?.formatter?.(value) ?? ''
: undefined;
const headerFormatter =
isTimeChart && xDomain && adjustedXDomain
Expand Down
4 changes: 1 addition & 3 deletions src/plugins/vis_type_xy/public/config/get_axis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import {
YScaleType,
SeriesParam,
} from '../types';
import { fillEmptyValue } from '../utils/get_series_name_fn';

export function getAxis<S extends XScaleType | YScaleType>(
{ type, title: axisTitle, labels, scale: axisScale, ...axis }: CategoryAxis,
Expand Down Expand Up @@ -90,8 +89,7 @@ function getLabelFormatter(
}

return (value: any) => {
const formattedStringValue = `${formatter ? formatter(value) : value}`;
const finalValue = fillEmptyValue(formattedStringValue);
const finalValue = `${formatter ? formatter(value) : value}`;

if (finalValue.length > truncate) {
return `${finalValue.slice(0, truncate)}...`;
Expand Down
13 changes: 1 addition & 12 deletions src/plugins/vis_type_xy/public/utils/get_series_name_fn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,10 @@

import { memoize } from 'lodash';

import { i18n } from '@kbn/i18n';
import { XYChartSeriesIdentifier, SeriesName } from '@elastic/charts';

import { VisConfig } from '../types';

const emptyTextLabel = i18n.translate('visTypeXy.emptyTextColumnValue', {
defaultMessage: '(empty)',
});

/**
* Returns empty values
*/
export const fillEmptyValue = <T extends string | number | undefined>(value: T) =>
value === '' ? emptyTextLabel : value;

function getSplitValues(
splitAccessors: XYChartSeriesIdentifier['splitAccessors'],
seriesAspects?: VisConfig['aspects']['series']
Expand All @@ -36,7 +25,7 @@ function getSplitValues(
const split = (seriesAspects ?? []).find(({ accessor }) => accessor === key);
splitValues.push(split?.formatter ? split?.formatter(value) : value);
});
return splitValues.map(fillEmptyValue);
return splitValues;
}

export const getSeriesNameFn = (aspects: VisConfig['aspects'], multipleY = false) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import type {
LensToggleAction,
} from './types';
import { ColumnConfig } from './table_basic';

import { desanitizeFilterContext } from '../../utils';
import { getOriginalId } from '../transpose_helpers';

export const createGridResizeHandler = (
Expand Down Expand Up @@ -92,7 +90,7 @@ export const createGridFilterHandler = (
timeFieldName,
};

onClickValue(desanitizeFilterContext(data));
onClickValue(data);
};

export const createTransposeColumnFilterHandler = (
Expand Down Expand Up @@ -125,7 +123,7 @@ export const createTransposeColumnFilterHandler = (
timeFieldName,
};

onClickValue(desanitizeFilterContext(data));
onClickValue(data);
};

export const createGridSortingConfig = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import { VisualizationContainer } from '../visualization_container';
import { HeatmapRenderProps } from './types';
import './index.scss';
import { LensBrushEvent, LensFilterEvent } from '../types';
import { desanitizeFilterContext } from '../utils';
import { EmptyPlaceholder } from '../shared_components';
import { LensIconChartHeatmap } from '../assets/chart_heatmap';

Expand Down Expand Up @@ -117,7 +116,7 @@ export const HeatmapComponent: FC<HeatmapRenderProps> = ({
})),
timeFieldName,
};
onClickValue(desanitizeFilterContext(context));
onClickValue(context);
}) as ElementClickListener;

const onBrushEnd = (e: HeatmapBrushEvent) => {
Expand Down Expand Up @@ -164,7 +163,7 @@ export const HeatmapComponent: FC<HeatmapRenderProps> = ({
})),
timeFieldName,
};
onClickValue(desanitizeFilterContext(context));
onClickValue(context);
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,29 +83,6 @@ describe('rename_columns', () => {
`);
});

it('should replace "" with a visible value', () => {
const input: Datatable = {
type: 'datatable',
columns: [{ id: 'a', name: 'A', meta: { type: 'string' } }],
rows: [{ a: '' }],
};

const idMap = {
a: {
id: 'a',
label: 'Austrailia',
},
};

const result = renameColumns.fn(
input,
{ idMap: JSON.stringify(idMap) },
createMockExecutionContext()
);

expect(result.rows[0].a).toEqual('(empty)');
});

it('should keep columns which are not mapped', () => {
const input: Datatable = {
type: 'datatable',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ export const renameColumns: ExpressionFunctionDefinition<

Object.entries(row).forEach(([id, value]) => {
if (id in idMap) {
mappedRow[idMap[id].id] = sanitizeValue(value);
mappedRow[idMap[id].id] = value;
} else {
mappedRow[id] = sanitizeValue(value);
mappedRow[id] = value;
}
});

Expand Down Expand Up @@ -86,13 +86,3 @@ function getColumnName(originalColumn: OriginalColumn, newColumn: DatatableColum

return originalColumn.label;
}

function sanitizeValue(value: unknown) {
if (value === '') {
return i18n.translate('xpack.lens.indexpattern.emptyTextColumnValue', {
defaultMessage: '(empty)',
});
}

return value;
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import { PieExpressionProps } from './types';
import { getSliceValue, getFilterContext } from './render_helpers';
import { EmptyPlaceholder } from '../shared_components';
import './visualization.scss';
import { desanitizeFilterContext } from '../utils';
import {
ChartsPluginSetup,
PaletteRegistry,
Expand Down Expand Up @@ -254,7 +253,7 @@ export function PieComponent(
const onElementClickHandler: ElementClickListener = (args) => {
const context = getFilterContext(args[0][0] as LayerValue[], groups, firstTable);

onClickValue(desanitizeFilterContext(context));
onClickValue(context);
};

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import React, { useState } from 'react';
import { i18n } from '@kbn/i18n';
import { EuiContextMenuPanelDescriptor, EuiIcon, EuiPopover, EuiContextMenu } from '@elastic/eui';
import type { LensFilterEvent } from '../types';
import { desanitizeFilterContext } from '../utils';

export interface LegendActionPopoverProps {
/**
Expand Down Expand Up @@ -45,7 +44,7 @@ export const LegendActionPopover: React.FunctionComponent<LegendActionPopoverPro
icon: <EuiIcon type="plusInCircle" size="m" />,
onClick: () => {
setPopoverOpen(false);
onFilter(desanitizeFilterContext(context));
onFilter(context);
},
},
{
Expand All @@ -56,7 +55,7 @@ export const LegendActionPopover: React.FunctionComponent<LegendActionPopoverPro
icon: <EuiIcon type="minusInCircle" size="m" />,
onClick: () => {
setPopoverOpen(false);
onFilter(desanitizeFilterContext({ ...context, negate: true }));
onFilter({ ...context, negate: true });
},
},
],
Expand Down
118 changes: 0 additions & 118 deletions x-pack/plugins/lens/public/utils.test.ts

This file was deleted.

Loading

0 comments on commit c896e79

Please sign in to comment.