diff options
-rw-r--r-- | apps/files/css/files.scss | 10 | ||||
-rw-r--r-- | apps/files/js/breadcrumb.js | 2 | ||||
-rw-r--r-- | apps/files/js/fileactions.js | 26 | ||||
-rw-r--r-- | apps/files/js/filelist.js | 19 | ||||
-rw-r--r-- | apps/files/tests/js/appSpec.js | 19 | ||||
-rw-r--r-- | apps/files/tests/js/breadcrumbSpec.js | 43 | ||||
-rw-r--r-- | apps/files/tests/js/fileactionsSpec.js | 73 | ||||
-rw-r--r-- | apps/files_sharing/templates/public.php | 6 | ||||
-rw-r--r-- | core/css/mobile.scss | 2 | ||||
-rw-r--r-- | core/css/styles.scss | 16 | ||||
-rw-r--r-- | tests/acceptance/features/bootstrap/FilesAppContext.php | 37 |
11 files changed, 200 insertions, 53 deletions
diff --git a/apps/files/css/files.scss b/apps/files/css/files.scss index 6135b1ceeca..8e81bf84b4f 100644 --- a/apps/files/css/files.scss +++ b/apps/files/css/files.scss @@ -20,7 +20,11 @@ .actions.creatable { position: relative; - z-index: -30; + display: flex; + flex: 1 1; + .button:not(:last-child) { + margin-right: 3px; + } } #trash { @@ -43,10 +47,6 @@ width: 100%; } -#filestable.has-controls { - top: 44px; -} - #filestable tbody tr { height: 51px; } diff --git a/apps/files/js/breadcrumb.js b/apps/files/js/breadcrumb.js index 35aeb8d357d..20b15e3cb93 100644 --- a/apps/files/js/breadcrumb.js +++ b/apps/files/js/breadcrumb.js @@ -331,7 +331,7 @@ // Used for testing since this.$el.parent fails if (!this.availableWidth) { - this.usedWidth = this.$el.parent().width() - (this.$el.parent().find('.button').length + 1) * 44; + this.usedWidth = this.$el.parent().width() - this.$el.parent().find('.actions.creatable').width(); } else { this.usedWidth = this.availableWidth; } diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js index 6c031ab06d5..2fb7dfba29f 100644 --- a/apps/files/js/fileactions.js +++ b/apps/files/js/fileactions.js @@ -47,14 +47,6 @@ */ $el: null, - /** - * List of handlers to be notified whenever a register() or - * setDefault() was called. - * - * @member {Function[]} - */ - _updateListeners: {}, - _fileActionTriggerTemplate: null, /** @@ -142,7 +134,22 @@ var mime = action.mime; var name = action.name; var actionSpec = { - action: action.actionHandler, + action: function(fileName, context) { + // Actions registered in one FileAction may be executed on a + // different one (for example, due to the "merge" function), + // so the listeners have to be updated on the FileActions + // from the context instead of on the one in which it was + // originally registered. + if (context && context.fileActions) { + context.fileActions._notifyUpdateListeners('beforeTriggerAction', {action: actionSpec, fileName: fileName, context: context}); + } + + action.actionHandler(fileName, context); + + if (context && context.fileActions) { + context.fileActions._notifyUpdateListeners('afterTriggerAction', {action: actionSpec, fileName: fileName, context: context}); + } + }, name: name, displayName: action.displayName, mime: mime, @@ -174,7 +181,6 @@ this.defaults = {}; this.icons = {}; this.currentFile = null; - this._updateListeners = []; }, /** * Sets the default action for a given mime type. diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index fa9819b78b5..7735e9357b1 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -676,8 +676,25 @@ $(event.target).closest('a').blur(); } } else { - this._updateDetailsView($tr.attr('data-file')); + // Even if there is no Details action the default event + // handler is prevented for consistency (although there + // should always be a Details action); otherwise the link + // would be downloaded by the browser when the user expected + // the details to be shown. event.preventDefault(); + var filename = $tr.attr('data-file'); + var mime = this.fileActions.getCurrentMimeType(); + var type = this.fileActions.getCurrentType(); + var permissions = this.fileActions.getCurrentPermissions(); + var action = this.fileActions.get(mime, type, permissions)['Details']; + if (action) { + action(filename, { + $file: $tr, + fileList: this, + fileActions: this.fileActions, + dir: $tr.attr('data-path') || this.getCurrentDirectory() + }); + } } } }, diff --git a/apps/files/tests/js/appSpec.js b/apps/files/tests/js/appSpec.js index b9c323e7c12..5728991e197 100644 --- a/apps/files/tests/js/appSpec.js +++ b/apps/files/tests/js/appSpec.js @@ -112,9 +112,22 @@ describe('OCA.Files.App tests', function() { App.initialize(); var actions = App.fileList.fileActions.actions; - expect(actions.all.OverwriteThis.action).toBe(actionStub); - expect(actions.all.LegacyTest.action).toBe(legacyActionStub); - expect(actions.all.RegularTest.action).toBe(actionStub); + var context = { fileActions: sinon.createStubInstance(OCA.Files.FileActions) }; + actions.all.OverwriteThis.action('testFileName', context); + expect(actionStub.calledOnce).toBe(true); + expect(context.fileActions._notifyUpdateListeners.callCount).toBe(2); + expect(context.fileActions._notifyUpdateListeners.getCall(0).calledWith('beforeTriggerAction')).toBe(true); + expect(context.fileActions._notifyUpdateListeners.getCall(1).calledWith('afterTriggerAction')).toBe(true); + actions.all.LegacyTest.action('testFileName', context); + expect(legacyActionStub.calledOnce).toBe(true); + expect(context.fileActions._notifyUpdateListeners.callCount).toBe(4); + expect(context.fileActions._notifyUpdateListeners.getCall(2).calledWith('beforeTriggerAction')).toBe(true); + expect(context.fileActions._notifyUpdateListeners.getCall(3).calledWith('afterTriggerAction')).toBe(true); + actions.all.RegularTest.action('testFileName', context); + expect(actionStub.calledTwice).toBe(true); + expect(context.fileActions._notifyUpdateListeners.callCount).toBe(6); + expect(context.fileActions._notifyUpdateListeners.getCall(4).calledWith('beforeTriggerAction')).toBe(true); + expect(context.fileActions._notifyUpdateListeners.getCall(5).calledWith('afterTriggerAction')).toBe(true); // default one still there expect(actions.dir.Open.action).toBeDefined(); }); diff --git a/apps/files/tests/js/breadcrumbSpec.js b/apps/files/tests/js/breadcrumbSpec.js index dd3eac017ec..5ec5ad2d6e8 100644 --- a/apps/files/tests/js/breadcrumbSpec.js +++ b/apps/files/tests/js/breadcrumbSpec.js @@ -242,6 +242,17 @@ describe('OCA.Files.BreadCrumb tests', function() { dummyDir = '/short name/longer name/looooooooooooonger/' + 'even longer long long long longer long/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/last one'; + bc = new BreadCrumb(); + // append dummy navigation and controls + // as they are currently used for measurements + $('#testArea').append( + '<div id="controls"></div>' + ); + $('#controls').append(bc.$el); + + // triggers resize implicitly + bc.setDirectory(dummyDir); + // using hard-coded widths (pre-measured) to avoid getting different // results on different browsers due to font engine differences // 51px is default size for menu and home @@ -250,14 +261,6 @@ describe('OCA.Files.BreadCrumb tests', function() { $('div.crumb').each(function(index){ $(this).css('width', widths[index]); }); - - bc = new BreadCrumb(); - // append dummy navigation and controls - // as they are currently used for measurements - $('#testArea').append( - '<div id="controls"></div>' - ); - $('#controls').append(bc.$el); }); afterEach(function() { bc = null; @@ -267,8 +270,6 @@ describe('OCA.Files.BreadCrumb tests', function() { bc.setMaxWidth(500); - // triggers resize implicitly - bc.setDirectory(dummyDir); $crumbs = bc.$el.find('.crumb'); // Menu and home are always visible @@ -282,6 +283,24 @@ describe('OCA.Files.BreadCrumb tests', function() { expect($crumbs.eq(6).hasClass('hidden')).toEqual(true); expect($crumbs.eq(7).hasClass('hidden')).toEqual(false); }); + it('Hides breadcrumbs to fit max allowed width', function() { + var $crumbs; + + bc.setMaxWidth(700); + + $crumbs = bc.$el.find('.crumb'); + + // Menu and home are always visible + expect($crumbs.eq(0).hasClass('hidden')).toEqual(false); + expect($crumbs.eq(1).hasClass('hidden')).toEqual(false); + + expect($crumbs.eq(2).hasClass('hidden')).toEqual(false); + expect($crumbs.eq(3).hasClass('hidden')).toEqual(false); + expect($crumbs.eq(4).hasClass('hidden')).toEqual(true); + expect($crumbs.eq(5).hasClass('hidden')).toEqual(true); + expect($crumbs.eq(6).hasClass('hidden')).toEqual(false); + expect($crumbs.eq(7).hasClass('hidden')).toEqual(false); + }); it('Updates the breadcrumbs when reducing max allowed width', function() { var $crumbs; @@ -290,7 +309,7 @@ describe('OCA.Files.BreadCrumb tests', function() { $crumbs = bc.$el.find('.crumb'); // Menu is hidden - expect($crumbs.eq(0).hasClass('hidden')).toEqual(false); + expect($crumbs.eq(0).hasClass('hidden')).toEqual(true); // triggers resize implicitly bc.setDirectory(dummyDir); @@ -304,7 +323,7 @@ describe('OCA.Files.BreadCrumb tests', function() { expect($crumbs.eq(2).hasClass('hidden')).toEqual(false); expect($crumbs.eq(3).hasClass('hidden')).toEqual(false); - expect($crumbs.eq(4).hasClass('hidden')).toEqual(false); + expect($crumbs.eq(4).hasClass('hidden')).toEqual(true); expect($crumbs.eq(5).hasClass('hidden')).toEqual(false); expect($crumbs.eq(6).hasClass('hidden')).toEqual(false); expect($crumbs.eq(7).hasClass('hidden')).toEqual(false); diff --git a/apps/files/tests/js/fileactionsSpec.js b/apps/files/tests/js/fileactionsSpec.js index 75a18713696..2dc8bb50920 100644 --- a/apps/files/tests/js/fileactionsSpec.js +++ b/apps/files/tests/js/fileactionsSpec.js @@ -299,6 +299,7 @@ describe('OCA.Files.FileActions tests', function() { clock.restore(); }); it('passes context to action handler', function() { + var notifyUpdateListenersSpy = sinon.spy(fileList.fileActions, '_notifyUpdateListeners'); $tr.find('.action-test').click(); expect(actionStub.calledOnce).toEqual(true); expect(actionStub.getCall(0).args[0]).toEqual('testName.txt'); @@ -309,6 +310,22 @@ describe('OCA.Files.FileActions tests', function() { expect(context.dir).toEqual('/subdir'); expect(context.fileInfoModel.get('name')).toEqual('testName.txt'); + expect(notifyUpdateListenersSpy.calledTwice).toEqual(true); + expect(notifyUpdateListenersSpy.calledBefore(actionStub)).toEqual(true); + expect(notifyUpdateListenersSpy.calledAfter(actionStub)).toEqual(true); + expect(notifyUpdateListenersSpy.getCall(0).args[0]).toEqual('beforeTriggerAction'); + expect(notifyUpdateListenersSpy.getCall(0).args[1]).toEqual({ + action: fileActions.getActions('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test'], + fileName: 'testName.txt', + context: context + }); + expect(notifyUpdateListenersSpy.getCall(1).args[0]).toEqual('afterTriggerAction'); + expect(notifyUpdateListenersSpy.getCall(1).args[1]).toEqual({ + action: fileActions.getActions('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test'], + fileName: 'testName.txt', + context: context + }); + // when data-path is defined actionStub.reset(); $tr.attr('data-path', '/somepath'); @@ -317,6 +334,7 @@ describe('OCA.Files.FileActions tests', function() { expect(context.dir).toEqual('/somepath'); }); it('also triggers action handler when calling triggerAction()', function() { + var notifyUpdateListenersSpy = sinon.spy(fileList.fileActions, '_notifyUpdateListeners'); var model = new OCA.Files.FileInfoModel({ id: 1, name: 'Test.txt', @@ -331,7 +349,62 @@ describe('OCA.Files.FileActions tests', function() { expect(actionStub.getCall(0).args[1].fileList).toEqual(fileList); expect(actionStub.getCall(0).args[1].fileActions).toEqual(fileActions); expect(actionStub.getCall(0).args[1].fileInfoModel).toEqual(model); + + expect(notifyUpdateListenersSpy.calledTwice).toEqual(true); + expect(notifyUpdateListenersSpy.calledBefore(actionStub)).toEqual(true); + expect(notifyUpdateListenersSpy.calledAfter(actionStub)).toEqual(true); + expect(notifyUpdateListenersSpy.getCall(0).args[0]).toEqual('beforeTriggerAction'); + expect(notifyUpdateListenersSpy.getCall(0).args[1]).toEqual({ + action: fileActions.getActions('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test'], + fileName: 'Test.txt', + context: { + fileActions: fileActions, + fileInfoModel: model, + dir: '/subdir', + fileList: fileList, + $file: fileList.findFileEl('Test.txt') + } + }); + expect(notifyUpdateListenersSpy.getCall(1).args[0]).toEqual('afterTriggerAction'); + expect(notifyUpdateListenersSpy.getCall(1).args[1]).toEqual({ + action: fileActions.getActions('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test'], + fileName: 'Test.txt', + context: { + fileActions: fileActions, + fileInfoModel: model, + dir: '/subdir', + fileList: fileList, + $file: fileList.findFileEl('Test.txt') + } + }); }); + it('triggers listener events when invoked directly', function() { + var context = {fileActions: new OCA.Files.FileActions()} + var notifyUpdateListenersSpy = sinon.spy(context.fileActions, '_notifyUpdateListeners'); + var testAction = fileActions.get('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test']; + + testAction('Test.txt', context); + + expect(actionStub.calledOnce).toEqual(true); + expect(actionStub.getCall(0).args[0]).toEqual('Test.txt'); + expect(actionStub.getCall(0).args[1]).toBe(context); + + expect(notifyUpdateListenersSpy.calledTwice).toEqual(true); + expect(notifyUpdateListenersSpy.calledBefore(actionStub)).toEqual(true); + expect(notifyUpdateListenersSpy.calledAfter(actionStub)).toEqual(true); + expect(notifyUpdateListenersSpy.getCall(0).args[0]).toEqual('beforeTriggerAction'); + expect(notifyUpdateListenersSpy.getCall(0).args[1]).toEqual({ + action: fileActions.getActions('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test'], + fileName: 'Test.txt', + context: context + }); + expect(notifyUpdateListenersSpy.getCall(1).args[0]).toEqual('afterTriggerAction'); + expect(notifyUpdateListenersSpy.getCall(1).args[1]).toEqual({ + action: fileActions.getActions('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test'], + fileName: 'Test.txt', + context: context + }); + }), describe('actions menu', function() { it('shows actions menu inside row when clicking the menu trigger', function() { expect($tr.find('td.filename .fileActionsMenu').length).toEqual(0); diff --git a/apps/files_sharing/templates/public.php b/apps/files_sharing/templates/public.php index e17595d548b..9d28c178dde 100644 --- a/apps/files_sharing/templates/public.php +++ b/apps/files_sharing/templates/public.php @@ -50,7 +50,7 @@ $maxUploadFilesize = min($upload_max_filesize, $post_max_size); <div class="header-right"> <?php if (!isset($_['hideFileList']) || (isset($_['hideFileList']) && $_['hideFileList'] === false)) { ?> - <a href="#" id="share-menutoggle" class="menutoggle icon-more-white"><span class="share-menutoggle-text"><?php p($l->t('Download')) ?></span></a> + <a id="share-menutoggle" class="menutoggle icon-more-white"><span class="share-menutoggle-text"><?php p($l->t('Download')) ?></span></a> <div id="share-menu" class="popovermenu menu"> <ul> <li> @@ -60,7 +60,7 @@ $maxUploadFilesize = min($upload_max_filesize, $post_max_size); </a> </li> <li> - <a href="#" id="directLink-container"> + <a id="directLink-container"> <span class="icon icon-public"></span> <label for="directLink"><?php p($l->t('Direct link')) ?></label> <input id="directLink" type="text" readonly value="<?php p($_['previewURL']); ?>"> @@ -68,7 +68,7 @@ $maxUploadFilesize = min($upload_max_filesize, $post_max_size); </li> <?php if ($_['server2serversharing']) { ?> <li> - <a href="#" id="save" data-protected="<?php p($_['protected']) ?>" + <a id="save" data-protected="<?php p($_['protected']) ?>" data-owner-display-name="<?php p($_['displayName']) ?>" data-owner="<?php p($_['owner']) ?>" data-name="<?php p($_['filename']) ?>"> <span class="icon icon-external"></span> <span id="save-button"><?php p($l->t('Add to your Nextcloud')) ?></span> diff --git a/core/css/mobile.scss b/core/css/mobile.scss index 19518479987..6f1583cb77a 100644 --- a/core/css/mobile.scss +++ b/core/css/mobile.scss @@ -83,9 +83,7 @@ /* position controls for apps with app-navigation */ #app-navigation+#app-content #controls { - left: 0 !important; padding-left: 44px; - width: 100%; } /* .viewer-mode is when text editor, PDF viewer, etc is open */ diff --git a/core/css/styles.scss b/core/css/styles.scss index 5474b41a2b4..1b76e3c68de 100644 --- a/core/css/styles.scss +++ b/core/css/styles.scss @@ -223,29 +223,23 @@ body { #controls { box-sizing: border-box; - position: fixed; - top: 45px; - right: 0; - left: 0; + position: -webkit-sticky; + position: sticky; height: 44px; - width: calc(100% - 250px); padding: 0; margin: 0; background-color: rgba($color-main-background, 0.95); - z-index: 50; + z-index: 55; -webkit-user-select: none; -moz-user-select: none; -ms-user-select: none; user-select: none; - display: inline-flex; + display: flex; + top: 0; } /* position controls for apps with app-navigation */ -#app-navigation + #app-content #controls { - left: 250px; -} - .viewer-mode #app-navigation + #app-content #controls { left: 0; } diff --git a/tests/acceptance/features/bootstrap/FilesAppContext.php b/tests/acceptance/features/bootstrap/FilesAppContext.php index 338823a9478..4951dc43f1d 100644 --- a/tests/acceptance/features/bootstrap/FilesAppContext.php +++ b/tests/acceptance/features/bootstrap/FilesAppContext.php @@ -452,7 +452,16 @@ class FilesAppContext implements Context, ActorAwareInterface { * @Given I write down the shared link */ public function iWriteDownTheSharedLink() { - $this->actor->getSharedNotebook()["shared link"] = $this->actor->find(self::shareLinkField(), 10)->getValue(); + // The shared link field always exists in the DOM (once the "Sharing" + // tab is loaded), but its value is the actual shared link only when it + // is visible. + if (!$this->waitForElementToBeEventuallyShown( + self::shareLinkField(), + $timeout = 10 * $this->actor->getFindTimeoutMultiplier())) { + PHPUnit_Framework_Assert::fail("The shared link was not shown yet after $timeout seconds"); + } + + $this->actor->getSharedNotebook()["shared link"] = $this->actor->find(self::shareLinkField())->getValue(); } /** @@ -606,7 +615,9 @@ class FilesAppContext implements Context, ActorAwareInterface { * @When I see that the :tabName tab in the details view is eventually loaded */ public function iSeeThatTheTabInTheDetailsViewIsEventuallyLoaded($tabName) { - if (!$this->waitForElementToBeEventuallyNotShown(self::loadingIconForTabInCurrentSectionDetailsViewNamed($tabName), $timeout = 10)) { + if (!$this->waitForElementToBeEventuallyNotShown( + self::loadingIconForTabInCurrentSectionDetailsViewNamed($tabName), + $timeout = 10 * $this->actor->getFindTimeoutMultiplier())) { PHPUnit_Framework_Assert::fail("The $tabName tab in the details view has not been loaded after $timeout seconds"); } } @@ -622,7 +633,9 @@ class FilesAppContext implements Context, ActorAwareInterface { * @Then I see that the working icon for password protect is eventually not shown */ public function iSeeThatTheWorkingIconForPasswordProtectIsEventuallyNotShown() { - if (!$this->waitForElementToBeEventuallyNotShown(self::passwordProtectWorkingIcon(), $timeout = 10)) { + if (!$this->waitForElementToBeEventuallyNotShown( + self::passwordProtectWorkingIcon(), + $timeout = 10 * $this->actor->getFindTimeoutMultiplier())) { PHPUnit_Framework_Assert::fail("The working icon for password protect is still shown after $timeout seconds"); } } @@ -637,10 +650,24 @@ class FilesAppContext implements Context, ActorAwareInterface { $this->iSeeThatTheWorkingIconForPasswordProtectIsEventuallyNotShown(); } + private function waitForElementToBeEventuallyShown($elementLocator, $timeout = 10, $timeoutStep = 1) { + $actor = $this->actor; + + $elementShownCallback = function() use ($actor, $elementLocator) { + try { + return $actor->find($elementLocator)->isVisible(); + } catch (NoSuchElementException $exception) { + return false; + } + }; + + return Utils::waitFor($elementShownCallback, $timeout, $timeoutStep); + } + private function waitForElementToBeEventuallyNotShown($elementLocator, $timeout = 10, $timeoutStep = 1) { $actor = $this->actor; - $elementNotFoundCallback = function() use ($actor, $elementLocator) { + $elementNotShownCallback = function() use ($actor, $elementLocator) { try { return !$actor->find($elementLocator)->isVisible(); } catch (NoSuchElementException $exception) { @@ -648,6 +675,6 @@ class FilesAppContext implements Context, ActorAwareInterface { } }; - return Utils::waitFor($elementNotFoundCallback, $timeout, $timeoutStep); + return Utils::waitFor($elementNotShownCallback, $timeout, $timeoutStep); } } |