Skip to content

Commit

Permalink
Don't show note in right-hand pane when editing in separate window
Browse files Browse the repository at this point in the history 
- When a child note is opened in a separate window, the parent window is
  selected. (This used to work but was broken in 5.0.)
- When a top-level note is opened (via double-click), the right-hand pane
  changes to show "Editing in separate window".
- If a note that's currently open in a separate window is clicked on,
  the right-hand pane shows "Editing in a separate window".
- If a note window is closed and the item is selected, the note editor
  reappears in the right-hand pane after the note is saved.

This will avoid unnecessary UI updates and data loss bugs from the two
notes getting out of sync (and is just generally cleaner).

Also:

- General cleanup of note display code
  • Loading branch information
@dstillman
dstillman committed Dec 11, 2017
1 parent b2c9a42 commit dcfaa55
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 81 deletions.
52 changes: 52 additions & 0 deletions chrome/content/zotero/itemPane.js
View file
Expand Up @@ -222,6 +222,58 @@ var ZoteroItemPane = new function() {
});


this.onNoteSelected = function (item, editable) {
// If an external note window is open for this item, don't show the editor
if (ZoteroPane.findNoteWindow(item.id)) {
this.showNoteWindowMessage();
return;
}

var noteEditor = document.getElementById('zotero-note-editor');
noteEditor.mode = editable ? 'edit' : 'view';

noteEditor.parent = null;
noteEditor.item = item;

// If loading new or different note, disable undo while we repopulate the text field
// so Undo doesn't end up clearing the field. This also ensures that Undo doesn't
// undo content from another note into the current one.
var clearUndo = noteEditor.item ? noteEditor.item.id != item.id : false;
if (clearUndo) {
noteEditor.clearUndo();
}

document.getElementById('zotero-view-note-button').hidden = !editable;
document.getElementById('zotero-item-pane-content').selectedIndex = 2;
};


this.showNoteWindowMessage = function () {
ZoteroPane.setItemPaneMessage(Zotero.getString('pane.item.notes.editingInWindow'));
};


/**
* Select the parent item and open the note editor
*/
this.openNoteWindow = async function () {
var noteEditor = document.getElementById('zotero-note-editor');
var item = noteEditor.item;
// We don't want to show the note in two places, since it causes unnecessary UI updates
// and can result in weird bugs where note content gets lost.
//
// If this is a child note, select the parent
if (item.parentID) {
await ZoteroPane.selectItem(item.parentID);
}
// Otherwise, hide note and replace with a message that we're editing externally
else {
this.showNoteWindowMessage();
}
ZoteroPane.openNoteWindow(item.id);
};


this.addNote = function (popup) {
ZoteroPane_Local.newNote(popup, _lastItem.key);
}
Expand Down
4 changes: 3 additions & 1 deletion chrome/content/zotero/itemPane.xul
View file
Expand Up @@ -114,7 +114,9 @@
<zoteronoteeditor id="zotero-note-editor" flex="1" notitle="1"
previousfocus="zotero-items-tree"
onerror="ZoteroPane.displayErrorMessage(); this.mode = 'view'"/>
<button id="zotero-view-note-button" label="&zotero.notes.separate;" oncommand="ZoteroPane_Local.openNoteWindow(this.getAttribute('noteID')); if(this.hasAttribute('sourceID')) ZoteroPane_Local.selectItem(this.getAttribute('sourceID'));"/>
<button id="zotero-view-note-button"
label="&zotero.notes.separate;"
oncommand="ZoteroItemPane.openNoteWindow()"/>
</groupbox>

<!-- Attachment item -->
Expand Down
68 changes: 34 additions & 34 deletions chrome/content/zotero/note.js
View file
Expand Up @@ -26,7 +26,7 @@
var noteEditor;
var notifierUnregisterID;

function onLoad() {
async function onLoad() {
noteEditor = document.getElementById('zotero-note-editor');
noteEditor.mode = 'edit';
noteEditor.focus();
Expand All @@ -37,42 +37,41 @@ function onLoad() {
if (window.arguments) {
var io = window.arguments[0];
}

var itemID = parseInt(io.itemID);
var collectionID = parseInt(io.collectionID);
var parentItemKey = io.parentItemKey;

return Zotero.spawn(function* () {
if (itemID) {
var ref = yield Zotero.Items.getAsync(itemID);

var clearUndo = noteEditor.item ? noteEditor.item.id != ref.id : false;

noteEditor.item = ref;

// If loading new or different note, disable undo while we repopulate the text field
// so Undo doesn't end up clearing the field. This also ensures that Undo doesn't
// undo content from another note into the current one.
if (clearUndo) {
noteEditor.clearUndo();
}

document.title = ref.getNoteTitle();
if (itemID) {
var ref = await Zotero.Items.getAsync(itemID);

var clearUndo = noteEditor.item ? noteEditor.item.id != ref.id : false;

noteEditor.item = ref;

// If loading new or different note, disable undo while we repopulate the text field
// so Undo doesn't end up clearing the field. This also ensures that Undo doesn't
// undo content from another note into the current one.
if (clearUndo) {
noteEditor.clearUndo();
}

document.title = ref.getNoteTitle();
}
else {
if (parentItemKey) {
var ref = Zotero.Items.getByLibraryAndKey(parentItemKey);
noteEditor.parentItem = ref;
}
else {
if (parentItemKey) {
var ref = Zotero.Items.getByLibraryAndKey(parentItemKey);
noteEditor.parentItem = ref;
}
else {
if (collectionID && collectionID != '' && collectionID != 'undefined') {
noteEditor.collection = Zotero.Collections.get(collectionID);
}
if (collectionID && collectionID != '' && collectionID != 'undefined') {
noteEditor.collection = Zotero.Collections.get(collectionID);
}
noteEditor.refresh();
}

notifierUnregisterID = Zotero.Notifier.registerObserver(NotifyCallback, 'item');
});
noteEditor.refresh();
}

notifierUnregisterID = Zotero.Notifier.registerObserver(NotifyCallback, 'item', 'noteWindow');
}

// If there's an error saving a note, close the window and crash the app
Expand All @@ -86,12 +85,13 @@ function onError() {
window.close();
}

function onUnload()
{
if(noteEditor && noteEditor.value)
noteEditor.save();


function onUnload() {
Zotero.Notifier.unregisterObserver(notifierUnregisterID);

if (noteEditor.item) {
window.opener.ZoteroPane.onNoteWindowClosed(noteEditor.item.id, noteEditor.value);
}
}

var NotifyCallback = {
Expand Down
1 change: 0 additions & 1 deletion chrome/content/zotero/note.xul
View file
Expand Up @@ -15,7 +15,6 @@
xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">

<script src="include.js"/>
<script src="itemPane.js"/>
<script src="note.js"/>

<keyset>
Expand Down
79 changes: 34 additions & 45 deletions chrome/content/zotero/zoteroPane.js
View file
Expand Up @@ -53,7 +53,6 @@ var ZoteroPane = new function()
this.setItemsPaneMessage = setItemsPaneMessage;
this.clearItemsPaneMessage = clearItemsPaneMessage;
this.contextPopupShowing = contextPopupShowing;
this.openNoteWindow = openNoteWindow;
this.viewSelectedAttachment = viewSelectedAttachment;
this.reportErrors = reportErrors;
this.displayErrorMessage = displayErrorMessage;
Expand Down Expand Up @@ -1479,37 +1478,7 @@ var ZoteroPane = new function()
var item = selectedItems[0];

if (item.isNote()) {
var noteEditor = document.getElementById('zotero-note-editor');
noteEditor.mode = this.collectionsView.editable ? 'edit' : 'view';

var clearUndo = noteEditor.item ? noteEditor.item.id != item.id : false;

noteEditor.parent = null;
noteEditor.item = item;

// If loading new or different note, disable undo while we repopulate the text field
// so Undo doesn't end up clearing the field. This also ensures that Undo doesn't
// undo content from another note into the current one.
if (clearUndo) {
noteEditor.clearUndo();
}

var viewButton = document.getElementById('zotero-view-note-button');
if (this.collectionsView.editable) {
viewButton.hidden = false;
viewButton.setAttribute('noteID', item.id);
if (!item.isTopLevelItem()) {
viewButton.setAttribute('parentItemID', item.parentItemID);
}
else {
viewButton.removeAttribute('parentItemID');
}
}
else {
viewButton.hidden = true;
}

document.getElementById('zotero-item-pane-content').selectedIndex = 2;
ZoteroItemPane.onNoteSelected(item, this.collectionsView.editable);
}

else if (item.isAttachment()) {
Expand Down Expand Up @@ -3640,8 +3609,7 @@ var ZoteroPane = new function()



function openNoteWindow(itemID, col, parentKey)
{
this.openNoteWindow = function (itemID, col, parentKey) {
if (!this.canEdit()) {
this.displayCannotEditLibraryMessage();
return;
Expand All @@ -3650,29 +3618,50 @@ var ZoteroPane = new function()
var name = null;

if (itemID) {
let w = this.findNoteWindow(itemID);
if (w) {
w.focus();
return;
}

// Create a name for this window so we can focus it later
//
// Collection is only used on new notes, so we don't need to
// include it in the name
name = 'zotero-note-' + itemID;

var wm = Components.classes["@mozilla.org/appshell/window-mediator;1"]
.getService(Components.interfaces.nsIWindowMediator);
var e = wm.getEnumerator('zotero:note');
while (e.hasMoreElements()) {
var w = e.getNext();
if (w.name == name) {
w.focus();
return;
}
}
}

var io = { itemID: itemID, collectionID: col, parentItemKey: parentKey };
window.openDialog('chrome://zotero/content/note.xul', name, 'chrome,resizable,centerscreen,dialog=false', io);
}


this.findNoteWindow = function (itemID) {
var name = 'zotero-note-' + itemID;
var wm = Services.wm;
var e = wm.getEnumerator('zotero:note');
while (e.hasMoreElements()) {
var w = e.getNext();
if (w.name == name) {
return w;
}
}
};


this.onNoteWindowClosed = async function (itemID, noteText) {
var item = Zotero.Items.get(itemID);
item.setNote(noteText);
await item.saveTx();

// If note is still selected, show the editor again when the note window closes
var selectedItems = this.getSelectedItems(true);
if (selectedItems.length == 1 && itemID == selectedItems[0]) {
ZoteroItemPane.onNoteSelected(item, this.collectionsView.editable);
}
};


this.addAttachmentFromURI = Zotero.Promise.method(function (link, itemID) {
if (!this.canEdit()) {
this.displayCannotEditLibraryMessage();
Expand Down
1 change: 1 addition & 0 deletions chrome/locale/en-US/zotero/zotero.properties
View file
Expand Up @@ -339,6 +339,7 @@ pane.item.notes.delete.confirm = Are you sure you want to delete this note?
pane.item.notes.count.zero = %S notes:
pane.item.notes.count.singular = %S note:
pane.item.notes.count.plural = %S notes:
pane.item.notes.editingInWindow = Editing in separate window
pane.item.attachments.rename.title = New title:
pane.item.attachments.rename.renameAssociatedFile = Rename associated file
pane.item.attachments.rename.error = An error occurred while renaming the file.
Expand Down

0 comments on commit dcfaa55

Please sign in to comment.