From 1182969534f63238a09ad7ed4b286b5b14917955 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Wed, 9 Sep 2020 11:01:05 -0500 Subject: [PATCH 1/2] feat: blind sorting option for vislib --- .../state/selectors/compute_series_domains.ts | 2 + src/chart_types/xy_chart/state/utils/utils.ts | 4 + src/chart_types/xy_chart/utils/series.ts | 172 ++++++++++++------ 3 files changed, 124 insertions(+), 54 deletions(-) diff --git a/src/chart_types/xy_chart/state/selectors/compute_series_domains.ts b/src/chart_types/xy_chart/state/selectors/compute_series_domains.ts index 8518f8ee64..97b1cd1474 100644 --- a/src/chart_types/xy_chart/state/selectors/compute_series_domains.ts +++ b/src/chart_types/xy_chart/state/selectors/compute_series_domains.ts @@ -38,6 +38,8 @@ export const computeSeriesDomainsSelector = createCachedSelector( customYDomainsByGroupId, deselectedDataSeries, settingsSpec.xDomain, + // @ts-ignore blind sort option for vislib + settingsSpec.vislibSort, ); return domains; }, diff --git a/src/chart_types/xy_chart/state/utils/utils.ts b/src/chart_types/xy_chart/state/utils/utils.ts index 69ae6c137b..b1d409f7e9 100644 --- a/src/chart_types/xy_chart/state/utils/utils.ts +++ b/src/chart_types/xy_chart/state/utils/utils.ts @@ -192,6 +192,8 @@ function getLastValues(formattedDataSeries: { * @param customYDomainsByGroupId custom Y domains grouped by GroupId * @param customXDomain if specified in , the custom X domain * @param deselectedDataSeries is optional; if not supplied, + * @param customXDomain is optional; if not supplied, + * @param vislibSort is optional; if not specified in , * then all series will be factored into computations. Otherwise, selectedDataSeries * is used to restrict the computation for just the selected series * @returns `SeriesDomainsAndData` @@ -202,10 +204,12 @@ export function computeSeriesDomains( customYDomainsByGroupId: Map = new Map(), deselectedDataSeries: SeriesIdentifier[] = [], customXDomain?: DomainRange | Domain, + vislibSort?: boolean, ): SeriesDomainsAndData { const { dataSeriesBySpecId, xValues, seriesCollection, fallbackScale } = getDataSeriesBySpecId( seriesSpecs, deselectedDataSeries, + vislibSort, ); // compute the x domain merging any custom domain diff --git a/src/chart_types/xy_chart/utils/series.ts b/src/chart_types/xy_chart/utils/series.ts index 1c9bb7c50a..ed5dca5e92 100644 --- a/src/chart_types/xy_chart/utils/series.ts +++ b/src/chart_types/xy_chart/utils/series.ts @@ -111,18 +111,21 @@ export function getSeriesIndex(series: SeriesIdentifier[], target: SeriesIdentif * `y` values and `mark` values are casted to number or null. * @internal */ -export function splitSeriesDataByAccessors({ - id: specId, - data, - xAccessor, - yAccessors, - y0Accessors, - markSizeAccessor, - splitSeriesAccessors = [], -}: Pick< - BasicSeriesSpec, - 'id' | 'data' | 'xAccessor' | 'yAccessors' | 'y0Accessors' | 'splitSeriesAccessors' | 'markSizeAccessor' ->): { +export function splitSeriesDataByAccessors( + { + id: specId, + data, + xAccessor, + yAccessors, + y0Accessors, + markSizeAccessor, + splitSeriesAccessors = [], + }: Pick< + BasicSeriesSpec, + 'id' | 'data' | 'xAccessor' | 'yAccessors' | 'y0Accessors' | 'splitSeriesAccessors' | 'markSizeAccessor' + >, + vislibSort = false, +): { dataSeries: Map; xValues: Array; } { @@ -130,57 +133,116 @@ export function splitSeriesDataByAccessors({ const xValues: Array = []; const nonNumericValues: any[] = []; - data.forEach((datum) => { - const splitAccessors = getSplitAccessors(datum, splitSeriesAccessors); - // if splitSeriesAccessors are defined we should have at least one split value to include datum - if (splitSeriesAccessors.length > 0 && splitAccessors.size < 1) { - return; - } - - // skip if the datum is not an object or null - if (typeof datum !== 'object' || datum === null) { - return null; - } + if (vislibSort) { + /* + * This logic is mostly duplicated from below but is a temporary fix before + * https://github.com/elastic/elastic-charts/issues/795 is completed to allow sorting + * The difference from below is that it loops through all the yAsccessors before the data. + */ + yAccessors.forEach((accessor, index) => { + data.forEach((datum) => { + const splitAccessors = getSplitAccessors(datum, splitSeriesAccessors); + // if splitSeriesAccessors are defined we should have at least one split value to include datum + if (splitSeriesAccessors.length > 0 && splitAccessors.size < 1) { + return; + } - const x = getAccessorValue(datum, xAccessor); + // skip if the datum is not an object or null + if (typeof datum !== 'object' || datum === null) { + return null; + } - // skip if the x value is not a string or a number - if (typeof x !== 'string' && typeof x !== 'number') { - return null; - } + const x = getAccessorValue(datum, xAccessor); - xValues.push(x); + // skip if the x value is not a string or a number + if (typeof x !== 'string' && typeof x !== 'number') { + return null; + } - yAccessors.forEach((accessor, index) => { - const cleanedDatum = extractYandMarkFromDatum( - datum, - accessor, - nonNumericValues, - y0Accessors && y0Accessors[index], - markSizeAccessor, - ); - const seriesKeys = [...splitAccessors.values(), accessor]; - const seriesKey = getSeriesKey({ - specId, - yAccessor: accessor, - splitAccessors, - }); - const newDatum = { x, ...cleanedDatum }; - const series = dataSeries.get(seriesKey); - if (series) { - series.data.push(newDatum); - } else { - dataSeries.set(seriesKey, { + xValues.push(x); + + const cleanedDatum = extractYandMarkFromDatum( + datum, + accessor, + nonNumericValues, + y0Accessors && y0Accessors[index], + markSizeAccessor, + ); + const seriesKeys = [...splitAccessors.values(), accessor]; + const seriesKey = getSeriesKey({ specId, yAccessor: accessor, splitAccessors, - data: [newDatum], - key: seriesKey, - seriesKeys, }); + const newDatum = { x, ...cleanedDatum }; + const series = dataSeries.get(seriesKey); + if (series) { + series.data.push(newDatum); + } else { + dataSeries.set(seriesKey, { + specId, + yAccessor: accessor, + splitAccessors, + data: [newDatum], + key: seriesKey, + seriesKeys, + }); + } + }); + }); + } else { + data.forEach((datum) => { + const splitAccessors = getSplitAccessors(datum, splitSeriesAccessors); + // if splitSeriesAccessors are defined we should have at least one split value to include datum + if (splitSeriesAccessors.length > 0 && splitAccessors.size < 1) { + return; } + + // skip if the datum is not an object or null + if (typeof datum !== 'object' || datum === null) { + return null; + } + + const x = getAccessorValue(datum, xAccessor); + + // skip if the x value is not a string or a number + if (typeof x !== 'string' && typeof x !== 'number') { + return null; + } + + xValues.push(x); + + yAccessors.forEach((accessor, index) => { + const cleanedDatum = extractYandMarkFromDatum( + datum, + accessor, + nonNumericValues, + y0Accessors && y0Accessors[index], + markSizeAccessor, + ); + const seriesKeys = [...splitAccessors.values(), accessor]; + const seriesKey = getSeriesKey({ + specId, + yAccessor: accessor, + splitAccessors, + }); + const newDatum = { x, ...cleanedDatum }; + const series = dataSeries.get(seriesKey); + if (series) { + series.data.push(newDatum); + } else { + dataSeries.set(seriesKey, { + specId, + yAccessor: accessor, + splitAccessors, + data: [newDatum], + key: seriesKey, + seriesKeys, + }); + } + }); }); - }); + } if (nonNumericValues.length > 0) { Logger.warn( @@ -350,11 +412,13 @@ function getDataSeriesBySpecGroup( * * @param seriesSpecs the map for all the series spec * @param deselectedDataSeries the array of deselected/hidden data series + * @param vislibSort is optional; if not specified in , * @internal */ export function getDataSeriesBySpecId( seriesSpecs: BasicSeriesSpec[], deselectedDataSeries: SeriesIdentifier[] = [], + vislibSort?: boolean, ): { dataSeriesBySpecId: Map; seriesCollection: Map; @@ -377,7 +441,7 @@ export function getDataSeriesBySpecId( isOrdinalScale = true; } - const { dataSeries, xValues } = splitSeriesDataByAccessors(spec); + const { dataSeries, xValues } = splitSeriesDataByAccessors(spec, vislibSort); // filter deleselected dataseries let filteredDataSeries: DataSeries[] = [...dataSeries.values()]; From 4355c0dc9c2e5f4902a8300361238ec164096ea6 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Wed, 9 Sep 2020 12:16:02 -0500 Subject: [PATCH 2/2] fix: update naming and loop logic --- .../state/selectors/compute_series_domains.ts | 2 +- src/chart_types/xy_chart/state/utils/utils.ts | 6 +++--- src/chart_types/xy_chart/utils/series.ts | 18 +++++++++--------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/chart_types/xy_chart/state/selectors/compute_series_domains.ts b/src/chart_types/xy_chart/state/selectors/compute_series_domains.ts index 97b1cd1474..fb62329e7d 100644 --- a/src/chart_types/xy_chart/state/selectors/compute_series_domains.ts +++ b/src/chart_types/xy_chart/state/selectors/compute_series_domains.ts @@ -39,7 +39,7 @@ export const computeSeriesDomainsSelector = createCachedSelector( deselectedDataSeries, settingsSpec.xDomain, // @ts-ignore blind sort option for vislib - settingsSpec.vislibSort, + settingsSpec.enableVislibSeriesSort, ); return domains; }, diff --git a/src/chart_types/xy_chart/state/utils/utils.ts b/src/chart_types/xy_chart/state/utils/utils.ts index b1d409f7e9..ac50c7f892 100644 --- a/src/chart_types/xy_chart/state/utils/utils.ts +++ b/src/chart_types/xy_chart/state/utils/utils.ts @@ -193,7 +193,7 @@ function getLastValues(formattedDataSeries: { * @param customXDomain if specified in , the custom X domain * @param deselectedDataSeries is optional; if not supplied, * @param customXDomain is optional; if not supplied, - * @param vislibSort is optional; if not specified in , + * @param enableVislibSeriesSort is optional; if not specified in , * then all series will be factored into computations. Otherwise, selectedDataSeries * is used to restrict the computation for just the selected series * @returns `SeriesDomainsAndData` @@ -204,12 +204,12 @@ export function computeSeriesDomains( customYDomainsByGroupId: Map = new Map(), deselectedDataSeries: SeriesIdentifier[] = [], customXDomain?: DomainRange | Domain, - vislibSort?: boolean, + enableVislibSeriesSort?: boolean, ): SeriesDomainsAndData { const { dataSeriesBySpecId, xValues, seriesCollection, fallbackScale } = getDataSeriesBySpecId( seriesSpecs, deselectedDataSeries, - vislibSort, + enableVislibSeriesSort, ); // compute the x domain merging any custom domain diff --git a/src/chart_types/xy_chart/utils/series.ts b/src/chart_types/xy_chart/utils/series.ts index ed5dca5e92..e19bafbecc 100644 --- a/src/chart_types/xy_chart/utils/series.ts +++ b/src/chart_types/xy_chart/utils/series.ts @@ -124,7 +124,7 @@ export function splitSeriesDataByAccessors( BasicSeriesSpec, 'id' | 'data' | 'xAccessor' | 'yAccessors' | 'y0Accessors' | 'splitSeriesAccessors' | 'markSizeAccessor' >, - vislibSort = false, + enableVislibSeriesSort = false, ): { dataSeries: Map; xValues: Array; @@ -133,7 +133,7 @@ export function splitSeriesDataByAccessors( const xValues: Array = []; const nonNumericValues: any[] = []; - if (vislibSort) { + if (enableVislibSeriesSort) { /* * This logic is mostly duplicated from below but is a temporary fix before * https://github.com/elastic/elastic-charts/issues/795 is completed to allow sorting @@ -149,14 +149,14 @@ export function splitSeriesDataByAccessors( // skip if the datum is not an object or null if (typeof datum !== 'object' || datum === null) { - return null; + return; } const x = getAccessorValue(datum, xAccessor); // skip if the x value is not a string or a number if (typeof x !== 'string' && typeof x !== 'number') { - return null; + return; } xValues.push(x); @@ -200,14 +200,14 @@ export function splitSeriesDataByAccessors( // skip if the datum is not an object or null if (typeof datum !== 'object' || datum === null) { - return null; + return; } const x = getAccessorValue(datum, xAccessor); // skip if the x value is not a string or a number if (typeof x !== 'string' && typeof x !== 'number') { - return null; + return; } xValues.push(x); @@ -412,13 +412,13 @@ function getDataSeriesBySpecGroup( * * @param seriesSpecs the map for all the series spec * @param deselectedDataSeries the array of deselected/hidden data series - * @param vislibSort is optional; if not specified in , + * @param enableVislibSeriesSort is optional; if not specified in , * @internal */ export function getDataSeriesBySpecId( seriesSpecs: BasicSeriesSpec[], deselectedDataSeries: SeriesIdentifier[] = [], - vislibSort?: boolean, + enableVislibSeriesSort?: boolean, ): { dataSeriesBySpecId: Map; seriesCollection: Map; @@ -441,7 +441,7 @@ export function getDataSeriesBySpecId( isOrdinalScale = true; } - const { dataSeries, xValues } = splitSeriesDataByAccessors(spec, vislibSort); + const { dataSeries, xValues } = splitSeriesDataByAccessors(spec, enableVislibSeriesSort); // filter deleselected dataseries let filteredDataSeries: DataSeries[] = [...dataSeries.values()];