From f822e3af4d5f32357f12b5573636876467725dab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 9 Jun 2017 03:02:33 +0200 Subject: [PATCH 1/7] Add acceptance tests for showing the input field for tags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- tests/acceptance/features/app-files.feature | 16 +++ .../features/bootstrap/FilesAppContext.php | 107 ++++++++++++++++++ 2 files changed, 123 insertions(+) diff --git a/tests/acceptance/features/app-files.feature b/tests/acceptance/features/app-files.feature index 6779b37e145..8d32508513a 100644 --- a/tests/acceptance/features/app-files.feature +++ b/tests/acceptance/features/app-files.feature @@ -52,3 +52,19 @@ Feature: app-files And I authenticate with password "fedcba" Then I see that the current page is the Authenticate page for the shared link I wrote down And I see that a wrong password for the shared file message is shown + + Scenario: show the input field for tags in the details view + Given I am logged in + And I open the details view for "welcome.txt" + And I see that the details view for "All files" section is open + When I open the input field for tags in the details view + Then I see that the input field for tags in the details view is shown + + Scenario: show the input field for tags in the details view after the sharing tab has loaded + Given I am logged in + And I open the details view for "welcome.txt" + And I see that the details view for "All files" section is open + And I open the "Sharing" tab in the details view + And I see that the "Sharing" tab in the details view is eventually loaded + When I open the input field for tags in the details view + Then I see that the input field for tags in the details view is shown diff --git a/tests/acceptance/features/bootstrap/FilesAppContext.php b/tests/acceptance/features/bootstrap/FilesAppContext.php index 5916fd4bec6..9d8e05a1325 100644 --- a/tests/acceptance/features/bootstrap/FilesAppContext.php +++ b/tests/acceptance/features/bootstrap/FilesAppContext.php @@ -102,6 +102,69 @@ class FilesAppContext implements Context, ActorAwareInterface { describedAs("Current section details view in Files app"); } + /** + * @return Locator + */ + public static function fileDetailsInCurrentSectionDetailsViewWithText($fileDetailsText) { + return Locator::forThe()->xpath("//span[normalize-space() = '$fileDetailsText']")-> + descendantOf(self::fileDetailsInCurrentSectionDetailsView())-> + describedAs("File details with text \"$fileDetailsText\" in current section details view in Files app"); + } + + /** + * @return Locator + */ + private static function fileDetailsInCurrentSectionDetailsView() { + return Locator::forThe()->css(".file-details")-> + descendantOf(self::currentSectionDetailsView())-> + describedAs("File details in current section details view in Files app"); + } + + /** + * @return Locator + */ + public static function inputFieldForTagsInCurrentSectionDetails() { + return Locator::forThe()->css(".systemTagsInfoView")-> + descendantOf(self::currentSectionDetailsView())-> + describedAs("Input field for tags in current section details view in Files app"); + } + + /** + * @return Locator + */ + public static function tabHeaderInCurrentSectionDetailsViewNamed($tabHeaderName) { + return Locator::forThe()->xpath("//li[normalize-space() = '$tabHeaderName']")-> + descendantOf(self::tabHeadersInCurrentSectionDetailsView())-> + describedAs("Tab header named $tabHeaderName in current section details view in Files app"); + } + + /** + * @return Locator + */ + private static function tabHeadersInCurrentSectionDetailsView() { + return Locator::forThe()->css(".tabHeaders")-> + descendantOf(self::currentSectionDetailsView())-> + describedAs("Tab headers in current section details view in Files app"); + } + + /** + * @return Locator + */ + public static function tabInCurrentSectionDetailsViewNamed($tabName) { + return Locator::forThe()->xpath("//div[@id=//*[contains(concat(' ', normalize-space(@class), ' '), ' tabHeader ') and normalize-space() = '$tabName']/@data-tabid]")-> + descendantOf(self::currentSectionDetailsView())-> + describedAs("Tab named $tabName in current section details view in Files app"); + } + + /** + * @return Locator + */ + public static function loadingIconForTabInCurrentSectionDetailsViewNamed($tabName) { + return Locator::forThe()->css(".loading")-> + descendantOf(self::tabInCurrentSectionDetailsViewNamed($tabName))-> + describedAs("Loading icon for tab named $tabName in current section details view in Files app"); + } + /** * @return Locator */ @@ -246,6 +309,20 @@ class FilesAppContext implements Context, ActorAwareInterface { $this->actor->find(self::detailsMenuItem(), 2)->click(); } + /** + * @Given I open the input field for tags in the details view + */ + public function iOpenTheInputFieldForTagsInTheDetailsView() { + $this->actor->find(self::fileDetailsInCurrentSectionDetailsViewWithText("Tags"), 10)->click(); + } + + /** + * @Given I open the :tabName tab in the details view + */ + public function iOpenTheTabInTheDetailsView($tabName) { + $this->actor->find(self::tabHeaderInCurrentSectionDetailsViewNamed($tabName), 10)->click(); + } + /** * @Given I mark :fileName as favorite */ @@ -343,6 +420,36 @@ class FilesAppContext implements Context, ActorAwareInterface { PHPUnit_Framework_Assert::assertNotNull($this->actor->find(self::favoritedStateIconForFile($fileName), 10)); } + /** + * @Then I see that the input field for tags in the details view is shown + */ + public function iSeeThatTheInputFieldForTagsInTheDetailsViewIsShown() { + PHPUnit_Framework_Assert::assertTrue( + $this->actor->find(self::inputFieldForTagsInCurrentSectionDetails(), 10)->isVisible()); + } + + /** + * @When I see that the :tabName tab in the details view is eventually loaded + */ + public function iSeeThatTheTabInTheDetailsViewIsEventuallyLoaded($tabName) { + $timeout = 10; + $timeoutStep = 1; + + $actor = $this->actor; + $loadingIcon = self::loadingIconForTabInCurrentSectionDetailsViewNamed($tabName); + + $loadingIconNotFoundCallback = function() use ($actor, $loadingIcon) { + try { + return !$actor->find($loadingIcon)->isVisible(); + } catch (NoSuchElementException $exception) { + return true; + } + }; + if (!Utils::waitFor($loadingIconNotFoundCallback, $timeout, $timeoutStep)) { + PHPUnit_Framework_Assert::fail("The $tabName tab in the details view has not been loaded after $timeout seconds"); + } + } + /** * @Then I see that the working icon for password protect is shown */ From bd626a9faa68ddc2918062d15852b66b720386b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 9 Jun 2017 03:03:47 +0200 Subject: [PATCH 2/7] Extract duplicated code to a method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../features/bootstrap/FilesAppContext.php | 43 +++++++------------ 1 file changed, 15 insertions(+), 28 deletions(-) diff --git a/tests/acceptance/features/bootstrap/FilesAppContext.php b/tests/acceptance/features/bootstrap/FilesAppContext.php index 9d8e05a1325..52f69c66796 100644 --- a/tests/acceptance/features/bootstrap/FilesAppContext.php +++ b/tests/acceptance/features/bootstrap/FilesAppContext.php @@ -432,20 +432,7 @@ class FilesAppContext implements Context, ActorAwareInterface { * @When I see that the :tabName tab in the details view is eventually loaded */ public function iSeeThatTheTabInTheDetailsViewIsEventuallyLoaded($tabName) { - $timeout = 10; - $timeoutStep = 1; - - $actor = $this->actor; - $loadingIcon = self::loadingIconForTabInCurrentSectionDetailsViewNamed($tabName); - - $loadingIconNotFoundCallback = function() use ($actor, $loadingIcon) { - try { - return !$actor->find($loadingIcon)->isVisible(); - } catch (NoSuchElementException $exception) { - return true; - } - }; - if (!Utils::waitFor($loadingIconNotFoundCallback, $timeout, $timeoutStep)) { + if (!$this->waitForElementToBeEventuallyNotShown(self::loadingIconForTabInCurrentSectionDetailsViewNamed($tabName), $timeout = 10)) { PHPUnit_Framework_Assert::fail("The $tabName tab in the details view has not been loaded after $timeout seconds"); } } @@ -461,20 +448,7 @@ class FilesAppContext implements Context, ActorAwareInterface { * @Then I see that the working icon for password protect is eventually not shown */ public function iSeeThatTheWorkingIconForPasswordProtectIsEventuallyNotShown() { - $timeout = 10; - $timeoutStep = 1; - - $actor = $this->actor; - $passwordProtectWorkingIcon = self::passwordProtectWorkingIcon(); - - $workingIconNotFoundCallback = function() use ($actor, $passwordProtectWorkingIcon) { - try { - return !$actor->find($passwordProtectWorkingIcon)->isVisible(); - } catch (NoSuchElementException $exception) { - return true; - } - }; - if (!Utils::waitFor($workingIconNotFoundCallback, $timeout, $timeoutStep)) { + if (!$this->waitForElementToBeEventuallyNotShown(self::passwordProtectWorkingIcon(), $timeout = 10)) { PHPUnit_Framework_Assert::fail("The working icon for password protect is still shown after $timeout seconds"); } } @@ -489,4 +463,17 @@ class FilesAppContext implements Context, ActorAwareInterface { $this->iSeeThatTheWorkingIconForPasswordProtectIsEventuallyNotShown(); } + private function waitForElementToBeEventuallyNotShown($elementLocator, $timeout = 10, $timeoutStep = 1) { + $actor = $this->actor; + + $elementNotFoundCallback = function() use ($actor, $elementLocator) { + try { + return !$actor->find($elementLocator)->isVisible(); + } catch (NoSuchElementException $exception) { + return true; + } + }; + + return Utils::waitFor($elementNotFoundCallback, $timeout, $timeoutStep); + } } From 706106408c70a3ce9dd4b6569f71d05aa4f588cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 9 Jun 2017 03:14:23 +0200 Subject: [PATCH 3/7] Make possible to know the registered detail views in a details view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In some cases, an app may need to act on a detail view registered by another app or the core, for example, to add extra elements to the element of the detail view. Signed-off-by: Daniel Calviño Sánchez --- apps/files/js/detailsview.js | 10 ++++++++++ apps/files/js/filelist.js | 15 +++++++++++++++ apps/files/tests/js/detailsviewSpec.js | 21 +++++++++++++++++++++ apps/files/tests/js/filelistSpec.js | 26 ++++++++++++++++++++++++++ 4 files changed, 72 insertions(+) diff --git a/apps/files/js/detailsview.js b/apps/files/js/detailsview.js index f04adcf1292..e53922ebb69 100644 --- a/apps/files/js/detailsview.js +++ b/apps/files/js/detailsview.js @@ -300,6 +300,16 @@ addDetailView: function(detailView) { this._detailFileInfoViews.push(detailView); this._dirty = true; + }, + + /** + * Returns an array with the added DetailFileInfoViews. + * + * @return Array an array with the added + * DetailFileInfoViews. + */ + getDetailViews: function() { + return [].concat(this._detailFileInfoViews); } }); diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 187ede8c0bd..b746588889b 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -3018,6 +3018,21 @@ if (this.breadcrumb) { this.breadcrumb.addDetailView(detailView); } + }, + + /** + * Returns the registered detail views. + * + * @return null|Array an array with the + * registered DetailFileInfoViews, or null if the details view + * is not enabled. + */ + getRegisteredDetailViews: function() { + if (this._detailsView) { + return this._detailsView.getDetailViews(); + } + + return null; } }; diff --git a/apps/files/tests/js/detailsviewSpec.js b/apps/files/tests/js/detailsviewSpec.js index 26a16b31530..0f483728bff 100644 --- a/apps/files/tests/js/detailsviewSpec.js +++ b/apps/files/tests/js/detailsviewSpec.js @@ -35,6 +35,27 @@ describe('OCA.Files.DetailsView tests', function() { expect(detailsView.$el.find('.tabsContainer').length).toEqual(1); }); describe('file info detail view', function() { + it('returns registered view', function() { + var testView = new OCA.Files.DetailFileInfoView(); + var testView2 = new OCA.Files.DetailFileInfoView(); + detailsView.addDetailView(testView); + detailsView.addDetailView(testView2); + + detailViews = detailsView.getDetailViews(); + + expect(detailViews).toContain(testView); + expect(detailViews).toContain(testView2); + + // Modify array and check that registered detail views are not + // modified + detailViews.pop(); + detailViews.pop(); + + detailViews = detailsView.getDetailViews(); + + expect(detailViews).toContain(testView); + expect(detailViews).toContain(testView2); + }); it('renders registered view', function() { var testView = new OCA.Files.DetailFileInfoView(); var testView2 = new OCA.Files.DetailFileInfoView(); diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js index 6b403e7fa85..b7ee9c8554e 100644 --- a/apps/files/tests/js/filelistSpec.js +++ b/apps/files/tests/js/filelistSpec.js @@ -2116,10 +2116,12 @@ describe('OCA.Files.FileList tests', function() { beforeEach(function() { addTabStub = sinon.stub(OCA.Files.DetailsView.prototype, 'addTabView'); addDetailStub = sinon.stub(OCA.Files.DetailsView.prototype, 'addDetailView'); + getDetailsStub = sinon.stub(OCA.Files.DetailsView.prototype, 'getDetailViews'); }); afterEach(function() { addTabStub.restore(); addDetailStub.restore(); + getDetailsStub.restore(); }); it('forward the registered views to the underlying DetailsView', function() { fileList.destroy(); @@ -2133,6 +2135,19 @@ describe('OCA.Files.FileList tests', function() { // twice because the filelist already registers one by default expect(addDetailStub.calledTwice).toEqual(true); }); + it('forward getting the registered views to the underlying DetailsView', function() { + fileList.destroy(); + fileList = new OCA.Files.FileList($('#app-content-files'), { + detailsViewEnabled: true + }); + var expectedRegisteredDetailsView = []; + getDetailsStub.returns(expectedRegisteredDetailsView); + + var registeredDetailViews = fileList.getRegisteredDetailViews(); + + expect(getDetailsStub.calledOnce).toEqual(true); + expect(registeredDetailViews).toEqual(expectedRegisteredDetailsView); + }); it('does not error when registering panels when not details view configured', function() { fileList.destroy(); fileList = new OCA.Files.FileList($('#app-content-files'), { @@ -2144,6 +2159,17 @@ describe('OCA.Files.FileList tests', function() { expect(addTabStub.notCalled).toEqual(true); expect(addDetailStub.notCalled).toEqual(true); }); + it('returns null when getting the registered views when not details view configured', function() { + fileList.destroy(); + fileList = new OCA.Files.FileList($('#app-content-files'), { + detailsViewEnabled: false + }); + + var registeredDetailViews = fileList.getRegisteredDetailViews(); + + expect(getDetailsStub.notCalled).toEqual(true); + expect(registeredDetailViews).toBeNull(); + }); }); it('triggers file action when clicking on row if no details view configured', function() { fileList.destroy(); From 2384703cf64f804c0cd0bc5444b5247447e1c003 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 9 Jun 2017 07:55:27 +0200 Subject: [PATCH 4/7] Trigger pre and post render events in MainFileInfoDetailsView MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The post-render event makes possible to modify the MainFileInfoDetailsView element once it has been rendered, which is needed by OCA.SystemTags.FilesPlugin to add the "Tags" label to the file details, while the pre-render event makes possible to detach added elements if needed before the MainFileInfoDetailsView is rendered again, as that removes the events from the child DOM elements even if they belong to other views. Signed-off-by: Daniel Calviño Sánchez --- apps/files/js/mainfileinfodetailview.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/files/js/mainfileinfodetailview.js b/apps/files/js/mainfileinfodetailview.js index f5f9e926f64..d703ca8371c 100644 --- a/apps/files/js/mainfileinfodetailview.js +++ b/apps/files/js/mainfileinfodetailview.js @@ -150,6 +150,8 @@ * Renders this details view */ render: function() { + this.trigger('pre-render'); + if (this.model) { var isFavorite = (this.model.get('tags') || []).indexOf(OC.TAG_FAVORITE) >= 0; this.$el.html(this.template({ @@ -188,6 +190,8 @@ this.$el.empty(); } this.delegateEvents(); + + this.trigger('post-render'); } }); From 365d7918b2df671180ab6d61b5a044754d24eb66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 9 Jun 2017 08:57:34 +0200 Subject: [PATCH 5/7] Fix toggle element being removed when MainFileInfoView is rendered again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The toggle element was added to the MainFileInfoView element when SystemTagsInfoView was rendered. However, if the MainFileInfoView was rendered again after that the toggle element was removed. Therefore, instead of adding it when SystemTagsInfoView is rendered, the toggle element has to be added when MainFileInfoView triggers its "post-render" event. Note, however, that when MainFileInfoView is rendered all the events are removed from its child elements. As the toggle uses a "click" event either the event has to be added back or the element has to be detached before the MainFileInfoView is rendered. Fixes #4944 Signed-off-by: Daniel Calviño Sánchez --- apps/systemtags/js/filesplugin.js | 11 ++++++++++- apps/systemtags/js/systemtagsinfoview.js | 25 +++++++++++++++--------- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/apps/systemtags/js/filesplugin.js b/apps/systemtags/js/filesplugin.js index db97b91a072..229d64da9d5 100644 --- a/apps/systemtags/js/filesplugin.js +++ b/apps/systemtags/js/filesplugin.js @@ -31,7 +31,16 @@ return; } - fileList.registerDetailView(new OCA.SystemTags.SystemTagsInfoView()); + var systemTagsInfoView = new OCA.SystemTags.SystemTagsInfoView(); + fileList.registerDetailView(systemTagsInfoView); + + _.each(fileList.getRegisteredDetailViews(), function(detailView) { + if (detailView instanceof OCA.Files.MainFileInfoDetailView) { + systemTagsInfoView.setMainFileInfoView(detailView); + + return; + } + }); } }; diff --git a/apps/systemtags/js/systemtagsinfoview.js b/apps/systemtags/js/systemtagsinfoview.js index f98c4b046e4..ce5de2048ab 100644 --- a/apps/systemtags/js/systemtagsinfoview.js +++ b/apps/systemtags/js/systemtagsinfoview.js @@ -63,6 +63,13 @@ this._toggleHandle = $('').addClass('tag-label').text(t('systemtags', 'Tags')); this._toggleHandle.prepend($('').addClass('icon icon-tag')); + + this._toggleHandle.on('click', function () { + self.$el.toggleClass('hidden'); + if (!self.$el.hasClass('hidden')) { + self.$el.find('.systemTagsInputField').select2('open'); + } + }); }, /** @@ -112,6 +119,15 @@ this.selectedTagsCollection.remove(tagId); }, + setMainFileInfoView: function(mainFileInfoView) { + this.listenTo(mainFileInfoView, 'pre-render', function() { + this._toggleHandle.detach(); + }); + this.listenTo(mainFileInfoView, 'post-render', function() { + mainFileInfoView.$el.find('.file-details').append(this._toggleHandle); + }); + }, + setFileInfo: function(fileInfo) { var self = this; if (!this._rendered) { @@ -147,15 +163,6 @@ this.$el.append(this._inputView.$el); this._inputView.render(); - - $('#app-sidebar').find('.mainFileInfoView .file-details').append(this._toggleHandle); - this._toggleHandle.off('click'); - this._toggleHandle.on('click', function () { - self.$el.toggleClass('hidden'); - if (!self.$el.hasClass('hidden')) { - self.$el.find('.systemTagsInputField').select2('open'); - } - }); }, remove: function() { From 5985ecb66dca8fac0f0ed87ef4791cc6f8b9e469 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 9 Jun 2017 03:34:56 +0200 Subject: [PATCH 6/7] Add visibility related methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SystemTagsInfoView now provides public methods related to its visibility in preparation to be used by external objects. Signed-off-by: Daniel Calviño Sánchez --- apps/systemtags/js/systemtagsinfoview.js | 30 +++++++++--- .../tests/js/systemtagsinfoviewSpec.js | 46 +++++++++++++++++++ 2 files changed, 70 insertions(+), 6 deletions(-) diff --git a/apps/systemtags/js/systemtagsinfoview.js b/apps/systemtags/js/systemtagsinfoview.js index ce5de2048ab..d42bf18761b 100644 --- a/apps/systemtags/js/systemtagsinfoview.js +++ b/apps/systemtags/js/systemtagsinfoview.js @@ -65,9 +65,11 @@ this._toggleHandle.prepend($('').addClass('icon icon-tag')); this._toggleHandle.on('click', function () { - self.$el.toggleClass('hidden'); - if (!self.$el.hasClass('hidden')) { - self.$el.find('.systemTagsInputField').select2('open'); + if (self.isVisible()) { + self.hide(); + } else { + self.show(); + self.openDropdown(); } }); }, @@ -144,15 +146,15 @@ self._inputView.setData(appliedTags); if (appliedTags.length !== 0) { - self.$el.removeClass('hidden'); + self.show(); } else { - self.$el.addClass('hidden'); + self.hide(); } } }); } - this.$el.addClass('hidden'); + this.hide(); }, /** @@ -165,6 +167,22 @@ this._inputView.render(); }, + isVisible: function() { + return !this.$el.hasClass('hidden'); + }, + + show: function() { + this.$el.removeClass('hidden'); + }, + + hide: function() { + this.$el.addClass('hidden'); + }, + + openDropdown: function() { + this.$el.find('.systemTagsInputField').select2('open'); + }, + remove: function() { this._inputView.remove(); this._toggleHandle.remove(); diff --git a/apps/systemtags/tests/js/systemtagsinfoviewSpec.js b/apps/systemtags/tests/js/systemtagsinfoviewSpec.js index 449dfd859d7..2f874688112 100644 --- a/apps/systemtags/tests/js/systemtagsinfoviewSpec.js +++ b/apps/systemtags/tests/js/systemtagsinfoviewSpec.js @@ -201,4 +201,50 @@ describe('OCA.SystemTags.SystemTagsInfoView tests', function() { }); }); + describe('visibility', function() { + it('reports visibility based on the "hidden" class name', function() { + view.$el.addClass('hidden'); + + expect(view.isVisible()).toBeFalsy(); + + view.$el.removeClass('hidden'); + + expect(view.isVisible()).toBeTruthy(); + }); + it('is not visible after rendering', function() { + view.render(); + + expect(view.isVisible()).toBeFalsy(); + }); + it('shows and hides the element', function() { + view.show(); + + expect(view.isVisible()).toBeTruthy(); + + view.hide(); + + expect(view.isVisible()).toBeFalsy(); + + view.show(); + + expect(view.isVisible()).toBeTruthy(); + }); + }); + describe('select2', function() { + var select2Stub; + + beforeEach(function() { + select2Stub = sinon.stub($.fn, 'select2'); + }); + afterEach(function() { + select2Stub.restore(); + }); + it('opens dropdown', function() { + view.openDropdown(); + + expect(select2Stub.calledOnce).toBeTruthy(); + expect(select2Stub.thisValues[0].selector).toEqual('.systemTagsInputField'); + expect(select2Stub.withArgs('open')).toBeTruthy(); + }); + }); }); From c9bc52153251863776e8b506533b29520ac12cee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 9 Jun 2017 09:09:41 +0200 Subject: [PATCH 7/7] Extract toggle visibility of a SystemTagsInfoView to its own view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SystemTagsInfoViewToggleView is a basic view that renders a label that, when clicked, toggles the visibility of an associated SystemTagsInfoView. In order to keep the view parent agnostic its attachment and detachment to/from the MainfFileInfoView is done in the FilesPlugin. Signed-off-by: Daniel Calviño Sánchez --- apps/systemtags/js/filesplugin.js | 16 ++- apps/systemtags/js/merged.json | 3 +- apps/systemtags/js/systemtagsinfoview.js | 24 ---- .../js/systemtagsinfoviewtoggleview.js | 103 ++++++++++++++++++ .../js/systemtagsinfoviewtoggleviewSpec.js | 93 ++++++++++++++++ tests/karma.config.js | 1 + 6 files changed, 214 insertions(+), 26 deletions(-) create mode 100644 apps/systemtags/js/systemtagsinfoviewtoggleview.js create mode 100644 apps/systemtags/tests/js/systemtagsinfoviewtoggleviewSpec.js diff --git a/apps/systemtags/js/filesplugin.js b/apps/systemtags/js/filesplugin.js index 229d64da9d5..fc2a227b5be 100644 --- a/apps/systemtags/js/filesplugin.js +++ b/apps/systemtags/js/filesplugin.js @@ -36,7 +36,21 @@ _.each(fileList.getRegisteredDetailViews(), function(detailView) { if (detailView instanceof OCA.Files.MainFileInfoDetailView) { - systemTagsInfoView.setMainFileInfoView(detailView); + var systemTagsInfoViewToggleView = + new OCA.SystemTags.SystemTagsInfoViewToggleView({ + systemTagsInfoView: systemTagsInfoView + }); + systemTagsInfoViewToggleView.render(); + + // The toggle view element is detached before the + // MainFileInfoDetailView is rendered to prevent its event + // handlers from being removed. + systemTagsInfoViewToggleView.listenTo(detailView, 'pre-render', function() { + systemTagsInfoViewToggleView.$el.detach(); + }); + systemTagsInfoViewToggleView.listenTo(detailView, 'post-render', function() { + detailView.$el.find('.file-details').append(systemTagsInfoViewToggleView.$el); + }); return; } diff --git a/apps/systemtags/js/merged.json b/apps/systemtags/js/merged.json index 0262077498a..632abf6777e 100644 --- a/apps/systemtags/js/merged.json +++ b/apps/systemtags/js/merged.json @@ -2,5 +2,6 @@ "app.js", "systemtagsfilelist.js", "filesplugin.js", - "systemtagsinfoview.js" + "systemtagsinfoview.js", + "systemtagsinfoviewtoggleview.js" ] diff --git a/apps/systemtags/js/systemtagsinfoview.js b/apps/systemtags/js/systemtagsinfoview.js index d42bf18761b..1bf7287342f 100644 --- a/apps/systemtags/js/systemtagsinfoview.js +++ b/apps/systemtags/js/systemtagsinfoview.js @@ -37,8 +37,6 @@ */ _inputView: null, - _toggleHandle: null, - initialize: function(options) { var self = this; options = options || {}; @@ -60,18 +58,6 @@ this._inputView.on('select', this._onSelectTag, this); this._inputView.on('deselect', this._onDeselectTag, this); - - this._toggleHandle = $('').addClass('tag-label').text(t('systemtags', 'Tags')); - this._toggleHandle.prepend($('').addClass('icon icon-tag')); - - this._toggleHandle.on('click', function () { - if (self.isVisible()) { - self.hide(); - } else { - self.show(); - self.openDropdown(); - } - }); }, /** @@ -121,15 +107,6 @@ this.selectedTagsCollection.remove(tagId); }, - setMainFileInfoView: function(mainFileInfoView) { - this.listenTo(mainFileInfoView, 'pre-render', function() { - this._toggleHandle.detach(); - }); - this.listenTo(mainFileInfoView, 'post-render', function() { - mainFileInfoView.$el.find('.file-details').append(this._toggleHandle); - }); - }, - setFileInfo: function(fileInfo) { var self = this; if (!this._rendered) { @@ -185,7 +162,6 @@ remove: function() { this._inputView.remove(); - this._toggleHandle.remove(); } }); diff --git a/apps/systemtags/js/systemtagsinfoviewtoggleview.js b/apps/systemtags/js/systemtagsinfoviewtoggleview.js new file mode 100644 index 00000000000..13a48e49cfb --- /dev/null +++ b/apps/systemtags/js/systemtagsinfoviewtoggleview.js @@ -0,0 +1,103 @@ +/** + * + * @copyright Copyright (c) 2017, Daniel Calviño Sánchez (danxuliu@gmail.com) + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +(function(OCA) { + + var TEMPLATE = + '' + t('systemtags', 'Tags'); + + /** + * @class OCA.SystemTags.SystemTagsInfoViewToggleView + * @classdesc + * + * View to toggle the visibility of a SystemTagsInfoView. + * + * This toggle view must be explicitly rendered before it is used. + */ + var SystemTagsInfoViewToggleView = OC.Backbone.View.extend( + /** @lends OC.Backbone.View.prototype */ { + + tagName: 'span', + + className: 'tag-label', + + events: { + 'click': 'click' + }, + + /** + * @type OCA.SystemTags.SystemTagsInfoView + */ + _systemTagsInfoView: null, + + template: function(data) { + if (!this._template) { + this._template = Handlebars.compile(TEMPLATE); + } + return this._template(data); + }, + + /** + * Initialize this toggle view. + * + * The options must provide a systemTagsInfoView parameter that + * references the SystemTagsInfoView to associate to this toggle view. + */ + initialize: function(options) { + var self = this; + options = options || {}; + + this._systemTagsInfoView = options.systemTagsInfoView; + if (!this._systemTagsInfoView) { + throw 'Missing required parameter "systemTagsInfoView"'; + } + }, + + /** + * Toggles the visibility of the associated SystemTagsInfoView. + * + * When the systemTagsInfoView is shown its dropdown is also opened. + */ + click: function() { + if (this._systemTagsInfoView.isVisible()) { + this._systemTagsInfoView.hide(); + } else { + this._systemTagsInfoView.show(); + this._systemTagsInfoView.openDropdown(); + } + }, + + /** + * Renders this toggle view. + * + * @return OCA.SystemTags.SystemTagsInfoViewToggleView this object. + */ + render: function() { + this.$el.html(this.template()); + + return this; + }, + + }); + + OCA.SystemTags.SystemTagsInfoViewToggleView = SystemTagsInfoViewToggleView; + +})(OCA); diff --git a/apps/systemtags/tests/js/systemtagsinfoviewtoggleviewSpec.js b/apps/systemtags/tests/js/systemtagsinfoviewtoggleviewSpec.js new file mode 100644 index 00000000000..5e6c2c820e9 --- /dev/null +++ b/apps/systemtags/tests/js/systemtagsinfoviewtoggleviewSpec.js @@ -0,0 +1,93 @@ +/** + * + * @copyright Copyright (c) 2017, Daniel Calviño Sánchez (danxuliu@gmail.com) + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +describe('OCA.SystemTags.SystemTagsInfoViewToggleView', function () { + + var systemTagsInfoView; + var view; + + beforeEach(function() { + systemTagsInfoView = new OCA.SystemTags.SystemTagsInfoView(); + view = new OCA.SystemTags.SystemTagsInfoViewToggleView({ systemTagsInfoView: systemTagsInfoView }); + }); + + afterEach(function() { + view.remove(); + systemTagsInfoView.remove(); + }); + + describe('initialize', function() { + it('fails if a "systemTagsInfoView" parameter is not provided', function() { + var constructor = function() { + return new OCA.SystemTags.SystemTagsInfoViewToggleView({}); + } + + expect(constructor).toThrow(); + }); + }); + + describe('click on element', function() { + + var isVisibleStub; + var showStub; + var hideStub; + var openDropdownStub; + + beforeEach(function() { + isVisibleStub = sinon.stub(systemTagsInfoView, 'isVisible'); + showStub = sinon.stub(systemTagsInfoView, 'show'); + hideStub = sinon.stub(systemTagsInfoView, 'hide'); + openDropdownStub = sinon.stub(systemTagsInfoView, 'openDropdown'); + }); + + afterEach(function() { + isVisibleStub.restore(); + showStub.restore(); + hideStub.restore(); + openDropdownStub.restore(); + }); + + it('shows a not visible SystemTagsInfoView', function() { + isVisibleStub.returns(false); + + view.$el.click(); + + expect(isVisibleStub.calledOnce).toBeTruthy(); + expect(showStub.calledOnce).toBeTruthy(); + expect(openDropdownStub.calledOnce).toBeTruthy(); + expect(openDropdownStub.calledAfter(showStub)).toBeTruthy(); + expect(hideStub.notCalled).toBeTruthy(); + }); + + it('hides a visible SystemTagsInfoView', function() { + isVisibleStub.returns(true); + + view.$el.click(); + + expect(isVisibleStub.calledOnce).toBeTruthy(); + expect(hideStub.calledOnce).toBeTruthy(); + expect(showStub.notCalled).toBeTruthy(); + expect(openDropdownStub.notCalled).toBeTruthy(); + }); + + }); + +}); diff --git a/tests/karma.config.js b/tests/karma.config.js index 62b5171dcd1..07dc2965346 100644 --- a/tests/karma.config.js +++ b/tests/karma.config.js @@ -102,6 +102,7 @@ module.exports = function(config) { // need to enforce loading order... 'apps/systemtags/js/app.js', 'apps/systemtags/js/systemtagsinfoview.js', + 'apps/systemtags/js/systemtagsinfoviewtoggleview.js', 'apps/systemtags/js/systemtagsfilelist.js', 'apps/systemtags/js/filesplugin.js' ],