Skip to content

Commit

Permalink
fix: Prompt for resetting layout (deephaven#1552)
Browse files Browse the repository at this point in the history
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 deephaven#1250
  • Loading branch information
bmingles authored Sep 28, 2023
1 parent 4441109 commit a273e64
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 33 deletions.
73 changes: 46 additions & 27 deletions packages/code-studio/src/main/AppMainContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import {
Link,
ColumnSelectionValidator,
getDashboardConnection,
NotebookPanel,
} from '@deephaven/dashboard-core-plugins';
import {
vsGear,
Expand Down Expand Up @@ -77,7 +78,7 @@ import {
ServerConfigValues,
DeephavenPluginModuleMap,
} from '@deephaven/redux';
import { PromiseUtils } from '@deephaven/utils';
import { bindAllMethods, PromiseUtils } from '@deephaven/utils';
import GoldenLayout from '@deephaven/golden-layout';
import type { ItemConfigType } from '@deephaven/golden-layout';
import {
Expand Down Expand Up @@ -139,7 +140,9 @@ interface AppMainContainerState {
isAuthFailed: boolean;
isDisconnected: boolean;
isPanelsMenuShown: boolean;
isResetLayoutPromptShown: boolean;
isSettingsMenuShown: boolean;
unsavedNotebookCount: number;
widgets: VariableDefinition[];
}

Expand Down Expand Up @@ -176,29 +179,8 @@ export class AppMainContainer extends Component<

constructor(props: AppMainContainerProps & RouteComponentProps) {
super(props);
this.handleSettingsMenuHide = this.handleSettingsMenuHide.bind(this);
this.handleSettingsMenuShow = this.handleSettingsMenuShow.bind(this);
this.handleError = this.handleError.bind(this);
this.handleControlSelect = this.handleControlSelect.bind(this);
this.handleToolSelect = this.handleToolSelect.bind(this);
this.handleClearFilter = this.handleClearFilter.bind(this);
this.handleDataChange = this.handleDataChange.bind(this);
this.handleAutoFillClick = this.handleAutoFillClick.bind(this);
this.handleGoldenLayoutChange = this.handleGoldenLayoutChange.bind(this);
this.handleLayoutConfigChange = this.handleLayoutConfigChange.bind(this);
this.handleExportLayoutClick = this.handleExportLayoutClick.bind(this);
this.handleImportLayoutClick = this.handleImportLayoutClick.bind(this);
this.handleImportLayoutFiles = this.handleImportLayoutFiles.bind(this);
this.handleResetLayoutClick = this.handleResetLayoutClick.bind(this);
this.handleWidgetMenuClick = this.handleWidgetMenuClick.bind(this);
this.handleWidgetsMenuClose = this.handleWidgetsMenuClose.bind(this);
this.handleWidgetSelect = this.handleWidgetSelect.bind(this);
this.handlePaste = this.handlePaste.bind(this);
this.hydrateDefault = this.hydrateDefault.bind(this);
this.openNotebookFromURL = this.openNotebookFromURL.bind(this);
this.handleDisconnect = this.handleDisconnect.bind(this);
this.handleReconnect = this.handleReconnect.bind(this);
this.handleReconnectAuthFailed = this.handleReconnectAuthFailed.bind(this);

bindAllMethods(this);

this.importElement = React.createRef();

Expand Down Expand Up @@ -231,7 +213,9 @@ export class AppMainContainer extends Component<
isAuthFailed: false,
isDisconnected: false,
isPanelsMenuShown: false,
isResetLayoutPromptShown: false,
isSettingsMenuShown: false,
unsavedNotebookCount: 0,
widgets: [],
};
}
Expand Down Expand Up @@ -347,6 +331,20 @@ export class AppMainContainer extends Component<
this.goldenLayout?.eventHub.emit(event, ...args);
}

handleCancelResetLayoutPrompt(): void {
this.setState({
isResetLayoutPromptShown: false,
});
}

handleConfirmResetLayoutPrompt(): void {
this.setState({
isResetLayoutPromptShown: false,
});

this.resetLayout();
}

// eslint-disable-next-line class-methods-use-this
handleError(error: unknown): void {
if (PromiseUtils.isCanceled(error)) {
Expand Down Expand Up @@ -517,9 +515,11 @@ export class AppMainContainer extends Component<
handleResetLayoutClick(): void {
log.info('handleResetLayoutClick');

this.setState({ isPanelsMenuShown: false });

this.resetLayout();
this.setState({
isPanelsMenuShown: false,
isResetLayoutPromptShown: true,
unsavedNotebookCount: NotebookPanel.unsavedNotebookCount(),
});
}

handleImportLayoutFiles(event: ChangeEvent<HTMLInputElement>): void {
Expand Down Expand Up @@ -718,7 +718,9 @@ export class AppMainContainer extends Component<
isAuthFailed,
isDisconnected,
isPanelsMenuShown,
isResetLayoutPromptShown,
isSettingsMenuShown,
unsavedNotebookCount,
widgets,
} = this.state;
const dashboardPlugins = this.getDashboardPlugins(plugins);
Expand Down Expand Up @@ -884,6 +886,23 @@ export class AppMainContainer extends Component<
subtitle="Please check your network connection."
/>
</DebouncedModal>
<BasicModal
confirmButtonText="Reset"
onConfirm={this.handleConfirmResetLayoutPrompt}
onCancel={this.handleCancelResetLayoutPrompt}
isConfirmDanger
isOpen={isResetLayoutPromptShown}
headerText={
unsavedNotebookCount === 0
? 'Reset Layout'
: 'Reset layout and discard unsaved changes'
}
bodyText={
unsavedNotebookCount === 0
? 'Do you want to reset your layout? Your existing layout will be lost.'
: 'Do you want to reset your layout? Any unsaved notebooks will be lost.'
}
/>
<BasicModal
confirmButtonText="Refresh"
onConfirm={AppMainContainer.handleRefresh}
Expand Down
41 changes: 41 additions & 0 deletions packages/dashboard-core-plugins/src/panels/NotebookPanel.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import NotebookPanel from './NotebookPanel';

beforeEach(() => {
document.body.innerHTML = '';
jest.clearAllMocks();
expect.hasAssertions();
});

describe('unsavedNotebookCount', () => {
function mockPanel(...classNames: string[]): HTMLDivElement {
const el = document.createElement('div');
el.className = classNames.join(' ');
return el;
}

const panel = {
random: mockPanel('some-random-class'),
saved: mockPanel(NotebookPanel.UNSAVED_INDICATOR_CLASS_NAME),
statusOnly: mockPanel(NotebookPanel.UNSAVED_STATUS_CLASS_NAME),
unsaved: mockPanel(
NotebookPanel.UNSAVED_INDICATOR_CLASS_NAME,
NotebookPanel.UNSAVED_STATUS_CLASS_NAME
),
};

it.each([
[[], 0],
[[panel.random], 0],
[[panel.saved], 0],
[[panel.statusOnly], 0],
[[panel.unsaved], 1],
[[panel.unsaved, panel.unsaved], 2],
[[panel.unsaved, panel.unsaved, panel.saved], 2],
] as const)(
'should return the count of unsaved notebooks: %s, %s',
(panels, expectedCount) => {
panels.forEach(p => document.body.appendChild(p.cloneNode()));
expect(NotebookPanel.unsavedNotebookCount()).toBe(expectedCount);
}
);
});
23 changes: 20 additions & 3 deletions packages/dashboard-core-plugins/src/panels/NotebookPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,26 @@ class NotebookPanel extends Component<NotebookPanelProps, NotebookPanelState> {

static DEFAULT_NAME = 'Untitled';

static UNSAVED_INDICATOR_CLASS_NAME = 'editor-unsaved-indicator';

static UNSAVED_STATUS_CLASS_NAME = 'is-unsaved';

static handleError(error: unknown): void {
if (PromiseUtils.isCanceled(error)) {
return;
}
log.error(error);
}

/**
* Returns number of unsaved notebooks.
*/
static unsavedNotebookCount(): number {
return document.querySelectorAll(
`.${NotebookPanel.UNSAVED_INDICATOR_CLASS_NAME}.${NotebookPanel.UNSAVED_STATUS_CLASS_NAME}`
).length;
}

static defaultProps = {
isDashboardActive: true,
isPreview: false,
Expand Down Expand Up @@ -1243,9 +1256,13 @@ class NotebookPanel extends Component<NotebookPanelProps, NotebookPanelState> {
{portal != null &&
ReactDOM.createPortal(
<span
className={classNames('editor-unsaved-indicator', {
'is-unsaved': changeCount !== savedChangeCount,
})}
className={classNames(
NotebookPanel.UNSAVED_INDICATOR_CLASS_NAME,
{
[NotebookPanel.UNSAVED_STATUS_CLASS_NAME]:
changeCount !== savedChangeCount,
}
)}
/>,
portal // tab.element is jquery element, we want a dom element
)}
Expand Down
34 changes: 31 additions & 3 deletions tests/golden-layout.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,37 @@ test.describe('tests golden-layout operations', () => {
});

test.afterAll(async () => {
// reset layout
await page.getByTestId('app-main-panels-button').click();
await page.getByLabel('Reset Layout').click();
/**
* Open panels menu, reset layout, confirm or cancel "Reset Layout" prompt
*/
async function resetLayout(confirm: boolean) {
await page.getByTestId('app-main-panels-button').click();
await page.getByLabel('Reset Layout').click();

if (confirm) {
await page
.locator('.modal .btn-danger')
.filter({ hasText: 'Reset' })
.click();
} else {
await page
.locator('[data-dismiss=modal]')
.filter({ hasText: 'Cancel' })
.click();
}

await expect(page.locator('.modal')).toHaveCount(0);
}

// Reset layout cancelled by user
await resetLayout(false);

await expect(
page.locator('.lm_tab').filter({ has: page.getByText('test-a') })
).toHaveCount(1);

// Reset layout confirmed by user
await resetLayout(true);

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

0 comments on commit a273e64

Please sign in to comment.