From 32c1b46d30a9baa768c00e16fe8ae63aceb9f2ed Mon Sep 17 00:00:00 2001 From: Felix Heidecke Date: Fri, 27 Oct 2017 16:25:50 +0200 Subject: [PATCH 1/2] Rework permissions in UX friendlier radio-button style, improve modal behavior on mobile devices. --- apps/files_sharing/css/sharetabview.css | 3 - core/css/jquery.ocdialog.css | 23 +++++-- core/js/jquery.ocdialog.js | 38 ----------- core/js/sharedialoglinkshareview.js | 86 ++++++++++--------------- 4 files changed, 52 insertions(+), 98 deletions(-) diff --git a/apps/files_sharing/css/sharetabview.css b/apps/files_sharing/css/sharetabview.css index a1a6cb137db3..3c24e5fb097a 100644 --- a/apps/files_sharing/css/sharetabview.css +++ b/apps/files_sharing/css/sharetabview.css @@ -254,9 +254,6 @@ } /* ---------------------------------------------------- PUBLIC LINK MODAL --- */ -.public-link-modal { - width: 340px; -} .public-link-modal--item { margin-bottom: 20px; } diff --git a/core/css/jquery.ocdialog.css b/core/css/jquery.ocdialog.css index d37fdf548d4d..90b261beefed 100644 --- a/core/css/jquery.ocdialog.css +++ b/core/css/jquery.ocdialog.css @@ -8,13 +8,28 @@ -webkit-box-sizing: border-box; -moz-box-sizing: border-box; box-sizing: border-box; - min-width: 200px; + min-width: 340px; +} + +@media (max-width: 512px) { + .oc-dialog { + position: absolute; + top: 55px; + right: 10px; + left: 10px; + } +} +@media (min-width: 513px) { /* Center positioning */ - left: 50%; - top: 50%; - transform: translate(-50%,-50%); + .oc-dialog { + position: fixed; + left: 50%; + top: 50%; + transform: translate(-50%,-50%); + } } + .oc-dialog-title { background: white; font-weight: bold; diff --git a/core/js/jquery.ocdialog.js b/core/js/jquery.ocdialog.js index d63680897793..24b88d8e1e24 100644 --- a/core/js/jquery.ocdialog.js +++ b/core/js/jquery.ocdialog.js @@ -10,12 +10,6 @@ _create: function() { var self = this; - this.originalCss = { - display: this.element[0].style.display, - width: this.element[0].style.width, - height: this.element[0].style.height - }; - this.originalTitle = this.element.attr('title'); this.options.title = this.options.title || this.originalTitle; @@ -29,11 +23,6 @@ this.$dialog.append(this.element.detach()); this.element.removeAttr('title').addClass('oc-dialog-content').appendTo(this.$dialog); - this.$dialog.css({ - display: 'inline-block', - position: 'fixed' - }); - $(document).on('keydown keyup', function(event) { if ( event.target !== self.$dialog.get(0) && @@ -138,12 +127,6 @@ this.$dialog.find('.oc-dialog-close').remove(); } break; - case 'width': - this.$dialog.css('width', value); - break; - case 'height': - this.$dialog.css('height', value); - break; case 'close': this.closeCB = value; break; @@ -155,27 +138,6 @@ //this._super(options); $.Widget.prototype._setOptions.apply(this, arguments); }, - _setSizes: function() { - // var content_height = this.$dialog.height(); - // if(this.$title) { - // content_height -= this.$title.outerHeight(true); - // } - // if(this.$buttonrow) { - // content_height -= this.$buttonrow.outerHeight(true); - // } - // this.parent = this.$dialog.parent().length > 0 ? this.$dialog.parent() : $('body'); - // content_height = Math.min(content_height, this.parent.height()-20); - // if (content_height> 0) { - // this.element.css({ - // height: content_height + 'px', - // width: this.$dialog.innerWidth()-20 + 'px' - // }); - // } else { - // this.element.css({ - // width : this.$dialog.innerWidth() - 20 + 'px' - // }); - // } - }, _createOverlay: function() { if(!this.options.modal) { return; diff --git a/core/js/sharedialoglinkshareview.js b/core/js/sharedialoglinkshareview.js index 0677ceda76be..5ad032bbbcc5 100644 --- a/core/js/sharedialoglinkshareview.js +++ b/core/js/sharedialoglinkshareview.js @@ -23,13 +23,20 @@ '' + '' + '{{#if publicUploadPossible}}' + - '
' + - '' + - '' + + '
' + + '' + + '' + + '

{{publicReadDescription}}

' + + '
' + + '
' + + '' + + '' + + '

{{publicReadWriteDescription}}

' + '
' + - '
' + - '' + - '' + + '
' + + '' + + '' + + '

{{publicUploadDescription}}

' + '
' + '{{/if}}' + '
' + @@ -69,10 +76,6 @@ /** @type {Function} **/ _template: undefined, - events: { - 'click .publicUploadCheckbox': '_updateCheckboxes' - }, - initialize: function (options) { if (!_.isUndefined(options.itemModel)) { this.itemModel = options.itemModel; @@ -89,16 +92,6 @@ OC.Plugins.attach('OCA.Share.ShareDialogLinkShareView', this); }, - _updateCheckboxes: function() { - var publicUploadAllowed = this.$('.publicUploadCheckbox').is(':checked'); - if (!publicUploadAllowed) { - this.$('.showListingCheckbox').prop('checked', true); - this.$('.showListingCheckbox').prop('disabled', true); - } else { - this.$('.showListingCheckbox').prop('disabled', false); - } - }, - /** * Returns the selected permissions as read from the checkboxes or * the absence thereof. @@ -106,29 +99,8 @@ * @return {int} permissions */ _getPermissions: function() { - var $showListingCheckbox = this.$('.showListingCheckbox'); - var $publicUploadCheckbox = this.$('.publicUploadCheckbox'); - var allowListing = (!$showListingCheckbox.length || $showListingCheckbox.is(':checked')); - var permissions = 0; - - // if the checkbox is missing, default to checked - if (allowListing) { - permissions |= OC.PERMISSION_READ; - } - - // if the checkbox is missing it is the equivalent of unchecked - if ($publicUploadCheckbox.is(':checked')) { - if (allowListing) { - permissions |= OC.PERMISSION_UPDATE | OC.PERMISSION_CREATE | OC.PERMISSION_DELETE; - } else { - // without listing only file creation is allowed, no overwrite nor delete - permissions |= OC.PERMISSION_CREATE; - } - } else { - // ignore listing perm, allow reading - permissions |= OC.PERMISSION_READ; - } - + var $permissionRadio = this.$('input[name="publicPermissions"]:checked'); + var permissions = $permissionRadio.val(); return permissions; }, @@ -248,18 +220,28 @@ this.$el.html(this.template({ cid: this.cid, - fileNameLabel : t('core', 'Filename'), - passwordLabel: t('core', 'Password'), passwordPlaceholder: isPasswordSet ? PASSWORD_PLACEHOLDER_STARS : PASSWORD_PLACEHOLDER_MESSAGE, isPasswordRequired: this.configModel.get('enforcePasswordForPublicLink'), namePlaceholder: t('core', 'Name'), name: this.model.get('name'), isPasswordSet: isPasswordSet, - publicUploadPossible: this._isPublicUploadPossible(), - publicUploadChecked: this.model.canCreate(), - publicUploadLabel: t('core', 'Allow editing'), - showListingChecked: this.model.canRead(), - showListingLabel: t('core', 'Show file listing'), + + fileNameLabel : t('core', 'Filename'), + passwordLabel : t('core', 'Password'), + + publicUploadPossible : this._isPublicUploadPossible(), + publicUploadSelected : this.model.get('permissions') === 4, + publicUploadLabel : t('core', 'Upload only (File Drop)'), + publicUploadDescription : t('core', 'Receive files from others without revealing the contents of the folder.'), + + publicReadSelected : this.model.get('permissions') === 1, + publicReadLabel : t('core', 'Read only'), + publicReadDescription : t('core', 'Users can view and download contents.'), + + publicReadWriteSelected : this.model.get('permissions') === 15, + publicReadWriteLabel : t('core', 'Read & Write'), + publicReadWriteDescription : t('core', 'Users can view, download, edit and upload contents.'), + isMailEnabled: showEmailField })); @@ -280,8 +262,6 @@ this.expirationView.render(); this.$('.expirationDateContainer').append(this.expirationView.$el); - this._updateCheckboxes(); - this.delegateEvents(); return this; @@ -368,7 +348,7 @@ }); } - }); +}); OC.Share.ShareDialogLinkShareView = ShareDialogLinkShareView; From f97b37d0f86a53a5b77b01bf432b091030f72877 Mon Sep 17 00:00:00 2001 From: Felix Heidecke Date: Mon, 30 Oct 2017 14:51:30 +0100 Subject: [PATCH 2/2] Update tests specs to meet new feature implementations --- core/js/sharedialoglinkshareview.js | 24 ++++--- .../specs/sharedialoglinkshareviewSpec.js | 62 +++++-------------- 2 files changed, 30 insertions(+), 56 deletions(-) diff --git a/core/js/sharedialoglinkshareview.js b/core/js/sharedialoglinkshareview.js index 5ad032bbbcc5..6828eb377901 100644 --- a/core/js/sharedialoglinkshareview.js +++ b/core/js/sharedialoglinkshareview.js @@ -22,19 +22,19 @@ '' + '' + '
' + - '{{#if publicUploadPossible}}' + '
' + - '' + + '' + '' + '

{{publicReadDescription}}

' + '
' + + '{{#if publicUploadPossible}}' + '
' + - '' + + '' + '' + '

{{publicReadWriteDescription}}

' + '
' + '
' + - '' + + '' + '' + '

{{publicUploadDescription}}

' + '
' + @@ -99,9 +99,9 @@ * @return {int} permissions */ _getPermissions: function() { - var $permissionRadio = this.$('input[name="publicPermissions"]:checked'); - var permissions = $permissionRadio.val(); - return permissions; + var permissions = this.$('input[name="publicPermissions"]:checked').val(); + + return (permissions) ? permissions : OC.PERMISSION_READ; }, _save: function () { @@ -230,17 +230,21 @@ passwordLabel : t('core', 'Password'), publicUploadPossible : this._isPublicUploadPossible(), - publicUploadSelected : this.model.get('permissions') === 4, + publicUploadLabel : t('core', 'Upload only (File Drop)'), publicUploadDescription : t('core', 'Receive files from others without revealing the contents of the folder.'), + publicUploadValue : OC.PERMISSION_CREATE, + publicUploadSelected : this.model.get('permissions') === OC.PERMISSION_CREATE, - publicReadSelected : this.model.get('permissions') === 1, publicReadLabel : t('core', 'Read only'), publicReadDescription : t('core', 'Users can view and download contents.'), + publicReadValue : OC.PERMISSION_READ, + publicReadSelected : this.model.get('permissions') === OC.PERMISSION_READ, - publicReadWriteSelected : this.model.get('permissions') === 15, publicReadWriteLabel : t('core', 'Read & Write'), publicReadWriteDescription : t('core', 'Users can view, download, edit and upload contents.'), + publicReadWriteValue : OC.PERMISSION_READ | OC.PERMISSION_UPDATE | OC.PERMISSION_CREATE | OC.PERMISSION_DELETE, + publicReadWriteSelected : this.model.get('permissions') >= (OC.PERMISSION_READ | OC.PERMISSION_UPDATE | OC.PERMISSION_CREATE | OC.PERMISSION_DELETE), isMailEnabled: showEmailField })); diff --git a/core/js/tests/specs/sharedialoglinkshareviewSpec.js b/core/js/tests/specs/sharedialoglinkshareviewSpec.js index cf981ad4685c..eed626fe3ea9 100644 --- a/core/js/tests/specs/sharedialoglinkshareviewSpec.js +++ b/core/js/tests/specs/sharedialoglinkshareviewSpec.js @@ -135,18 +135,18 @@ describe('OC.Share.ShareDialogLinkShareView', function() { publicUploadConfigStub.returns(true); view.render(); expect(view.$('[name=linkName]').val()).toEqual('first link'); - expect(view.$('.publicUploadCheckbox').prop('checked')).toEqual(false); + expect(view.$('.publicPermissions').prop('checked')).toEqual(true); expect(view.$('.linkPassText').val()).toEqual(''); expect(view.$('.expirationDate').val()).toEqual(''); model.set({ password: 'set', expireDate: '2017-10-12', - permissions: OC.PERMISSION_ALL + permissions: OC.PERMISSION_CREATE }); view.render(); - expect(view.$('.publicUploadCheckbox').prop('checked')).toEqual(true); + expect(parseInt(view.$('.publicPermissions:checked').val())).toBe(OC.PERMISSION_CREATE); expect(view.$('.linkPassText').val()).toEqual(''); expect(view.$('.expirationDate').val()).toEqual('12-10-2017'); }); @@ -208,37 +208,15 @@ describe('OC.Share.ShareDialogLinkShareView', function() { permissions: OC.PERMISSION_READ | OC.PERMISSION_CREATE }); view.render(); - expect(view.$('.showListingCheckbox').length).toEqual(1); - expect(view.$('.showListingCheckbox').is(':checked')).toEqual(true); - expect(view.$('.showListingCheckbox').is(':disabled')).toEqual(false); + expect(view.$('.publicPermissions').length).toEqual(3); }); - it('renders listing checkbox disabled when public upload is disallowed by user', function() { + it('renders checkbox disabled when public upload is disallowed by user', function() { publicUploadConfigStub.returns(true); model.set({ permissions: OC.PERMISSION_READ }); view.render(); - expect(view.$('.showListingCheckbox').length).toEqual(1); - expect(view.$('.showListingCheckbox').is(':checked')).toEqual(true); - expect(view.$('.showListingCheckbox').is(':disabled')).toEqual(true); - }); - it('disables listing checkbox when ticking public upload', function() { - publicUploadConfigStub.returns(true); - model.set({ - permissions: OC.PERMISSION_CREATE - }); - view.render(); - - expect(view.$('.showListingCheckbox').length).toEqual(1); - expect(view.$('.showListingCheckbox').is(':checked')).toEqual(false); - expect(view.$('.showListingCheckbox').is(':disabled')).toEqual(false); - - expect(view.$('.publicUploadCheckbox').length).toEqual(1); - expect(view.$('.publicUploadCheckbox').is(':checked')).toEqual(true); - view.$('.publicUploadCheckbox').trigger(new $.Event('click')); - expect(view.$('.publicUploadCheckbox').is(':checked')).toEqual(false); - expect(view.$('.showListingCheckbox').is(':checked')).toEqual(true); - expect(view.$('.showListingCheckbox').is(':disabled')).toEqual(true); + expect(view.$('.showListingCheckbox').length).toEqual(0); }); }); describe('password logic', function() { @@ -293,7 +271,7 @@ describe('OC.Share.ShareDialogLinkShareView', function() { name: 'first link', expireDate: '', password: 'newpassword', - permissions: OC.PERMISSION_READ, + permissions: OC.PERMISSION_READ.toString(), shareType: OC.Share.SHARE_TYPE_LINK }); }); @@ -306,7 +284,7 @@ describe('OC.Share.ShareDialogLinkShareView', function() { expect(saveStub.getCall(0).args[0]).toEqual({ name: 'first link', expireDate: '', - permissions: OC.PERMISSION_READ, + permissions: OC.PERMISSION_READ.toString(), shareType: OC.Share.SHARE_TYPE_LINK }); }); @@ -394,30 +372,22 @@ describe('OC.Share.ShareDialogLinkShareView', function() { }); var dataProvider = [ - // globally enabled - [true, true, true, OC.PERMISSION_READ | OC.PERMISSION_CREATE | OC.PERMISSION_UPDATE | OC.PERMISSION_DELETE], - [true, true, false, OC.PERMISSION_CREATE], - [true, false, true, OC.PERMISSION_READ], - [true, false, false, OC.PERMISSION_READ], - - // globally disabled, permission stays regardless - [false, false, false, OC.PERMISSION_READ], - [false, true, false, OC.PERMISSION_READ], - [false, true, false, OC.PERMISSION_READ], - [false, true, true, OC.PERMISSION_READ], + [true, OC.PERMISSION_READ | OC.PERMISSION_CREATE | OC.PERMISSION_UPDATE | OC.PERMISSION_DELETE], + [true, OC.PERMISSION_CREATE], + [true, OC.PERMISSION_READ], + [false, OC.PERMISSION_READ], // globally disabled, permission stays regardless ]; - function testPermissions(globalEnabled, uploadChecked, listingChecked, expectedPerms) { + function testPermissions(globalEnabled, expectedPerms) { + expectedPerms = expectedPerms.toString(); it('sets permissions to ' + expectedPerms + ' if global enabled is ' + globalEnabled + - ' and public upload checkbox is ' + uploadChecked + - ' and listing checkbox is ' + listingChecked, function() { + ' and corresponding radiobutton is checked', function() { publicUploadConfigStub.returns(globalEnabled); view.render(); - view.$('.publicUploadCheckbox').prop('checked', uploadChecked); - view.$('.showListingCheckbox').prop('checked', listingChecked); + view.$('input[name="publicPermissions"]:checked').val(expectedPerms); view._save();