Merge pull request #2398 from nextcloud/fix-2368

flicker-free permission change in share dialog

2 files changed, 157 insertions, 65 deletions

diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js
index a0a7bbfa2dc..0fe0747dd59 100644
--- a/core/js/sharedialogshareelistview.js
+++ b/core/js/sharedialogshareelistview.js
@@ -35,47 +35,7 @@
 						'{{/unless}}' +
 						'{{/if}}' +
 						'<a href="#"><span class="icon icon-more"></span></a>' +
-						'<div class="popovermenu bubble hidden menu">' +
-							'<ul>' +
-								'{{#if isResharingAllowed}} {{#if sharePermissionPossible}} {{#unless isMailShare}}' +
-								'<li>' +
-									'<span class="shareOption">' +
-										'<input id="canShare-{{cid}}-{{shareWith}}" type="checkbox" name="share" class="permissions checkbox" {{#if hasSharePermission}}checked="checked"{{/if}} data-permissions="{{sharePermission}}" />' +
-										'<label for="canShare-{{cid}}-{{shareWith}}">{{canShareLabel}}</label>' +
-									'</span>' +
-								'</li>' +
-								'{{/unless}} {{/if}} {{/if}}' +
-								'{{#if isFolder}}' +
-									'{{#if createPermissionPossible}}{{#unless isMailShare}}' +
-									'<li>' +
-										'<span class="shareOption">' +
-											'<input id="canCreate-{{cid}}-{{shareWith}}" type="checkbox" name="create" class="permissions checkbox" {{#if hasCreatePermission}}checked="checked"{{/if}} data-permissions="{{createPermission}}"/>' +
-											'<label for="canCreate-{{cid}}-{{shareWith}}">{{createPermissionLabel}}</label>' +
-										'</span>' +
-									'</li>' +
-									'{{/unless}}{{/if}}' +
-									'{{#if updatePermissionPossible}}{{#unless isMailShare}}' +
-									'<li>' +
-										'<span class="shareOption">' +
-											'<input id="canUpdate-{{cid}}-{{shareWith}}" type="checkbox" name="update" class="permissions checkbox" {{#if hasUpdatePermission}}checked="checked"{{/if}} data-permissions="{{updatePermission}}"/>' +
-											'<label for="canUpdate-{{cid}}-{{shareWith}}">{{updatePermissionLabel}}</label>' +
-										'</span>' +
-									'</li>' +
-									'{{/unless}}{{/if}}' +
-									'{{#if deletePermissionPossible}}{{#unless isMailShare}}' +
-									'<li>' +
-										'<span class="shareOption">' +
-											'<input id="canDelete-{{cid}}-{{shareWith}}" type="checkbox" name="delete" class="permissions checkbox" {{#if hasDeletePermission}}checked="checked"{{/if}} data-permissions="{{deletePermission}}"/>' +
-											'<label for="canDelete-{{cid}}-{{shareWith}}">{{deletePermissionLabel}}</label>' +
-										'</span>' +
-									'</li>' +
-									'{{/unless}}{{/if}}' +
-								'{{/if}}' +
-								'<li>' +
-									'<a href="#" class="unshare"><span class="icon-loading-small hidden"></span><span class="icon icon-delete"></span><span>{{unshareLabel}}</span></a>' +
-								'</li>' +
-							'</ul>' +
-						'</div>' +
+						'{{{popoverMenu}}}' +
 					'</span>' +
 				'</li>' +
 			'{{/each}}' +
@@ -94,6 +54,49 @@
 			'</ul>'
 		;
 
+	var TEMPLATE_POPOVER_MENU =
+		'<div class="popovermenu bubble hidden menu">' +
+			'<ul>' +
+				'{{#if isResharingAllowed}} {{#if sharePermissionPossible}} {{#unless isMailShare}}' +
+				'<li>' +
+					'<span class="shareOption">' +
+						'<input id="canShare-{{cid}}-{{shareWith}}" type="checkbox" name="share" class="permissions checkbox" {{#if hasSharePermission}}checked="checked"{{/if}} data-permissions="{{sharePermission}}" />' +
+						'<label for="canShare-{{cid}}-{{shareWith}}">{{canShareLabel}}</label>' +
+					'</span>' +
+				'</li>' +
+				'{{/unless}} {{/if}} {{/if}}' +
+				'{{#if isFolder}}' +
+					'{{#if createPermissionPossible}}{{#unless isMailShare}}' +
+					'<li>' +
+						'<span class="shareOption">' +
+							'<input id="canCreate-{{cid}}-{{shareWith}}" type="checkbox" name="create" class="permissions checkbox" {{#if hasCreatePermission}}checked="checked"{{/if}} data-permissions="{{createPermission}}"/>' +
+							'<label for="canCreate-{{cid}}-{{shareWith}}">{{createPermissionLabel}}</label>' +
+						'</span>' +
+					'</li>' +
+					'{{/unless}}{{/if}}' +
+					'{{#if updatePermissionPossible}}{{#unless isMailShare}}' +
+					'<li>' +
+						'<span class="shareOption">' +
+							'<input id="canUpdate-{{cid}}-{{shareWith}}" type="checkbox" name="update" class="permissions checkbox" {{#if hasUpdatePermission}}checked="checked"{{/if}} data-permissions="{{updatePermission}}"/>' +
+							'<label for="canUpdate-{{cid}}-{{shareWith}}">{{updatePermissionLabel}}</label>' +
+						'</span>' +
+					'</li>' +
+					'{{/unless}}{{/if}}' +
+					'{{#if deletePermissionPossible}}{{#unless isMailShare}}' +
+					'<li>' +
+						'<span class="shareOption">' +
+							'<input id="canDelete-{{cid}}-{{shareWith}}" type="checkbox" name="delete" class="permissions checkbox" {{#if hasDeletePermission}}checked="checked"{{/if}} data-permissions="{{deletePermission}}"/>' +
+							'<label for="canDelete-{{cid}}-{{shareWith}}">{{deletePermissionLabel}}</label>' +
+						'</span>' +
+					'</li>' +
+					'{{/unless}}{{/if}}' +
+				'{{/if}}' +
+				'<li>' +
+					'<a href="#" class="unshare"><span class="icon-loading-small hidden"></span><span class="icon icon-delete"></span><span>{{unshareLabel}}</span></a>' +
+				'</li>' +
+			'</ul>' +
+		'</div>';
+
 	/**
 	 * @class OCA.Share.ShareDialogShareeListView
 	 * @member {OC.Share.ShareItemModel} model
@@ -114,8 +117,14 @@
 		/** @type {Function} **/
 		_template: undefined,
 
+		/** @type {Function} **/
+		_popoverMenuTemplate: undefined,
+
 		_menuOpen: false,
 
+		/** @type {boolean|number} **/
+		_renderPermissionChange: false,
+
 		events: {
 			'click .unshare': 'onUnshare',
 			'click .icon-more': 'onToggleMenu',
@@ -182,8 +191,8 @@
 			});
 		},
 
-		getShareeList: function() {
-			var universal = {
+		getShareProperties: function() {
+			return {
 				avatarEnabled: this.configModel.areAvatarsEnabled(),
 				unshareLabel: t('core', 'Unshare'),
 				canShareLabel: t('core', 'can reshare'),
@@ -205,6 +214,15 @@
 				deletePermission: OC.PERMISSION_DELETE,
 				isFolder: this.model.isFolder()
 			};
+		},
+
+		/**
+		 * get an array of sharees' share properties
+		 *
+		 * @returns {Array}
+		 */
+		getShareeList: function() {
+			var universal = this.getShareProperties();
 
 			if(!this.model.hasUserShares()) {
 				return [];
@@ -256,29 +274,45 @@
 		},
 
 		render: function() {
-			this.$el.html(this.template({
-				cid: this.cid,
-				sharees: this.getShareeList(),
-				linkReshares: this.getLinkReshares()
-			}));
-
-			if(this.configModel.areAvatarsEnabled()) {
-				this.$('.avatar').each(function() {
-					var $this = $(this);
-					if ($this.hasClass('imageplaceholderseed')) {
-						$this.css({width: 32, height: 32});
-						$this.imageplaceholder($this.data('seed'));
-					} else {
-						//                         user,   size,  ie8fix, hidedefault,  callback, displayname
-						$this.avatar($this.data('username'), 32, undefined, undefined, undefined, $this.data('displayname'));
-					}
+			if(!this._renderPermissionChange) {
+				this.$el.html(this.template({
+					cid: this.cid,
+					sharees: this.getShareeList(),
+					linkReshares: this.getLinkReshares()
+				}));
+
+				if (this.configModel.areAvatarsEnabled()) {
+					this.$('.avatar').each(function () {
+						var $this = $(this);
+						if ($this.hasClass('imageplaceholderseed')) {
+							$this.css({width: 32, height: 32});
+							$this.imageplaceholder($this.data('seed'));
+						} else {
+						    //                         user,   size,  ie8fix, hidedefault,  callback, displayname
+							$this.avatar($this.data('username'), 32, undefined, undefined, undefined, $this.data('displayname'));
+						}
+					});
+				}
+
+				this.$('.has-tooltip').tooltip({
+					placement: 'bottom'
 				});
+			} else {
+				var permissionChangeShareId = parseInt(this._renderPermissionChange, 10);
+				var shareWithIndex = this.model.findShareWithIndex(permissionChangeShareId);
+				var sharee = this.getShareeObject(shareWithIndex);
+				$.extend(sharee, this.getShareProperties());
+				var $li = this.$('li[data-share-id=' + permissionChangeShareId + ']');
+				$li.find('.popovermenu').replaceWith(this.popoverMenuTemplate(sharee));
+
+				var checkBoxId = 'canEdit-' + this.cid + '-' + sharee.shareWith;
+				checkBoxId = '#' + checkBoxId.replace( /(:|\.|\[|\]|,|=|@)/g, "\\$1");
+				var $edit = $li.parent().find(checkBoxId);
+				if($edit.length === 1) {
+					$edit.prop('checked', sharee.hasEditPermission);
+				}
 			}
 
-			this.$('.has-tooltip').tooltip({
-				placement: 'bottom'
-			});
-
 			var _this = this;
 			this.$('.popovermenu').on('afterHide', function() {
 				_this._menuOpen = false;
@@ -292,6 +326,8 @@
 				}
 			}
 
+			this._renderPermissionChange = false;
+
 			this.delegateEvents();
 
 			return this;
@@ -305,9 +341,28 @@
 			if (!this._template) {
 				this._template = Handlebars.compile(TEMPLATE);
 			}
+			var sharees = data.sharees;
+			if(_.isArray(sharees)) {
+				for (var i = 0; i < sharees.length; i++) {
+					data.sharees[i].popoverMenu = this.popoverMenuTemplate(sharees[i]);
+				}
+			}
 			return this._template(data);
 		},
 
+		/**
+		 * renders the popover template and returns the resulting HTML
+		 *
+		 * @param {Object} data
+		 * @returns {string}
+		 */
+		popoverMenuTemplate: function(data) {
+			if(!this._popoverMenuTemplate) {
+				this._popoverMenuTemplate = Handlebars.compile(TEMPLATE_POPOVER_MENU);
+			}
+			return this._popoverMenuTemplate(data);
+		},
+
 		onUnshare: function(event) {
 			event.preventDefault();
 			event.stopPropagation();
@@ -385,7 +440,20 @@
 				permissions |= $(checkbox).data('permissions');
 			});
 
-			this.model.updateShare(shareId, {permissions: permissions});
+
+			/** disable checkboxes during save operation to avoid race conditions **/
+			$li.find('input[type=checkbox]').prop('disabled', true);
+			var enableCb = function() {
+				$li.find('input[type=checkbox]').prop('disabled', false);
+			};
+			var errorCb = function(elem, msg) {
+				OC.dialogs.alert(msg, t('core', 'Error while sharing'));
+				enableCb();
+			};
+
+			this.model.updateShare(shareId, {permissions: permissions}, {error: errorCb, success: enableCb});
+
+			this._renderPermissionChange = shareId;
 		},
 	});
 
diff --git a/core/js/shareitemmodel.js b/core/js/shareitemmodel.js
index a784f59f67f..b01f0f790ac 100644
--- a/core/js/shareitemmodel.js
+++ b/core/js/shareitemmodel.js
@@ -391,6 +391,26 @@
 			return share.share_with_displayname;
 		},
 
+		/**
+		 * returns the array index of a sharee for a provided shareId
+		 *
+		 * @param shareId
+		 * @returns {number}
+		 */
+		findShareWithIndex: function(shareId) {
+			var shares = this.get('shares');
+			if(!_.isArray(shares)) {
+				throw "Unknown Share";
+			}
+			for(var i = 0; i < shares.length; i++) {
+				var shareWith = shares[i];
+				if(shareWith.id === shareId) {
+					return i;
+				}
+			}
+			throw "Unknown Sharee";
+		},
+
 		getShareType: function(shareIndex) {
 			/** @type OC.Share.Types.ShareInfo **/
 			var share = this.get('shares')[shareIndex];
@@ -553,7 +573,7 @@
 			return superShare;
 		},
 
-		fetch: function() {
+		fetch: function(options) {
 			var model = this;
 			this.trigger('request', this);
 
@@ -577,6 +597,10 @@
 					shares: sharesMap,
 					reshare: reshare
 				}));
+
+				if(!_.isUndefined(options) && _.isFunction(options.success)) {
+					options.success();
+				}
 			});
 
 			return deferred;