Skip to content

Commit b2510d5

Browse files
committed
[FIX] pivot: error on removing max number of columns/rows
When deleting the pivot style numberOfRows or numberOfColumns in the pivot design panel, the pivot would be in error. This was because we didn't handle the cases where the content on the input was not a number, and would set `NaN` values. closes #7384 Task: 5220970 Signed-off-by: Lucas Lefèvre (lul) <[email protected]>
1 parent 65cfa1d commit b2510d5

File tree

3 files changed

+33
-13
lines changed

3 files changed

+33
-13
lines changed

src/components/side_panel/pivot/pivot_side_panel/pivot_design_panel/pivot_design_panel.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { SpreadsheetChildEnv } from "@odoo/o-spreadsheet-engine/types/spreadshee
33
import { Component } from "@odoo/owl";
44
import { Store, useLocalStore } from "../../../../../store_engine";
55
import { PivotStyle, UID } from "../../../../../types";
6+
import { NumberInput } from "../../../../number_input/number_input";
67
import { Checkbox } from "../../../components/checkbox/checkbox";
78
import { Section } from "../../../components/section/section";
89
import { PivotSidePanelStore } from "../pivot_side_panel_store";
@@ -14,14 +15,19 @@ interface Props {
1415
export class PivotDesignPanel extends Component<Props, SpreadsheetChildEnv> {
1516
static template = "o-spreadsheet-PivotDesignPanel";
1617
static props = { pivotId: String };
17-
static components = { Section, Checkbox };
18+
static components = { Section, Checkbox, NumberInput };
1819

1920
store!: Store<PivotSidePanelStore>;
2021

2122
setup() {
2223
this.store = useLocalStore(PivotSidePanelStore, this.props.pivotId, "neverDefer");
2324
}
2425

26+
updatePivotStyleNumberProperty(valueStr: string, key: keyof PivotStyle) {
27+
const value = parseInt(valueStr);
28+
this.store.update({ style: { ...this.pivotStyle, [key]: isNaN(value) ? undefined : value } });
29+
}
30+
2531
updatePivotStyleProperty(key: keyof PivotStyle, value: PivotStyle[keyof PivotStyle]) {
2632
this.store.update({ style: { ...this.pivotStyle, [key]: value } });
2733
}

src/components/side_panel/pivot/pivot_side_panel/pivot_design_panel/pivot_design_panel.xml

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,24 +6,22 @@
66
<div class="row mb-2 align-items-center">
77
<div class="col-6">Max rows:</div>
88
<div class="col-6 d-flex align-items-center">
9-
<input
10-
t-att-value="pivotStyle.numberOfRows ?? ''"
11-
type="number"
12-
class="o-pivot-n-of-rows o-input"
13-
placeholder="e.g. 10"
14-
t-on-change="(ev) => this.updatePivotStyleProperty('numberOfRows', ev.target.valueAsNumber)"
9+
<NumberInput
10+
value="pivotStyle.numberOfRows ?? ''"
11+
class="'o-pivot-n-of-rows'"
12+
placeholder.translate="e.g. 10"
13+
onChange="(value) => this.updatePivotStyleNumberProperty(value, 'numberOfRows')"
1514
/>
1615
</div>
1716
</div>
1817
<div class="row mb-2 align-items-center">
1918
<div class="col-6">Max columns:</div>
2019
<div class="col-6 d-flex align-items-center">
21-
<input
22-
t-att-value="pivotStyle.numberOfColumns ?? ''"
23-
type="number"
24-
class="o-pivot-n-of-columns o-input"
25-
placeholder="e.g. 5"
26-
t-on-change="(ev) => this.updatePivotStyleProperty('numberOfColumns', ev.target.valueAsNumber)"
20+
<NumberInput
21+
value="pivotStyle.numberOfColumns ?? ''"
22+
class="'o-pivot-n-of-columns'"
23+
placeholder.translate="e.g. 5"
24+
onChange="(value) => this.updatePivotStyleNumberProperty(value, 'numberOfColumns')"
2725
/>
2826
</div>
2927
</div>

tests/pivots/pivot_design_side_panel.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,4 +94,20 @@ describe("Spreadsheet pivot side panel", () => {
9494
await setInputValueAndTrigger("input.o-pivot-n-of-rows", "12");
9595
expect(model.getters.getPivotCoreDefinition("1").style?.numberOfRows).toBe(12);
9696
});
97+
98+
test("Editing a number input with a non-number value make the property undefined and not NaN", async () => {
99+
updatePivot(model, "1", { style: { numberOfRows: 5, numberOfColumns: 10 } });
100+
await nextTick();
101+
102+
jest.useFakeTimers();
103+
setInputValueAndTrigger("input.o-pivot-n-of-rows", "olà");
104+
setInputValueAndTrigger("input.o-pivot-n-of-columns", "");
105+
jest.advanceTimersByTime(1000);
106+
107+
expect(model.getters.getPivotCoreDefinition("1").style).toEqual({
108+
numberOfRows: undefined,
109+
numberOfColumns: undefined,
110+
});
111+
jest.useRealTimers();
112+
});
97113
});

0 commit comments

Comments
 (0)