After closing the overwrite modal, reload button no longer works

[unnamedplay-r]

How to replicate:
Save the notebook, change the notebook in an editor then try to save. The notebook will trigger the modal. Once open, click cancel then save again to open the modal. The 'Reload' button no longer works.

Issue:
The event handler binded to it originally was not binded again after the modal is removed.

I can tackle this, I just wanted to create an issue to reference when I submit a pull request.

[takluyver]

I can still reproduce this on master. @unnamedplay-r are you still interested in making a pull request?

You can find the code which displays this dialog here:

notebook/notebook/static/notebook/js/notebook.js

Lines 2761 to 2791 in faa0cab

	if (that._changed_on_disk_dialog !== null) {
	// update save callback on the confirmation button
	that._changed_on_disk_dialog.find('.save-confirm-btn').click(_save);
	// redisplay existing dialog
	that._changed_on_disk_dialog.modal('show');
	} else {
	// create new dialog
	that._changed_on_disk_dialog = dialog.modal({
	notebook: that,
	keyboard_manager: that.keyboard_manager,
	title: i18n.msg._("Notebook changed"),
	body: i18n.msg._("The notebook file has changed on disk since the last time we opened or saved it. "
	+ "Do you want to overwrite the file on disk with the version open here, or load "
	+ "the version on disk (reload the page)?"),
	buttons: {
	Reload: {
	class: 'btn-warning',
	click: function() {
	window.location.reload();
	}
	},
	Cancel: {},
	Overwrite: {
	class: 'btn-danger save-confirm-btn',
	click: function () {
	_save();
	}
	},
	}
	});
	}

[unnamedplay-r]

@takluyver, I unfortunately won't be able to tackle this. It's a relatively simple fix that has a solution demonstrated in the editor.js file of my pull request. It might be a nice way to introduce someone to the codebase.

[takluyver]

Thanks. I've marked this 'good first issue', but if no-one new tackles it, one of us can do it before releasing 5.5.

[sunilhari]

@unnamedplay-r Is this issue fixed?

Can you explain me how to reproduce it.I was not able to reproduce.I might be doing something wrong.

Kindly guide me through it

[takluyver]

Can you see how to get the "notebook changed on disk" message to appear? If so, it should be possible to reproduce the problem by clicking 'cancel' on that dialog, then getting it to appear again on the same page, and then clicking reload.

[sunilhari]

@takluyver I have tried modifying the notebook from an external editor.However i did not get the modal you specified above.

[unnamedplay-r]

@sunilhari Here's some detailed instructions:

1. Open up the same notebook twice from Jupyter notebook (NB₁, NB₂).
2. In NB₂ edit a cell, then save it.
3. In NB₁, save immediately and you will trigger the Notebook changed error.
4. In this modal, press Cancel.
5. In NB₁, save again, the Notebook changed error will appear again.
6. In the Modal, press Reload. It will close the modal and not reload the page.

This is the unintended behavior.

[unnamedplay-r]

Here's the relevant code in Jupyter notebook's editor that solves this issue.

notebook/notebook/static/edit/js/editor.js

Lines 282 to 290 in ec14450

	if (that._changed_on_disk_dialog !== null) {
	// since the modal's event bindings are removed when destroyed, we reinstate
	// save & reload callbacks on the confirmation & reload buttons
	that._changed_on_disk_dialog.find('.save-confirm-btn').click(_save);
	that._changed_on_disk_dialog.find('.btn-warning').click(function () {window.location.reload()});
	// redisplay existing dialog
	that._changed_on_disk_dialog.modal('show');
	} else {

[takluyver]

@sunilhari are you still interested in working on this? Can you reproduce the issue yet? I'm aiming to release 5.5 in the next few days. It would be good to get this fixed for the release, but if you want more time to look at it, we can leave it for the next release.

[sunilhari]

I was actually confused to read @unamedplay -r comment.Was in the impression that it is fixed.

[takluyver]

There are two very similar dialogs - one for the notebook editor, which has the bug, and one for the text editor, which doesn't. @unnamedplay-r wrote the text editor one, so he's pointing to that as an example - the notebook one needs to do the some thing that the editor one does.

[sunilhari]

Hi @takluyver
Am able to reproduce the issue with steps mentioned by @unnamedplay-r .Thanks for your help
I will work on this issue.

[sunilhari]

A PR had been submitted against the issue.Kindly Review it
@unnamedplay-r Thanks again for letting me work.

[takluyver]

Closed by #3589 - thanks @sunilhari !