Skip to content

Commit

Permalink
fix: fix sizing for showLegendExtra and ordinal xScale
Browse files Browse the repository at this point in the history
  • Loading branch information
rshen91 committed Oct 16, 2020
1 parent ba63b41 commit a054fdc
Show file tree
Hide file tree
Showing 61 changed files with 104 additions and 33 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
6 changes: 6 additions & 0 deletions integration/tests/interactions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,4 +295,10 @@ describe('Interactions', () => {
);
});
});
it('should size legends with ordinal x axis', async () => {
await common.expectChartWithMouseAtUrlToMatchScreenshot(
'http://localhost:9001/iframe.html?id=axes--different-tooltip-formatter',
{ left: 350, top: 130 },
);
});
});
52 changes: 44 additions & 8 deletions src/chart_types/xy_chart/legend/legend.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@ import { Position, RecursivePartial } from '../../../utils/commons';
import { AxisStyle } from '../../../utils/themes/theme';
import { SeriesCollectionValue, getSeriesName } from '../utils/series';
import { AxisSpec, BasicSeriesSpec, SeriesTypes } from '../utils/specs';
import { computeLegend } from './legend';
import { computeLegend, getLegendExtra } from './legend';

const nullDisplayValue = {
formatted: null,
raw: null,
legendSizingLabel: null,
};
const seriesCollectionValue1a = {
seriesIdentifier: {
Expand Down Expand Up @@ -129,7 +130,7 @@ describe('Legends', () => {
});
it('compute legend for a single series', () => {
seriesCollection.set('seriesCollectionValue1a', seriesCollectionValue1a);
const legend = computeLegend(seriesCollection, seriesCollectionMap, specs, 'violet', axesSpecs);
const legend = computeLegend(seriesCollection, seriesCollectionMap, specs, 'violet', axesSpecs, false);
const expected: LegendItem[] = [
{
color: 'red',
Expand All @@ -147,7 +148,7 @@ describe('Legends', () => {
it('compute legend for a single spec but with multiple series', () => {
seriesCollection.set('seriesCollectionValue1a', seriesCollectionValue1a);
seriesCollection.set('seriesCollectionValue1b', seriesCollectionValue1b);
const legend = computeLegend(seriesCollection, seriesCollectionMap, specs, 'violet', axesSpecs);
const legend = computeLegend(seriesCollection, seriesCollectionMap, specs, 'violet', axesSpecs, false);
const expected: LegendItem[] = [
{
color: 'red',
Expand Down Expand Up @@ -175,7 +176,7 @@ describe('Legends', () => {
it('compute legend for multiple specs', () => {
seriesCollection.set('seriesCollectionValue1a', seriesCollectionValue1a);
seriesCollection.set('seriesCollectionValue2a', seriesCollectionValue2a);
const legend = computeLegend(seriesCollection, seriesCollectionMap, specs, 'violet', axesSpecs);
const legend = computeLegend(seriesCollection, seriesCollectionMap, specs, 'violet', axesSpecs, false);
const expected: LegendItem[] = [
{
color: 'red',
Expand All @@ -202,13 +203,13 @@ describe('Legends', () => {
});
it('empty legend for missing spec', () => {
seriesCollection.set('seriesCollectionValue2b', seriesCollectionValue2b);
const legend = computeLegend(seriesCollection, seriesCollectionMap, specs, 'violet', axesSpecs);
const legend = computeLegend(seriesCollection, seriesCollectionMap, specs, 'violet', axesSpecs, false);
expect(legend.length).toEqual(0);
});
it('compute legend with default color for missing series color', () => {
seriesCollection.set('seriesCollectionValue1a', seriesCollectionValue1a);
const emptyColorMap = new Map<string, string>();
const legend = computeLegend(seriesCollection, emptyColorMap, specs, 'violet', axesSpecs);
const legend = computeLegend(seriesCollection, emptyColorMap, specs, 'violet', axesSpecs, false);
const expected: LegendItem[] = [
{
color: 'violet',
Expand All @@ -231,7 +232,7 @@ describe('Legends', () => {

const emptyColorMap = new Map<string, string>();

const legend = computeLegend(seriesCollection, emptyColorMap, specs, 'violet', axesSpecs);
const legend = computeLegend(seriesCollection, emptyColorMap, specs, 'violet', axesSpecs, false);

const visibility = [...legend.values()].map((item) => !item.isSeriesHidden);

Expand All @@ -246,7 +247,15 @@ describe('Legends', () => {
const emptyColorMap = new Map<string, string>();
const deselectedDataSeries = [seriesCollectionValue1a.seriesIdentifier, seriesCollectionValue1b.seriesIdentifier];

const legend = computeLegend(seriesCollection, emptyColorMap, specs, 'violet', axesSpecs, deselectedDataSeries);
const legend = computeLegend(
seriesCollection,
emptyColorMap,
specs,
'violet',
axesSpecs,
false,
deselectedDataSeries,
);

const visibility = [...legend.values()].map((item) => !item.isSeriesHidden);
expect(visibility).toEqual([false, false, true]);
Expand Down Expand Up @@ -344,4 +353,31 @@ describe('Legends', () => {
name = getSeriesName(seriesIdentifier1, false, false, specWithSplit);
expect(name).toBe('Spec 1 title');
});
it('should return correct legendSizingLabel with linear scale and showExtraLegend set to true', () => {
const formatter = (d: string | number) => `${Number(d).toFixed(2)} dogs`;
const lastValues = { y0: null, y1: 14 };
const showExtraLegend = true;
const xScaleIsLinear = ScaleType.Linear;

expect(getLegendExtra(showExtraLegend, xScaleIsLinear, formatter, 'y1', lastValues)).toMatchObject({
legendSizingLabel: '14.00 dogs',
});
});
it('should return formatted to null with ordinal scale and showExtraLegend set to true', () => {
const formatter = (d: string | number) => `${Number(d).toFixed(2)} dogs`;
const lastValues = { y0: null, y1: 14 };

expect(getLegendExtra(true, ScaleType.Ordinal, formatter, 'y1', lastValues)).toMatchObject({
formatted: null,
});
});
it('should return legendSizingLabel null with showLegendExtra set to false', () => {
const formatter = (d: string | number) => `${Number(d).toFixed(2)} dogs`;
const lastValues = { y0: null, y1: 14 };
const showLegendExtra = false;

expect(getLegendExtra(showLegendExtra, ScaleType.Ordinal, formatter, 'y1', lastValues)).toMatchObject({
legendSizingLabel: null,
});
});
});
39 changes: 31 additions & 8 deletions src/chart_types/xy_chart/legend/legend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,12 @@

import { LegendItem } from '../../../commons/legend';
import { SeriesKey, SeriesIdentifier } from '../../../commons/series_id';
import { ScaleType } from '../../../scales/constants';
import { TickFormatterOptions } from '../../../specs';
import { Color } from '../../../utils/commons';
import { BandedAccessorType } from '../../../utils/geometry';
import { getAxesSpecForSpecId, getSpecsById } from '../state/utils/spec';
import { LastValues } from '../state/utils/types';
import { Y0_ACCESSOR_POSTFIX, Y1_ACCESSOR_POSTFIX } from '../tooltip/tooltip';
import { defaultTickFormatter } from '../utils/axis_utils';
import {
Expand Down Expand Up @@ -55,6 +58,29 @@ function getBandedLegendItemLabel(name: string, yAccessor: BandedAccessorType, p
? `${name}${postfixes.y1AccessorFormat}`
: `${name}${postfixes.y0AccessorFormat}`;
}
export function getLegendExtra(
showLegendExtra: boolean,
xScaleType: ScaleType,
formatter: (value: any, options?: TickFormatterOptions | undefined) => string,
key: keyof LastValues,
lastValue?: LastValues,
): LegendItem['defaultExtra'] {
if (showLegendExtra) {
const rawValue = (lastValue && lastValue[key]) ?? null;
const formattedValue = rawValue !== null ? formatter(rawValue) : null;

return {
raw: rawValue !== null ? rawValue : null,
formatted: xScaleType === ScaleType.Ordinal ? null : formattedValue,
legendSizingLabel: formattedValue,
};
}
return {
raw: null,
formatted: null,
legendSizingLabel: null,
};
}

/** @internal */
export function computeLegend(
Expand All @@ -63,9 +89,11 @@ export function computeLegend(
specs: BasicSeriesSpec[],
defaultColor: string,
axesSpecs: AxisSpec[],
showLegendExtra: boolean,
deselectedDataSeries: SeriesIdentifier[] = [],
): LegendItem[] {
const legendItems: LegendItem[] = [];

const sortedCollection = getSortedDataSeriesColorsValuesMap(seriesCollection);

sortedCollection.forEach((series, key) => {
Expand All @@ -79,6 +107,7 @@ export function computeLegend(
if (name === '' || !spec) {
return;
}

const postFixes = getPostfix(spec);
const labelY1 = banded ? getBandedLegendItemLabel(name, BandedAccessorType.Y1, postFixes) : name;

Expand All @@ -95,10 +124,7 @@ export function computeLegend(
isSeriesHidden,
isItemHidden: hideInLegend,
isToggleable: true,
defaultExtra: {
raw: lastValue && lastValue.y1 !== null ? lastValue.y1 : null,
formatted: lastValue && lastValue.y1 !== null ? formatter(lastValue.y1) : null,
},
defaultExtra: getLegendExtra(showLegendExtra, spec.xScaleType, formatter, 'y1', lastValue),
});
if (banded) {
const labelY0 = getBandedLegendItemLabel(name, BandedAccessorType.Y0, postFixes);
Expand All @@ -110,10 +136,7 @@ export function computeLegend(
isSeriesHidden,
isItemHidden: hideInLegend,
isToggleable: true,
defaultExtra: {
raw: lastValue && lastValue.y0 !== null ? lastValue.y0 : null,
formatted: lastValue && lastValue.y0 !== null ? formatter(lastValue.y0) : null,
},
defaultExtra: getLegendExtra(showLegendExtra, spec.xScaleType, formatter, 'y0', lastValue),
});
}
});
Expand Down
1 change: 1 addition & 0 deletions src/chart_types/xy_chart/rendering/rendering.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ describe('Rendering utils', () => {
defaultExtra: {
formatted: null,
raw: null,
legendSizingLabel: null,
},
};

Expand Down
2 changes: 2 additions & 0 deletions src/chart_types/xy_chart/state/chart_state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ describe.skip('Chart Store', () => {
defaultExtra: {
raw: null,
formatted: 'formatted-last',
legendSizingLabel: 'formatted-last',
},
};

Expand All @@ -93,6 +94,7 @@ describe.skip('Chart Store', () => {
defaultExtra: {
raw: null,
formatted: 'formatted-last',
legendSizingLabel: 'formatted-last',
},
};
beforeEach(() => {
Expand Down
13 changes: 12 additions & 1 deletion src/chart_types/xy_chart/state/selectors/compute_legend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { LegendItem } from '../../../../commons/legend';
import { getChartIdSelector } from '../../../../state/selectors/get_chart_id';
import { getChartThemeSelector } from '../../../../state/selectors/get_chart_theme';
import { getDeselectedSeriesSelector } from '../../../../state/selectors/get_deselected_data_series';
import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs';
import { computeLegend } from '../../legend/legend';
import { computeSeriesDomainsSelector } from './compute_series_domains';
import { getSeriesColorsSelector } from './get_series_color_map';
Expand All @@ -37,14 +38,24 @@ export const computeLegendSelector = createCachedSelector(
getSeriesColorsSelector,
getAxisSpecsSelector,
getDeselectedSeriesSelector,
getSettingsSpecSelector,
],
(seriesSpecs, seriesDomainsAndData, chartTheme, seriesColors, axesSpecs, deselectedDataSeries): LegendItem[] =>
(
seriesSpecs,
seriesDomainsAndData,
chartTheme,
seriesColors,
axesSpecs,
deselectedDataSeries,
{ showLegendExtra },
): LegendItem[] =>
computeLegend(
seriesDomainsAndData.seriesCollection,
seriesColors,
seriesSpecs,
chartTheme.colors.defaultVizColor,
axesSpecs,
showLegendExtra,
deselectedDataSeries,
),
)(getChartIdSelector);
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ export const getLegendItemsLabelsSelector = createCachedSelector(
[computeLegendSelector, getSettingsSpecSelector],
(legendItems, { showLegendExtra }): LegendItemLabel[] =>
legendItems.map(({ label, defaultExtra }) => {
if (defaultExtra?.formatted != null) {
return { label: `${label}${showLegendExtra ? defaultExtra.formatted : ''}`, depth: 0 };
if (defaultExtra?.legendSizingLabel != null) {
return { label: `${label}${showLegendExtra ? defaultExtra.legendSizingLabel : ''}`, depth: 0 };
}
return { label, depth: 0 };
}),
Expand Down
8 changes: 4 additions & 4 deletions src/chart_types/xy_chart/state/utils/common.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ describe('Type Checks', () => {
key: 'specId:{bars},colors:{a}',
specId: 'bars',
},
defaultExtra: { raw: 6, formatted: '6.00' },
defaultExtra: { raw: 6, formatted: '6.00', legendSizingLabel: '6.00' },
isSeriesHidden: true,
},
{
Expand All @@ -151,7 +151,7 @@ describe('Type Checks', () => {
key: 'specId:{bars},colors:{b}',
specId: 'bars',
},
defaultExtra: { raw: 2, formatted: '2.00' },
defaultExtra: { raw: 2, formatted: '2.00', legendSizingLabel: '2.00' },
isSeriesHidden: true,
},
];
Expand All @@ -166,7 +166,7 @@ describe('Type Checks', () => {
key: 'specId:{bars},colors:{a}',
specId: 'bars',
},
defaultExtra: { raw: 6, formatted: '6.00' },
defaultExtra: { raw: 6, formatted: '6.00', legendSizingLabel: '6.00' },
isSeriesHidden: false,
},
{
Expand All @@ -176,7 +176,7 @@ describe('Type Checks', () => {
key: 'specId:{bars},colors:{b}',
specId: 'bars',
},
defaultExtra: { raw: 2, formatted: '2.00' },
defaultExtra: { raw: 2, formatted: '2.00', legendSizingLabel: '2.00' },
isSeriesHidden: true,
},
];
Expand Down
5 changes: 1 addition & 4 deletions src/chart_types/xy_chart/state/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

import { SeriesKey, SeriesIdentifier } from '../../../../commons/series_id';
import { Scale } from '../../../../scales';
import { ScaleType } from '../../../../scales/constants';
import { OrderBy } from '../../../../specs/settings';
import { mergePartial, Rotation, Color, isUniqueArray } from '../../../../utils/commons';
import { CurveType } from '../../../../utils/curves';
Expand Down Expand Up @@ -139,9 +138,7 @@ function getLastValues(
xDomain: XDomain,
): Map<SeriesKey, LastValues> {
const lastValues = new Map<SeriesKey, LastValues>();
if (xDomain.scaleType === ScaleType.Ordinal) {
return lastValues;
}

// we need to get the latest
formattedDataSeries.stacked.forEach(({ dataSeries, stackMode }) => {
dataSeries.forEach((series) => {
Expand Down
1 change: 1 addition & 0 deletions src/commons/legend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export type LegendItem = {
defaultExtra?: {
raw: number | null;
formatted: number | string | null;
legendSizingLabel: number | string | null;
};
// TODO: Remove when partition layers are toggleable
isToggleable?: boolean;
Expand Down
6 changes: 0 additions & 6 deletions stories/axes/6a_different_tooltip_formatter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,6 @@ export const Example = () => {
return (
<Chart className="story-chart">
<Settings
theme={{
legend: {
// Used to allow space for long formatting
spacingBuffer: 60,
},
}}
showLegendExtra
showLegend={showLegend}
tooltip={{
Expand Down

0 comments on commit a054fdc

Please sign in to comment.