Skip to content

Commit a273e64

Browse files
bminglesbmingles
authored
fix: Prompt for resetting layout (#1552)
User is not prompted to confirm when using the "reset layout" feature. - With unsaved notebook changes, prompt title will be "Reset layout and discard unsaved changes" - With open notebooks that are saved or no open notebooks, prompt title will be "Reset Layout" - If user clicks "Reset", layout will be reset. Unsaved notebook changes will be lost - If user clicks "Cancel", layout will not be reset and notebooks remain open fixes #1250
1 parent 4441109 commit a273e64

4 files changed

Lines changed: 138 additions & 33 deletions

File tree

‎packages/code-studio/src/main/AppMainContainer.tsx‎

Lines changed: 46 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import {
4747
Link,
4848
ColumnSelectionValidator,
4949
getDashboardConnection,
50+
NotebookPanel,
5051
} from '@deephaven/dashboard-core-plugins';
5152
import {
5253
vsGear,
@@ -77,7 +78,7 @@ import {
7778
ServerConfigValues,
7879
DeephavenPluginModuleMap,
7980
} from '@deephaven/redux';
80-
import { PromiseUtils } from '@deephaven/utils';
81+
import { bindAllMethods, PromiseUtils } from '@deephaven/utils';
8182
import GoldenLayout from '@deephaven/golden-layout';
8283
import type { ItemConfigType } from '@deephaven/golden-layout';
8384
import {
@@ -139,7 +140,9 @@ interface AppMainContainerState {
139140
isAuthFailed: boolean;
140141
isDisconnected: boolean;
141142
isPanelsMenuShown: boolean;
143+
isResetLayoutPromptShown: boolean;
142144
isSettingsMenuShown: boolean;
145+
unsavedNotebookCount: number;
143146
widgets: VariableDefinition[];
144147
}
145148

@@ -176,29 +179,8 @@ export class AppMainContainer extends Component<
176179

177180
constructor(props: AppMainContainerProps & RouteComponentProps) {
178181
super(props);
179-
this.handleSettingsMenuHide = this.handleSettingsMenuHide.bind(this);
180-
this.handleSettingsMenuShow = this.handleSettingsMenuShow.bind(this);
181-
this.handleError = this.handleError.bind(this);
182-
this.handleControlSelect = this.handleControlSelect.bind(this);
183-
this.handleToolSelect = this.handleToolSelect.bind(this);
184-
this.handleClearFilter = this.handleClearFilter.bind(this);
185-
this.handleDataChange = this.handleDataChange.bind(this);
186-
this.handleAutoFillClick = this.handleAutoFillClick.bind(this);
187-
this.handleGoldenLayoutChange = this.handleGoldenLayoutChange.bind(this);
188-
this.handleLayoutConfigChange = this.handleLayoutConfigChange.bind(this);
189-
this.handleExportLayoutClick = this.handleExportLayoutClick.bind(this);
190-
this.handleImportLayoutClick = this.handleImportLayoutClick.bind(this);
191-
this.handleImportLayoutFiles = this.handleImportLayoutFiles.bind(this);
192-
this.handleResetLayoutClick = this.handleResetLayoutClick.bind(this);
193-
this.handleWidgetMenuClick = this.handleWidgetMenuClick.bind(this);
194-
this.handleWidgetsMenuClose = this.handleWidgetsMenuClose.bind(this);
195-
this.handleWidgetSelect = this.handleWidgetSelect.bind(this);
196-
this.handlePaste = this.handlePaste.bind(this);
197-
this.hydrateDefault = this.hydrateDefault.bind(this);
198-
this.openNotebookFromURL = this.openNotebookFromURL.bind(this);
199-
this.handleDisconnect = this.handleDisconnect.bind(this);
200-
this.handleReconnect = this.handleReconnect.bind(this);
201-
this.handleReconnectAuthFailed = this.handleReconnectAuthFailed.bind(this);
182+
183+
bindAllMethods(this);
202184

203185
this.importElement = React.createRef();
204186

@@ -231,7 +213,9 @@ export class AppMainContainer extends Component<
231213
isAuthFailed: false,
232214
isDisconnected: false,
233215
isPanelsMenuShown: false,
216+
isResetLayoutPromptShown: false,
234217
isSettingsMenuShown: false,
218+
unsavedNotebookCount: 0,
235219
widgets: [],
236220
};
237221
}
@@ -347,6 +331,20 @@ export class AppMainContainer extends Component<
347331
this.goldenLayout?.eventHub.emit(event, ...args);
348332
}
349333

334+
handleCancelResetLayoutPrompt(): void {
335+
this.setState({
336+
isResetLayoutPromptShown: false,
337+
});
338+
}
339+
340+
handleConfirmResetLayoutPrompt(): void {
341+
this.setState({
342+
isResetLayoutPromptShown: false,
343+
});
344+
345+
this.resetLayout();
346+
}
347+
350348
// eslint-disable-next-line class-methods-use-this
351349
handleError(error: unknown): void {
352350
if (PromiseUtils.isCanceled(error)) {
@@ -517,9 +515,11 @@ export class AppMainContainer extends Component<
517515
handleResetLayoutClick(): void {
518516
log.info('handleResetLayoutClick');
519517

520-
this.setState({ isPanelsMenuShown: false });
521-
522-
this.resetLayout();
518+
this.setState({
519+
isPanelsMenuShown: false,
520+
isResetLayoutPromptShown: true,
521+
unsavedNotebookCount: NotebookPanel.unsavedNotebookCount(),
522+
});
523523
}
524524

525525
handleImportLayoutFiles(event: ChangeEvent<HTMLInputElement>): void {
@@ -718,7 +718,9 @@ export class AppMainContainer extends Component<
718718
isAuthFailed,
719719
isDisconnected,
720720
isPanelsMenuShown,
721+
isResetLayoutPromptShown,
721722
isSettingsMenuShown,
723+
unsavedNotebookCount,
722724
widgets,
723725
} = this.state;
724726
const dashboardPlugins = this.getDashboardPlugins(plugins);
@@ -884,6 +886,23 @@ export class AppMainContainer extends Component<
884886
subtitle="Please check your network connection."
885887
/>
886888
</DebouncedModal>
889+
<BasicModal
890+
confirmButtonText="Reset"
891+
onConfirm={this.handleConfirmResetLayoutPrompt}
892+
onCancel={this.handleCancelResetLayoutPrompt}
893+
isConfirmDanger
894+
isOpen={isResetLayoutPromptShown}
895+
headerText={
896+
unsavedNotebookCount === 0
897+
? 'Reset Layout'
898+
: 'Reset layout and discard unsaved changes'
899+
}
900+
bodyText={
901+
unsavedNotebookCount === 0
902+
? 'Do you want to reset your layout? Your existing layout will be lost.'
903+
: 'Do you want to reset your layout? Any unsaved notebooks will be lost.'
904+
}
905+
/>
887906
<BasicModal
888907
confirmButtonText="Refresh"
889908
onConfirm={AppMainContainer.handleRefresh}

‎packages/dashboard-core-plugins/src/panels/NotebookPanel.test.tsx‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import NotebookPanel from './NotebookPanel';
2+
3+
beforeEach(() => {
4+
document.body.innerHTML = '';
5+
jest.clearAllMocks();
6+
expect.hasAssertions();
7+
});
8+
9+
describe('unsavedNotebookCount', () => {
10+
function mockPanel(...classNames: string[]): HTMLDivElement {
11+
const el = document.createElement('div');
12+
el.className = classNames.join(' ');
13+
return el;
14+
}
15+
16+
const panel = {
17+
random: mockPanel('some-random-class'),
18+
saved: mockPanel(NotebookPanel.UNSAVED_INDICATOR_CLASS_NAME),
19+
statusOnly: mockPanel(NotebookPanel.UNSAVED_STATUS_CLASS_NAME),
20+
unsaved: mockPanel(
21+
NotebookPanel.UNSAVED_INDICATOR_CLASS_NAME,
22+
NotebookPanel.UNSAVED_STATUS_CLASS_NAME
23+
),
24+
};
25+
26+
it.each([
27+
[[], 0],
28+
[[panel.random], 0],
29+
[[panel.saved], 0],
30+
[[panel.statusOnly], 0],
31+
[[panel.unsaved], 1],
32+
[[panel.unsaved, panel.unsaved], 2],
33+
[[panel.unsaved, panel.unsaved, panel.saved], 2],
34+
] as const)(
35+
'should return the count of unsaved notebooks: %s, %s',
36+
(panels, expectedCount) => {
37+
panels.forEach(p => document.body.appendChild(p.cloneNode()));
38+
expect(NotebookPanel.unsavedNotebookCount()).toBe(expectedCount);
39+
}
40+
);
41+
});

‎packages/dashboard-core-plugins/src/panels/NotebookPanel.tsx‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,13 +130,26 @@ class NotebookPanel extends Component<NotebookPanelProps, NotebookPanelState> {
130130

131131
static DEFAULT_NAME = 'Untitled';
132132

133+
static UNSAVED_INDICATOR_CLASS_NAME = 'editor-unsaved-indicator';
134+
135+
static UNSAVED_STATUS_CLASS_NAME = 'is-unsaved';
136+
133137
static handleError(error: unknown): void {
134138
if (PromiseUtils.isCanceled(error)) {
135139
return;
136140
}
137141
log.error(error);
138142
}
139143

144+
/**
145+
* Returns number of unsaved notebooks.
146+
*/
147+
static unsavedNotebookCount(): number {
148+
return document.querySelectorAll(
149+
`.${NotebookPanel.UNSAVED_INDICATOR_CLASS_NAME}.${NotebookPanel.UNSAVED_STATUS_CLASS_NAME}`
150+
).length;
151+
}
152+
140153
static defaultProps = {
141154
isDashboardActive: true,
142155
isPreview: false,
@@ -1243,9 +1256,13 @@ class NotebookPanel extends Component<NotebookPanelProps, NotebookPanelState> {
12431256
{portal != null &&
12441257
ReactDOM.createPortal(
12451258
<span
1246-
className={classNames('editor-unsaved-indicator', {
1247-
'is-unsaved': changeCount !== savedChangeCount,
1248-
})}
1259+
className={classNames(
1260+
NotebookPanel.UNSAVED_INDICATOR_CLASS_NAME,
1261+
{
1262+
[NotebookPanel.UNSAVED_STATUS_CLASS_NAME]:
1263+
changeCount !== savedChangeCount,
1264+
}
1265+
)}
12491266
/>,
12501267
portal // tab.element is jquery element, we want a dom element
12511268
)}

‎tests/golden-layout.spec.ts‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,37 @@ test.describe('tests golden-layout operations', () => {
2626
});
2727

2828
test.afterAll(async () => {
29-
// reset layout
30-
await page.getByTestId('app-main-panels-button').click();
31-
await page.getByLabel('Reset Layout').click();
29+
/**
30+
* Open panels menu, reset layout, confirm or cancel "Reset Layout" prompt
31+
*/
32+
async function resetLayout(confirm: boolean) {
33+
await page.getByTestId('app-main-panels-button').click();
34+
await page.getByLabel('Reset Layout').click();
35+
36+
if (confirm) {
37+
await page
38+
.locator('.modal .btn-danger')
39+
.filter({ hasText: 'Reset' })
40+
.click();
41+
} else {
42+
await page
43+
.locator('[data-dismiss=modal]')
44+
.filter({ hasText: 'Cancel' })
45+
.click();
46+
}
47+
48+
await expect(page.locator('.modal')).toHaveCount(0);
49+
}
50+
51+
// Reset layout cancelled by user
52+
await resetLayout(false);
53+
54+
await expect(
55+
page.locator('.lm_tab').filter({ has: page.getByText('test-a') })
56+
).toHaveCount(1);
57+
58+
// Reset layout confirmed by user
59+
await resetLayout(true);
3260

3361
await expect(
3462
page.locator('.lm_tab').filter({ has: page.getByText('test-a') })

0 commit comments

Comments
 (0)
⚡