fix: group legend items by label and color#999
Conversation
Codecov Report
@@ Coverage Diff @@
## master #999 +/- ##
==========================================
+ Coverage 72.13% 72.78% +0.65%
==========================================
Files 355 363 +8
Lines 11160 11211 +51
Branches 2449 2434 -15
==========================================
+ Hits 8050 8160 +110
+ Misses 3095 3037 -58
+ Partials 15 14 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
I feel like this approach will suffice in the short term but it seems like this solution could be problematic. Mainly around grouping by color and not some arbitrary value. So in Grid color picker example code/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { EuiWrappingPopover, EuiColorPicker, EuiSpacer, EuiFlexItem, EuiButtonEmpty, EuiButton } from '@elastic/eui';
import { action } from '@storybook/addon-actions';
import { boolean } from '@storybook/addon-knobs';
import { DateTime } from 'luxon';
import React, { useMemo, useState } from 'react';
import {
ScaleType,
Position,
Chart,
Axis,
LineSeries,
GroupBy,
SmallMultiples,
Settings,
LIGHT_THEME,
niceTimeFormatByDay,
timeFormatter,
LegendColorPicker,
} from '../../src';
import { SeriesKey } from '../../src/common/series_id';
import { SeededDataGenerator } from '../../src/mocks/utils';
import { Color, toEntries } from '../../src/utils/common';
import { SB_SOURCE_PANEL } from '../utils/storybook';
const dg = new SeededDataGenerator();
const numOfDays = 90;
const groupNames = new Array(16).fill(0).map((d, i) => String.fromCharCode(97 + i));
const data = dg.generateGroupedSeries(numOfDays, 16).map((d) => {
return {
y: d.y,
x: DateTime.fromISO('2020-01-01T00:00:00Z').plus({ days: d.x }).toMillis(),
g: d.g,
h: `host ${groupNames.indexOf(d.g) % 4}`,
v: `metric ${Math.floor(groupNames.indexOf(d.g) / 4)}`,
};
});
const onChangeAction = action('onChange');
const onCloseAction = action('onClose');
export const Example = () => {
const showLegend = boolean('Show Legend', true);
const onElementClick = action('onElementClick');
const [colors, setColors] = useState<Record<SeriesKey, Color | null>>({});
const CustomColorPicker: LegendColorPicker = useMemo(
() => ({ anchor, color, onClose, seriesIdentifiers, onChange }) => {
const handleClose = () => {
onClose();
onCloseAction();
setColors((prevColors) => ({
...prevColors,
...toEntries(seriesIdentifiers, 'key', color),
}));
};
const handleChange = (c: Color | null) => {
setColors((prevColors) => ({
...prevColors,
...toEntries(seriesIdentifiers, 'key', c),
}));
onChange(c);
onChangeAction(c);
};
return (
<>
<EuiWrappingPopover isOpen button={anchor} closePopover={handleClose} anchorPosition="leftCenter" ownFocus>
<EuiColorPicker display="inline" color={color} onChange={handleChange} />
<EuiSpacer size="m" />
<EuiFlexItem grow={false}>
<EuiButtonEmpty size="s" onClick={() => handleChange(null)}>
Clear color
</EuiButtonEmpty>
</EuiFlexItem>
<EuiButton fullWidth size="s" onClick={handleClose}>
Done
</EuiButton>
</EuiWrappingPopover>
</>
);
},
[setColors],
);
CustomColorPicker.displayName = 'CustomColorPicker';
console.log(colors);
return (
<Chart className="story-chart">
<Settings
legendColorPicker={CustomColorPicker}
onElementClick={onElementClick}
showLegend={showLegend}
theme={{
lineSeriesStyle: {
point: {
visible: false,
},
},
}}
/>
<Axis
id="time"
title="timestamp"
position={Position.Bottom}
gridLine={{ visible: true }}
ticks={2}
style={{
tickLabel: {
padding: 10,
},
axisTitle: {
padding: 0,
},
tickLine: {
visible: false,
},
axisLine: {
visible: false,
},
}}
tickFormat={timeFormatter(niceTimeFormatByDay(numOfDays))}
/>
<Axis
id="y"
title="metric"
position={Position.Left}
gridLine={{ visible: true }}
domain={{
max: 10,
}}
ticks={2}
style={{
axisTitle: {
padding: 0,
},
tickLabel: {
padding: 5,
},
tickLine: {
visible: false,
},
axisLine: {
visible: false,
},
}}
tickFormat={(d) => d.toFixed(2)}
/>
<GroupBy
id="v_split"
by={(spec, { v }) => {
return v;
}}
sort="alphaAsc"
/>
<GroupBy
id="h_split"
by={(spec, { h }) => {
return h;
}}
sort="alphaAsc"
/>
<SmallMultiples
splitVertically="v_split"
splitHorizontally="h_split"
style={{ verticalPanelPadding: [0, 0.3] }}
/>
<LineSeries
id="line"
xScaleType={ScaleType.Time}
yScaleType={ScaleType.Linear}
timeZone="UTC"
xAccessor="x"
yAccessors={['y']}
color={({ smHorizontalAccessorValue, key }) => {
const val = Number(`${smHorizontalAccessorValue}`.split('host ')[1]);
return colors[key] ?? LIGHT_THEME.colors.vizColors[val];
}}
data={data}
/>
</Chart>
);
};
Example.story = {
parameters: {
options: { selectedPanel: SB_SOURCE_PANEL },
info: {
text: `It is possible to add either a vertical and horizontal \`<GroupBy/>\` operations to create a grid of
small multiples.
The assignment of the series colors can be handled by defining an accessor in the \`color\` prop of the series that
consider the \`smHorizontalAccessorValue\` or \`smVerticalAccessorValue\` values when returning the assigned color.
`,
},
},
}; |
|
@nickofthyme thanks for the example, you are right, a situation like this can create issues in the future. |
| insertIndex: i, | ||
| })); | ||
|
|
||
| const smallMultipleUniqueValues = dataSeries.reduce<{ |
There was a problem hiding this comment.
Great general code changes in your PR! Besides replacing the let, also avoids the recreation of the Sets and nicely scopes things into a tight block
@markov00 the first thing that comes to my mind is just a unique string to group the series by, that is not tied to any property. Not sure how that works implementation-wise but thought I'd put it out there. |
|
I tested this in kibana to implement in the One thing I am not so sure about is the multiple |
nickofthyme
left a comment
There was a problem hiding this comment.
Looks good to me, just left a few comments to address and I'll approve. Also tested storybook locally on Chrome and saw no issues.
nickofthyme
left a comment
There was a problem hiding this comment.
The latest code changes look good to me.
# [25.0.0](v24.6.0...v25.0.0) (2021-02-16) ### Bug Fixes * group legend items by label and color ([#999](#999)) ([5d32f23](5d32f23)) ### Features * **axis:** log scale improvements and options ([#1014](#1014)) ([0f52688](0f52688)) ### BREAKING CHANGES * The `LegendActionProps` and the `LegendColorPickerProps`, used to add actions and color picker through the legend now receive an array of `SeriesIdentifiers`
|
🎉 This PR is included in version 25.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
# [25.0.0](elastic/elastic-charts@v24.6.0...v25.0.0) (2021-02-16) ### Bug Fixes * group legend items by label and color ([opensearch-project#999](elastic/elastic-charts#999)) ([ce0bfba](elastic/elastic-charts@ce0bfba)) ### Features * **axis:** log scale improvements and options ([opensearch-project#1014](elastic/elastic-charts#1014)) ([8bac5e8](elastic/elastic-charts@8bac5e8)) ### BREAKING CHANGES * The `LegendActionProps` and the `LegendColorPickerProps`, used to add actions and color picker through the legend now receive an array of `SeriesIdentifiers`


Summary
This PR fixes the duplicated legend elements when displaying small multiples.
The legend item concept should allow the user to uniquely link a geometry color to a label. There are situation where multiple geometries or series are colored with the same color, like in a small multiple, and all of these series are related to the same metric for example. This PR removes any duplicate of the key
color,labelin the legend item and introduce the ability to have multiple series referenced by the same legend item.BREAKING CHANGE:
The
LegendActionPropsand theLegendColorPickerProps, used to add actions and color picker through the legend now receive an array ofSeriesIdentifiers instead of a single one:Checklist
Delete any items that are not applicable to this PR.
src/index.ts(and stories only import from../srcexcept for test data & storybook)