Skip to content

Commit

Permalink
fix: dialog clobbered on multiple requests
Browse files Browse the repository at this point in the history
When multiple dialogs are requested to be shown, they now wait for the previous dialog to be closed and cleaned up.
  • Loading branch information
samuelmaddock committed Nov 23, 2024
1 parent a7fcf2c commit da4be65
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 5 deletions.
31 changes: 27 additions & 4 deletions src/renderer/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
computed,
makeObservable,
observable,
runInAction,
when,
} from 'mobx';

Expand Down Expand Up @@ -975,12 +976,34 @@ export class AppState {
public async showGenericDialog(
opts: GenericDialogOptions,
): Promise<{ confirm: boolean; input: string }> {
this.genericDialogLastResult = null;
this.genericDialogOptions = opts;
this.isGenericDialogShowing = true;
// Wait for any existing dialog to be closed and cleaned up.
await when(() => {
if (
!this.isGenericDialogShowing &&
this.genericDialogLastResult === null
) {
// Set dialog immediately to prevent any other queued dialogs from
// showing.
runInAction(() => {
this.genericDialogOptions = opts;
this.isGenericDialogShowing = true;
});
return true;
}
return false;
});

// Wait for dialog to be closed.
await when(() => !this.isGenericDialogShowing);

// Cleanup to allow queued dialog to show.
const result = this.genericDialogLastResult;
runInAction(() => {
this.genericDialogLastResult = null;
});

return {
confirm: Boolean(this.genericDialogLastResult),
confirm: Boolean(result),
input: this.genericDialogLastInput || opts.defaultInput || '',
};
}
Expand Down
49 changes: 48 additions & 1 deletion tests/renderer/state-spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { mocked } from 'jest-mock';
import { IReactionDisposer, reaction } from 'mobx';
import { IReactionDisposer, reaction, runInAction, when } from 'mobx';

import {
AppStateBroadcastMessageType,
Expand Down Expand Up @@ -585,6 +585,53 @@ describe('AppState', () => {
const result = await appState.showGenericDialog(Opts);
expect(result).toHaveProperty('input', '');
});

it('queues dialogs', async () => {
const optsA = {
label: 'A',
ok: 'Close',
type: GenericDialogType.warning,
wantsInput: false,
} as const;

const optsB = {
label: 'B',
ok: 'Close',
type: GenericDialogType.warning,
wantsInput: false,
} as const;

// Show Dialog A
const dialogPromiseA = appState.showGenericDialog(optsA);
await when(() => appState.genericDialogOptions?.label === optsA.label);

// Queue Dialog B
const dialogPromiseB = appState.showGenericDialog(optsB);
expect(appState).toHaveProperty('isGenericDialogShowing', true);
expect(appState.genericDialogOptions?.label).toEqual(optsA.label);

// Close Dialog A
runInAction(() => {
appState.genericDialogLastResult = true;
appState.isGenericDialogShowing = false;
});
expect(appState.genericDialogOptions?.label).toEqual(optsA.label);
await dialogPromiseA;

// Expect dialog set to Dialog B
expect(appState).toHaveProperty('isGenericDialogShowing', true);
expect(appState.genericDialogOptions?.label).toEqual(optsB.label);

// Close Dialog B
runInAction(() => {
appState.genericDialogLastResult = true;
appState.isGenericDialogShowing = false;
});
await dialogPromiseB;

// No remaining dialogs queued
expect(appState).toHaveProperty('isGenericDialogShowing', false);
});
});

describe('showInputDialog', () => {
Expand Down

0 comments on commit da4be65

Please sign in to comment.