suzanne.soy/zotero
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Save open field when selecting from an item pane context menu

Browse Source 
If a field is open and the user right-clicks on another field (e.g., swap
names, creator type, transform text), any changed value in the open field was
lost.

Also:

- Don't show swap-names menu in single-field mode

I can't quite get programmatic access to context menus to work correctly, so
tests are disabled for now. (They work individually, but not together.)
This commit is contained in:
Dan Stillman 2016-06-29 05:24:14 -04:00
parent 2cce099928
commit 2a55e56f3f
2 changed files with 129 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
chrome/content/zotero/bindings/itembox.xml
View File

@ -786,7 +786,7 @@
firstlast.lastChild.setAttribute('hidden', true); firstlast.lastChild.setAttribute('hidden', true);
} }
if (this.editable) { if (this.editable && fieldMode == 0) {
firstlast.setAttribute('contextmenu', 'zotero-creator-transform-menu'); firstlast.setAttribute('contextmenu', 'zotero-creator-transform-menu');
} }
@ -2021,8 +2021,8 @@
<method name="textTransform"> <method name="textTransform">
<parameter name="label"/> <parameter name="label"/>
<parameter name="mode"/> <parameter name="mode"/>
<body> <body><![CDATA[
<![CDATA[ return Zotero.spawn(function* () {
var val = this._getFieldValue(label); var val = this._getFieldValue(label);
switch (mode) { switch (mode) {
case 'title': case 'title':
@ -2040,9 +2040,13 @@
throw ("Invalid transform mode '" + mode + "' in zoteroitembox.textTransform()"); throw ("Invalid transform mode '" + mode + "' in zoteroitembox.textTransform()");
} }
this._setFieldValue(label, newVal); this._setFieldValue(label, newVal);
this._modifyField(label.getAttribute('fieldname'), newVal, this.saveOnEdit).done(); if (this.saveOnEdit) {
]]> // See note in modifyCreator()
</body> yield this.blurOpenField();
}
return this._modifyField(label.getAttribute('fieldname'), newVal, this.saveOnEdit);
}, this);
]]></body>
</method> </method>
@ -2083,7 +2087,7 @@
<parameter name="fields"/> <parameter name="fields"/>
<parameter name="skipSave"/> <parameter name="skipSave"/>
<body><![CDATA[ <body><![CDATA[
return Zotero.Promise.try(function () { return Zotero.spawn(function* () {
var libraryID = this.item.libraryID; var libraryID = this.item.libraryID;
var firstName = fields.firstName; var firstName = fields.firstName;
var lastName = fields.lastName; var lastName = fields.lastName;
@ -2099,6 +2103,11 @@
} }
this.item.removeCreator(index); this.item.removeCreator(index);
if (this.saveOnEdit && !skipSave) { if (this.saveOnEdit && !skipSave) {
// Make sure any open field is saved, since a blur() isn't otherwise
// triggered clicking directly to a popup menu. (If a field is open, the
// saveTx() below will become a no-op.)
yield this.blurOpenField();
return this.item.saveTx(); return this.item.saveTx();
} }
return; return;
@ -2106,9 +2115,12 @@
var changed = this.item.setCreator(index, fields); var changed = this.item.setCreator(index, fields);
if (changed && this.saveOnEdit && !skipSave) { if (changed && this.saveOnEdit && !skipSave) {
// See note above
yield this.blurOpenField();
return this.item.saveTx(); return this.item.saveTx();
} }
}.bind(this)); }, this);
]]></body> ]]></body>
</method> </method>
@ -2117,6 +2129,7 @@
@return {Promise} @return {Promise}
--> -->
<method name="swapNames"> <method name="swapNames">
<parameter name="event"/>
<body><![CDATA[ <body><![CDATA[
return Zotero.Promise.try(function () { return Zotero.Promise.try(function () {
var row = Zotero.getAncestorByTagName(document.popupNode, 'row'); var row = Zotero.getAncestorByTagName(document.popupNode, 'row');
@ -2138,9 +2151,8 @@
<method name="moveCreator"> <method name="moveCreator">
<parameter name="index"/> <parameter name="index"/>
<parameter name="moveUp"/> <parameter name="moveUp"/>
<body> <body><![CDATA[
<![CDATA[ return Zotero.spawn(function* () {
return Zotero.Promise.try(function () {
if (index == 0 && moveUp) { if (index == 0 && moveUp) {
Zotero.debug("Can't move up creator 0"); Zotero.debug("Can't move up creator 0");
return; return;
@ -2156,11 +2168,13 @@
this.item.setCreator(newIndex, a); this.item.setCreator(newIndex, a);
this.item.setCreator(index, b); this.item.setCreator(index, b);
if (this.saveOnEdit) { if (this.saveOnEdit) {
// See note in modifyCreator()
yield this.blurOpenField();
return this.item.saveTx(); return this.item.saveTx();
} }
}.bind(this)); }, this);
]]> ]]></body>
</body>
</method> </method>
@ -2394,7 +2408,7 @@
var hideTransforms = !exists || !!fieldMode; var hideTransforms = !exists || !!fieldMode;
return !hideTransforms;"> return !hideTransforms;">
<menuitem label="&zotero.item.creatorTransform.nameSwap;" <menuitem label="&zotero.item.creatorTransform.nameSwap;"
oncommand="document.getBindingParent(this).swapNames();"/> oncommand="document.getBindingParent(this).swapNames(event);"/>
</menupopup> </menupopup>
<menupopup id="zotero-doi-menu"> <menupopup id="zotero-doi-menu">
<menuitem id="zotero-doi-menu-view-online" label="&zotero.item.viewOnline;"/> <menuitem id="zotero-doi-menu-view-online" label="&zotero.item.viewOnline;"/>

83
test/tests/itemPaneTest.js
View File

@ -27,6 +27,89 @@ describe("Item pane", function () {
yield Zotero.Items.erase(id); yield Zotero.Items.erase(id);
}) })
it.skip("should swap creator names", function* () {
var item = new Zotero.Item('book');
item.setCreators([
{
firstName: "First",
lastName: "Last",
creatorType: "author"
}
]);
yield item.saveTx();
var itemBox = doc.getElementById('zotero-editpane-item-box');
var label = doc.getAnonymousNodes(itemBox)[0].getElementsByAttribute('fieldname', 'creator-0-lastName')[0];
var parent = label.parentNode;
assert.isTrue(parent.hasAttribute('contextmenu'));
var menupopup = doc.getAnonymousNodes(itemBox)[0]
.getElementsByAttribute('id', 'zotero-creator-transform-menu')[0];
// Fake a right-click
doc.popupNode = parent;
menupopup.openPopup(
parent, "after_start", 0, 0, true, false, new MouseEvent('click', { button: 2 })
);
var menuitem = menupopup.getElementsByTagName('menuitem')[0];
menuitem.click();
yield waitForItemEvent('modify');
var creator = item.getCreators()[0];
assert.propertyVal(creator, 'firstName', 'Last');
assert.propertyVal(creator, 'lastName', 'First');
});
it("shouldn't show Swap Names menu for single-field mode", function* () {
var item = new Zotero.Item('book');
item.setCreators([
{
name: "Name",
creatorType: "author"
}
]);
yield item.saveTx();
var itemBox = doc.getElementById('zotero-editpane-item-box');
var label = doc.getAnonymousNodes(itemBox)[0].getElementsByAttribute('fieldname', 'creator-0-lastName')[0];
assert.isFalse(label.parentNode.hasAttribute('contextmenu'));
});
// Note: This issue applies to all context menus in the item box (text transform, name swap),
// though the others aren't tested. This might go away with the XUL->HTML transition.
it.skip("should save open field after changing creator type", function* () {
var item = new Zotero.Item('book');
item.setCreators([
{
firstName: "First",
lastName: "Last",
creatorType: "author"
}
]);
var id = yield item.saveTx();
var itemBox = doc.getElementById('zotero-editpane-item-box');
var label = doc.getAnonymousNodes(itemBox)[0].getElementsByAttribute('fieldname', 'place')[1];
label.click();
var textbox = doc.getAnonymousNodes(itemBox)[0].getElementsByAttribute('fieldname', 'place')[1];
textbox.value = "Place";
var menuLabel = doc.getAnonymousNodes(itemBox)[0].getElementsByAttribute('fieldname', 'creator-0-typeID')[0];
menuLabel.click();
var menupopup = itemBox._creatorTypeMenu;
var menuItems = menupopup.getElementsByTagName('menuitem');
menuItems[1].click();
yield waitForItemEvent('modify');
assert.equal(item.getField('place'), 'Place');
assert.equal(Zotero.CreatorTypes.getName(item.getCreators()[0].creatorTypeID), 'contributor');
// Wait for no-op saveTx()
yield Zotero.Promise.delay(1);
});
}) })
describe("Attachment pane", function () { describe("Attachment pane", function () {