Skip to content

Commit f6018f7

Browse files
kibanamachineWylie Conlon
andauthored
[Lens] Fix pie chart with 0 decimal places for percent (#105672) (#105787)
Co-authored-by: Wylie Conlon <william.conlon@elastic.co>
1 parent 43cab2d commit f6018f7

File tree

5 files changed

+41
-17
lines changed

5 files changed

+41
-17
lines changed

x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ export function TableDimensionEditor(
8484
onChange: onSummaryLabelChangeToDebounce,
8585
value: column?.summaryLabel,
8686
},
87-
{ allowEmptyString: true } // empty string is a valid label for this feature
87+
{ allowFalsyValue: true } // falsy values are valid for this feature
8888
);
8989

9090
if (!column) return null;

x-pack/plugins/lens/public/pie_visualization/toolbar.tsx

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,12 +182,12 @@ export function PieToolbar(props: VisualizationToolbarProps<PieVisualizationStat
182182
>
183183
<DecimalPlaceSlider
184184
value={layer.percentDecimals ?? DEFAULT_PERCENT_DECIMALS}
185-
setValue={(value) =>
185+
setValue={(value) => {
186186
setState({
187187
...state,
188188
layers: [{ ...layer, percentDecimals: value }],
189-
})
190-
}
189+
});
190+
}}
191191
/>
192192
</EuiFormRow>
193193
</ToolbarPopover>
@@ -232,7 +232,13 @@ const DecimalPlaceSlider = ({
232232
value: number;
233233
setValue: (value: number) => void;
234234
}) => {
235-
const { inputValue, handleInputChange } = useDebouncedValue({ value, onChange: setValue });
235+
const { inputValue, handleInputChange } = useDebouncedValue(
236+
{
237+
value,
238+
onChange: setValue,
239+
},
240+
{ allowFalsyValue: true }
241+
);
236242
return (
237243
<EuiRange
238244
data-test-subj="indexPattern-dimension-formatDecimals"

x-pack/plugins/lens/public/shared_components/debounced_value.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ describe('useDebouncedValue', () => {
4343
it('should allow empty input to be updated', () => {
4444
const onChangeMock = jest.fn();
4545
const { result } = renderHook(() =>
46-
useDebouncedValue({ value: 'a', onChange: onChangeMock }, { allowEmptyString: true })
46+
useDebouncedValue({ value: 'a', onChange: onChangeMock }, { allowFalsyValue: true })
4747
);
4848

4949
act(() => {

x-pack/plugins/lens/public/shared_components/debounced_value.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ export const useDebouncedValue = <T>(
2121
onChange: (val: T) => void;
2222
value: T;
2323
},
24-
{ allowEmptyString }: { allowEmptyString?: boolean } = {}
24+
{ allowFalsyValue }: { allowFalsyValue?: boolean } = {}
2525
) => {
2626
const [inputValue, setInputValue] = useState(value);
2727
const unflushedChanges = useRef(false);
28-
const shouldUpdateWithEmptyString = Boolean(allowEmptyString);
28+
const shouldUpdateWithFalsyValue = Boolean(allowFalsyValue);
2929

3030
// Save the initial value
3131
const initialValue = useRef(value);
@@ -57,7 +57,7 @@ export const useDebouncedValue = <T>(
5757

5858
const handleInputChange = (val: T) => {
5959
setInputValue(val);
60-
const valueToUpload = shouldUpdateWithEmptyString
60+
const valueToUpload = shouldUpdateWithFalsyValue
6161
? val ?? initialValue.current
6262
: val || initialValue.current;
6363
onChangeDebounced(valueToUpload);

x-pack/test/functional/apps/lens/chart_data.ts

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,15 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
4545
{ x: 'Other', y: 5722.77 },
4646
];
4747

48+
const expectedPieData = [
49+
{ name: '97.220.3.248', value: 19755 },
50+
{ name: '169.228.188.120', value: 18994 },
51+
{ name: '78.83.247.30', value: 17246 },
52+
{ name: '226.82.228.233', value: 15687 },
53+
{ name: '93.28.27.24', value: 15614.33 },
54+
{ name: '__other__', value: 5722.77 },
55+
];
56+
4857
function assertMatchesExpectedData(state: DebugState) {
4958
expect(
5059
state.bars![0].bars.map((bar) => ({
@@ -54,32 +63,41 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
5463
).to.eql(expectedData);
5564
}
5665

66+
function assertMatchesExpectedPieData(state: DebugState) {
67+
expect(
68+
state
69+
.partition![0].partitions.map((partition) => ({
70+
name: partition.name,
71+
value: Math.floor(partition.value * 100) / 100,
72+
}))
73+
.sort(({ value: a }, { value: b }) => b - a)
74+
).to.eql(expectedPieData);
75+
}
76+
5777
it('should render xy chart', async () => {
5878
const data = await PageObjects.lens.getCurrentChartDebugState();
5979
assertMatchesExpectedData(data!);
6080
});
6181

62-
// Partition chart tests have to be skipped until
63-
// https://github.com/elastic/elastic-charts/issues/917 gets fixed
64-
it.skip('should render pie chart', async () => {
82+
it('should render pie chart', async () => {
6583
await PageObjects.lens.switchToVisualization('pie');
6684
await PageObjects.lens.waitForVisualization();
6785
const data = await PageObjects.lens.getCurrentChartDebugState();
68-
assertMatchesExpectedData(data!);
86+
assertMatchesExpectedPieData(data!);
6987
});
7088

71-
it.skip('should render donut chart', async () => {
89+
it('should render donut chart', async () => {
7290
await PageObjects.lens.switchToVisualization('donut');
7391
await PageObjects.lens.waitForVisualization();
7492
const data = await PageObjects.lens.getCurrentChartDebugState();
75-
assertMatchesExpectedData(data!);
93+
assertMatchesExpectedPieData(data!);
7694
});
7795

78-
it.skip('should render treemap chart', async () => {
96+
it('should render treemap chart', async () => {
7997
await PageObjects.lens.switchToVisualization('treemap', 'treemap');
8098
await PageObjects.lens.waitForVisualization();
8199
const data = await PageObjects.lens.getCurrentChartDebugState();
82-
assertMatchesExpectedData(data!);
100+
assertMatchesExpectedPieData(data!);
83101
});
84102

85103
it('should render heatmap chart', async () => {

0 commit comments

Comments
 (0)