Skip to content

Commit db3faaa

Browse files
committed
[FIX] side_panel: fix open/close logic for nested navigation
Steps to reproduce: - Open the global filter side panel that lists all existing filters - Pin the panel - Click on a specific filter to view/edit Before this commit: - A second side panel was opened instead of drilling down in the current one (unlike CF/DV) - Clicking 'Cancel' on the new panel did nothing - Clicking 'Remove' caused a traceback After this commit: - New side panel opens in the same source panel (main or secondary), Preserving navigation flow - Clicking 'Cancel' or 'Remove' correctly restores the previous side panel in It's a panel slot Task: 4911603
1 parent 29b6458 commit db3faaa

File tree

8 files changed

+87
-14
lines changed

8 files changed

+87
-14
lines changed

src/actions/insert_actions.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ export const insertDropdown: ActionSpec = {
319319
env.openSidePanel("DataValidationEditor", {
320320
rule: localizeDataValidationRule(rule, env.model.getters.getLocale()),
321321
onExit: () => {
322-
env.openSidePanel("DataValidation");
322+
env.openSidePanel("DataValidation", {}, "DataValidationEditor");
323323
},
324324
});
325325
},

src/components/side_panel/pivot/pivot_layout_configurator/pivot_measure/pivot_measure.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,14 @@ export class PivotMeasureEditor extends Component<Props> {
7979
}
8080

8181
openShowValuesAs() {
82-
this.env.openSidePanel("PivotMeasureDisplayPanel", {
83-
pivotId: this.props.pivotId,
84-
measure: this.props.measure,
85-
});
82+
this.env.openSidePanel(
83+
"PivotMeasureDisplayPanel",
84+
{
85+
pivotId: this.props.pivotId,
86+
measure: this.props.measure,
87+
},
88+
`pivot_key_${this.props.pivotId}`
89+
);
8690
}
8791

8892
getColoredSymbolToken(token: Token): Color | undefined {

src/components/side_panel/pivot/pivot_measure_display_panel/pivot_measure_display_panel.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,20 @@ export class PivotMeasureDisplayPanel extends Component<Props, SpreadsheetChildE
5454
}
5555

5656
onSave() {
57-
this.env.openSidePanel("PivotSidePanel", { pivotId: this.props.pivotId });
57+
this.env.openSidePanel(
58+
"PivotSidePanel",
59+
{ pivotId: this.props.pivotId },
60+
`pivot_measure_display_${this.props.pivotId}`
61+
);
5862
}
5963

6064
onCancel() {
6165
this.store.cancelMeasureDisplayEdition();
62-
this.env.openSidePanel("PivotSidePanel", { pivotId: this.props.pivotId });
66+
this.env.openSidePanel(
67+
"PivotSidePanel",
68+
{ pivotId: this.props.pivotId },
69+
`pivot_measure_display_${this.props.pivotId}`
70+
);
6371
}
6472

6573
get fieldChoices() {

src/components/side_panel/side_panel/side_panel_store.ts

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,11 @@ export class SidePanelStore extends SpreadsheetStore {
9999
return undefined;
100100
}
101101

102-
open(componentTag: string, initialPanelProps: SidePanelComponentProps = {}) {
102+
open(
103+
componentTag: string,
104+
initialPanelProps: SidePanelComponentProps = {},
105+
sourcePanelKey?: string
106+
) {
103107
if (this.screenWidthStore.isSmall) {
104108
return;
105109
}
@@ -110,8 +114,12 @@ export class SidePanelStore extends SpreadsheetStore {
110114
return;
111115
}
112116

113-
const mainPanelKey = this.mainPanel ? this.getPanelKey(this.mainPanel) : undefined;
114-
if (!this.mainPanel || !this.mainPanel.isPinned || mainPanelKey === state.key) {
117+
if (sourcePanelKey) {
118+
this._handleSourcePanel(sourcePanelKey, newPanelInfo, state);
119+
return;
120+
}
121+
122+
if (!this.mainPanel || !this.mainPanel.isPinned || this.mainPanelKey === state.key) {
115123
this._openPanel("mainPanel", newPanelInfo, state);
116124
return;
117125
}
@@ -135,6 +143,26 @@ export class SidePanelStore extends SpreadsheetStore {
135143
this._openPanel("secondaryPanel", newPanelInfo, state);
136144
}
137145

146+
private _handleSourcePanel(
147+
sourcePanelKey: string,
148+
newPanelInfo: PanelInfo,
149+
state: OpenSidePanel
150+
) {
151+
const isMainPanel = this.mainPanelKey === state.key;
152+
const isSecondaryPanel = this.secondaryPanelKey === state.key;
153+
if (isMainPanel && this.secondaryPanel) {
154+
this.close();
155+
return;
156+
}
157+
if (isSecondaryPanel && this.secondaryPanel) {
158+
this.closeMainPanel();
159+
this.togglePinPanel();
160+
return;
161+
}
162+
const targetPanel = this.mainPanelKey === sourcePanelKey ? "mainPanel" : "secondaryPanel";
163+
this._openPanel(targetPanel, newPanelInfo, state);
164+
}
165+
138166
private _openPanel(
139167
panel: "mainPanel" | "secondaryPanel",
140168
newPanel: PanelInfo,

src/registries/side_panel_registry.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ sidePanelRegistry.add("PivotMeasureDisplayPanel", {
145145
try {
146146
// This will throw if the pivot or measure does not exist
147147
getters.getPivot(props.pivotId).getMeasure(props.measure.id);
148-
return { isOpen: true, props, key: "pivot_measure_display" };
148+
return { isOpen: true, props, key: `pivot_measure_display_${props.pivotId}` };
149149
} catch (e) {
150150
return { isOpen: false };
151151
}

src/types/env.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export interface SpreadsheetChildEnv extends NotificationStoreMethods {
2323
model: Model;
2424
imageProvider?: ImageProviderInterface;
2525
isDashboard: () => boolean;
26-
openSidePanel: (panel: string, panelProps?: any) => void;
26+
openSidePanel: (panel: string, panelProps?: any, sourcePanelKey?: string) => void;
2727
toggleSidePanel: (panel: string, panelProps?: any) => void;
2828
clipboard: ClipboardInterface;
2929
startCellEdition: (content?: string) => void;

tests/pivots/pivot_measure/pivot_measure_display_panel.test.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,11 @@ describe("Standalone side panel tests", () => {
223223
await mountPanel();
224224
await click(fixture, ".o-pivot-measure-save");
225225

226-
expect(openSidePanelSpy).toHaveBeenCalledWith("PivotSidePanel", { pivotId });
226+
expect(openSidePanelSpy).toHaveBeenCalledWith(
227+
"PivotSidePanel",
228+
{ pivotId },
229+
`pivot_measure_display_${pivotId}`
230+
);
227231
});
228232

229233
test("Can cancel the edition of the measure display", async () => {
@@ -234,7 +238,11 @@ describe("Standalone side panel tests", () => {
234238
expect(getPivotMeasures()[0].display).toEqual({ type: "%_of_grand_total" });
235239

236240
await click(fixture, ".o-pivot-measure-cancel");
237-
expect(openSidePanelSpy).toHaveBeenCalledWith("PivotSidePanel", { pivotId });
241+
expect(openSidePanelSpy).toHaveBeenCalledWith(
242+
"PivotSidePanel",
243+
{ pivotId },
244+
`pivot_measure_display_${pivotId}`
245+
);
238246
expect(getPivotMeasures()[0].display).toEqual(undefined);
239247
});
240248
});

tests/spreadsheet/side_panel_component.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,31 @@ describe("Side Panel", () => {
459459
expect(".o-sidePanelTitle").toHaveText("Custom Panel");
460460
});
461461

462+
test("Reopening pinned main panel from secondary panel closes secondary panel", async () => {
463+
await click(fixture, ".o-pin-panel");
464+
465+
parent.env.openSidePanel("CUSTOM_PANEL_2");
466+
await nextTick();
467+
expect(".o-sidePanel").toHaveCount(2);
468+
469+
parent.env.openSidePanel("CUSTOM_PANEL", {}, "CUSTOM_PANEL_2");
470+
await nextTick();
471+
expect(".o-sidePanel").toHaveCount(1);
472+
expect(".o-sidePanelTitle").toHaveText("Custom Panel");
473+
});
474+
475+
test("Reopening pinned main panel directly does not close secondary panel", async () => {
476+
await click(fixture, ".o-pin-panel");
477+
478+
parent.env.openSidePanel("CUSTOM_PANEL_2");
479+
await nextTick();
480+
expect(".o-sidePanel").toHaveCount(2);
481+
482+
parent.env.openSidePanel("CUSTOM_PANEL", {});
483+
await nextTick();
484+
expect(".o-sidePanel").toHaveCount(2);
485+
});
486+
462487
test("Re-opening the same panel un-collapses it", async () => {
463488
await click(fixture, ".o-pin-panel");
464489
await click(fixture, ".o-collapse-panel");

0 commit comments

Comments
 (0)