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

[Lens] Move empty string handling into field formatter #102877

Merged
merged 9 commits into from
Jun 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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