Skip to content

Commit

Permalink
Fix save as without changing the file name (#14212)
Browse files Browse the repository at this point in the history
* Fixes saving with the same name

* Review

* Add test for dismissed save as dialog

---------

Co-authored-by: Frédéric Collonval <fcollonval@gmail.com>
  • Loading branch information
hbcarlos and fcollonval committed Mar 17, 2023
1 parent 02c087f commit b80523f
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 10 deletions.
2 changes: 1 addition & 1 deletion packages/docmanager-extension/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,7 @@ function addCommands(
args.newValue &&
args.newValue.path !== context.path
) {
void docManager.closeFile(context.path);
void commands.execute(CommandIDs.open, {
path: args.newValue.path
});
Expand All @@ -977,7 +978,6 @@ function addCommands(
docManager.services.contents.fileChanged.connect(onChange);
context
.saveAs()
.then(() => void docManager.closeFile(context.path))
.finally(() =>
docManager.services.contents.fileChanged.disconnect(onChange)
);
Expand Down
5 changes: 1 addition & 4 deletions packages/docregistry/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,7 @@ export class Context<
const localPath = this._manager.contents.localPath(this.path);
const newLocalPath = await Private.getSavePath(localPath);

if (!newLocalPath) {
return Promise.reject('Save as cancelled by user.');
}
if (this.isDisposed) {
if (this.isDisposed || !newLocalPath) {
return;
}

Expand Down
29 changes: 24 additions & 5 deletions packages/docregistry/test/context.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { Contents, ServiceManager } from '@jupyterlab/services';
import {
acceptDialog,
dismissDialog,
signalToPromise,
waitForDialog
} from '@jupyterlab/testing';
import { ServiceManagerMock } from '@jupyterlab/services/lib/testutils';
Expand Down Expand Up @@ -366,18 +367,26 @@ describe('docregistry/context', () => {
await waitForDialog();
const dialog = document.body.getElementsByClassName('jp-Dialog')[0];
const input = dialog.getElementsByTagName('input')[0];

input.value = newPath;
await acceptDialog();
};
const promise = func();
await initialize;

const changed = signalToPromise(manager.contents.fileChanged);
const oldPath = context.path;
await context.saveAs();
await promise;

// We no longer rename the current document
//expect(context.path).toBe(newPath);

// Make sure the signal emitted has a different path
const res = await changed;
expect(res[1].type).toBe('save');
expect(res[1].newValue?.path).toEqual(newPath);
expect(res[1].newValue?.path !== oldPath).toBe(true);

// Make sure the both files are there now.
const model = await manager.contents.get('', { content: true });
expect(model.content.find((x: any) => x.name === oldPath)).toBeTruthy();
Expand Down Expand Up @@ -449,21 +458,31 @@ describe('docregistry/context', () => {
});

it('should just save if the file name does not change', async () => {
const changed = signalToPromise(manager.contents.fileChanged);

const path = context.path;
await context.initialize(true);
const promise = context.saveAs();
await acceptDialog();
await promise;
expect(context.path).toBe(path);

const res = await changed;
expect(res[1].newValue?.path).toEqual(path);
});

it('should be rejected if the user cancel the dialog', async () => {
it('should no trigger save signal if the user cancel the dialog', async () => {
let saveEmitted = false;
await context.initialize(true);

manager.contents.fileChanged.connect((sender, args) => {
if (args.type === 'save') {
saveEmitted = true;
}
});
const promise = context.saveAs();
await dismissDialog();

await expect(promise).rejects.toEqual('Save as cancelled by user.');
await promise;
expect(saveEmitted).toEqual(false);
});
});

Expand Down

0 comments on commit b80523f

Please sign in to comment.