From a9552de089b7d587ffa94b79560436a1f65c2ea8 Mon Sep 17 00:00:00 2001
From: Daniel Calviño Sánchez <danxuliu@gmail.com>
Date: Mon, 19 Feb 2018 20:42:29 +0100
Subject: Set the width of the parent element in breadcrumb tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Setting the width of the parent element of the breadcrumbs and then
explicitly calling "_resize" is enough to test the resizing behaviour.
This makes possible to remove the "setMaxWidth" method and its related
code, which was used only for testing purposes.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
---
 apps/files/js/breadcrumb.js           | 20 +-------------------
 apps/files/tests/js/breadcrumbSpec.js | 21 +++++++++++++--------
 2 files changed, 14 insertions(+), 27 deletions(-)

diff --git a/apps/files/js/breadcrumb.js b/apps/files/js/breadcrumb.js
index 20b15e3cb93..fe8e54c3a6c 100644
--- a/apps/files/js/breadcrumb.js
+++ b/apps/files/js/breadcrumb.js
@@ -238,19 +238,6 @@
 			return crumbs;
 		},
 
-		/**
-		 * Show/hide breadcrumbs to fit the given width
-		 * Mostly used by tests
-		 *
-		 * @param {int} availableWidth available width
-		 */
-		setMaxWidth: function (availableWidth) {
-			if (this.availableWidth !== availableWidth) {
-				this.availableWidth = availableWidth;
-				this._resize();
-			}
-		},
-
 		/**
 		 * Calculate real width based on individual crumbs
 		 * More accurate and works with tests
@@ -329,12 +316,7 @@
 				return;
 			}
 
-			// Used for testing since this.$el.parent fails
-			if (!this.availableWidth) {
-				this.usedWidth = this.$el.parent().width() - this.$el.parent().find('.actions.creatable').width();
-			} else {
-				this.usedWidth = this.availableWidth;
-			}
+			this.usedWidth = this.$el.parent().width() - this.$el.parent().find('.actions.creatable').width();
 
 			// If container is smaller than content
 			// AND if there are crumbs left to hide
diff --git a/apps/files/tests/js/breadcrumbSpec.js b/apps/files/tests/js/breadcrumbSpec.js
index 5ec5ad2d6e8..d30d9c48e24 100644
--- a/apps/files/tests/js/breadcrumbSpec.js
+++ b/apps/files/tests/js/breadcrumbSpec.js
@@ -188,7 +188,7 @@ describe('OCA.Files.BreadCrumb tests', function() {
 			$('#controls').append(bc.$el);
 
 			// Shrink to show popovermenu
-			bc.setMaxWidth(300);
+			$('#controls').width(300);
 
 			// triggers resize implicitly
 			bc.setDirectory(dummyDir);
@@ -265,10 +265,11 @@ describe('OCA.Files.BreadCrumb tests', function() {
 		afterEach(function() {
 			bc = null;
 		});
-		it('Hides breadcrumbs to fit max allowed width', function() {
+		it('Hides breadcrumbs to fit available width', function() {
 			var $crumbs;
 
-			bc.setMaxWidth(500);
+			$('#controls').width(500);
+			bc._resize();
 
 			$crumbs = bc.$el.find('.crumb');
 
@@ -283,10 +284,11 @@ 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() {
+		it('Hides breadcrumbs to fit available width', function() {
 			var $crumbs;
 
-			bc.setMaxWidth(700);
+			$('#controls').width(700);
+			bc._resize();
 
 			$crumbs = bc.$el.find('.crumb');
 
@@ -301,11 +303,13 @@ describe('OCA.Files.BreadCrumb tests', function() {
 			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() {
+		it('Updates the breadcrumbs when reducing available width', function() {
 			var $crumbs;
 
 			// enough space
-			bc.setMaxWidth(1800);
+			$('#controls').width(1800);
+			bc._resize();
+
 			$crumbs = bc.$el.find('.crumb');
 
 			// Menu is hidden
@@ -315,7 +319,8 @@ describe('OCA.Files.BreadCrumb tests', function() {
 			bc.setDirectory(dummyDir);
 
 			// simulate decrease
-			bc.setMaxWidth(950);
+			$('#controls').width(950);
+			bc._resize();
 
 			// Menu and home are always visible
 			expect($crumbs.eq(0).hasClass('hidden')).toEqual(false);
-- 
cgit v1.2.3


From a828e7b12a157d4636f5659856f45982c9cc1621 Mon Sep 17 00:00:00 2001
From: Daniel Calviño Sánchez <danxuliu@gmail.com>
Date: Mon, 19 Feb 2018 20:46:49 +0100
Subject: Replace attribute with local variable
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The "usedWidth" attribute was not used elsewhere outside the "_resize"
method, so it was replaced with a local variable. Moreover, it was also
renamed to a more suitable name ("availableWidth").

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
---
 apps/files/js/breadcrumb.js | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/apps/files/js/breadcrumb.js b/apps/files/js/breadcrumb.js
index fe8e54c3a6c..c02ed488da0 100644
--- a/apps/files/js/breadcrumb.js
+++ b/apps/files/js/breadcrumb.js
@@ -316,18 +316,18 @@
 				return;
 			}
 
-			this.usedWidth = this.$el.parent().width() - this.$el.parent().find('.actions.creatable').width();
+			var availableWidth = this.$el.parent().width() - this.$el.parent().find('.actions.creatable').width();
 
 			// If container is smaller than content
 			// AND if there are crumbs left to hide
-			while (this.getTotalWidth() > this.usedWidth
+			while (this.getTotalWidth() > availableWidth
 				&& this.$el.find(this.crumbSelector).length > 0) {
 				this._hideCrumb();
 			}
 			// If container is bigger than content + element to be shown
 			// AND if there is at least one hidden crumb
 			while (this.$el.find('.crumb.hidden').length > 0
-				&& this.getTotalWidth() + this._getCrumbElement().width() < this.usedWidth) {
+				&& this.getTotalWidth() + this._getCrumbElement().width() < availableWidth) {
 				this._showCrumb();
 			}
 
-- 
cgit v1.2.3


From 3850221ae118f05df4fff9016606ad2572c0057f Mon Sep 17 00:00:00 2001
From: Daniel Calviño Sánchez <danxuliu@gmail.com>
Date: Mon, 19 Feb 2018 21:06:41 +0100
Subject: Do not render the breadcrumbs again in resize tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

There is no need to call "setDirectory" again in resize tests; it is
enough to simply resize them (and isolates them better to just test the
resizing behaviour).

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
---
 apps/files/tests/js/breadcrumbSpec.js | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/apps/files/tests/js/breadcrumbSpec.js b/apps/files/tests/js/breadcrumbSpec.js
index d30d9c48e24..9898c556cfd 100644
--- a/apps/files/tests/js/breadcrumbSpec.js
+++ b/apps/files/tests/js/breadcrumbSpec.js
@@ -315,9 +315,6 @@ describe('OCA.Files.BreadCrumb tests', function() {
 			// Menu is hidden
 			expect($crumbs.eq(0).hasClass('hidden')).toEqual(true);
 
-			// triggers resize implicitly
-			bc.setDirectory(dummyDir);
-
 			// simulate decrease
 			$('#controls').width(950);
 			bc._resize();
-- 
cgit v1.2.3


From e37c4fd7f308be6f4dbece2c6cd2e8ff2d211c09 Mon Sep 17 00:00:00 2001
From: Daniel Calviño Sánchez <danxuliu@gmail.com>
Date: Mon, 19 Feb 2018 23:47:19 +0100
Subject: Take padding and margin of the creatable actions div into account
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

There are some differences in width handling between the browsers used
to run the tests, most likely due to their support (or lack of) of
certain CSS features: PhantomJS requires "width" to be set (probably
because it does not handle flex displays and treats it like a block, so
"min-width" does not matter in this case), while Firefox requires
"min-width" to be set (otherwise the children of "#controls" could be
compressed due to its use of flex display and the elements would end
with a different width than the one needed for the tests). Due to all
that the width of the breadcrumb siblings must be specified in the tests
using both "width" and "min-width".

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
---
 apps/files/js/breadcrumb.js           |  2 +-
 apps/files/tests/js/breadcrumbSpec.js | 82 +++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/apps/files/js/breadcrumb.js b/apps/files/js/breadcrumb.js
index c02ed488da0..16762f386be 100644
--- a/apps/files/js/breadcrumb.js
+++ b/apps/files/js/breadcrumb.js
@@ -316,7 +316,7 @@
 				return;
 			}
 
-			var availableWidth = this.$el.parent().width() - this.$el.parent().find('.actions.creatable').width();
+			var availableWidth = this.$el.parent().width() - this.$el.parent().find('.actions.creatable').outerWidth(true);
 
 			// If container is smaller than content
 			// AND if there are crumbs left to hide
diff --git a/apps/files/tests/js/breadcrumbSpec.js b/apps/files/tests/js/breadcrumbSpec.js
index 9898c556cfd..dcb9391f6ed 100644
--- a/apps/files/tests/js/breadcrumbSpec.js
+++ b/apps/files/tests/js/breadcrumbSpec.js
@@ -303,6 +303,88 @@ describe('OCA.Files.BreadCrumb tests', function() {
 			expect($crumbs.eq(6).hasClass('hidden')).toEqual(false);
 			expect($crumbs.eq(7).hasClass('hidden')).toEqual(false);
 		});
+		it('Hides breadcrumbs to fit available width left by siblings', function() {
+			var $crumbs;
+
+			$('#controls').width(700);
+			bc._resize();
+
+			$crumbs = bc.$el.find('.crumb');
+
+			// Third and fourth crumb are hidden and everything else is 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);
+
+			var $creatableActions = $('<div class="actions creatable"></div>');
+			// Set both the width and the min-width to even differences in width
+			// handling in the browsers used to run the tests.
+			$creatableActions.css('width', '200px');
+			$creatableActions.css('min-width', '200px');
+			$('#controls').append($creatableActions);
+
+			bc._resize();
+
+			// Second, third, fourth and fifth crumb are hidden and everything
+			// else is 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(true);
+			expect($crumbs.eq(4).hasClass('hidden')).toEqual(true);
+			expect($crumbs.eq(5).hasClass('hidden')).toEqual(true);
+			expect($crumbs.eq(6).hasClass('hidden')).toEqual(true);
+			expect($crumbs.eq(7).hasClass('hidden')).toEqual(false);
+		});
+		it('Hides breadcrumbs to fit available width left by siblings with paddings and margins', function() {
+			var $crumbs;
+
+			$('#controls').width(700);
+			bc._resize();
+
+			$crumbs = bc.$el.find('.crumb');
+
+			// Third and fourth crumb are hidden and everything else is 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);
+
+			var $creatableActions = $('<div class="actions creatable"></div>');
+			// Set both the width and the min-width to even differences in width
+			// handling in the browsers used to run the tests.
+			$creatableActions.css('width', '20px');
+			$creatableActions.css('min-width', '20px');
+			$creatableActions.css('margin-left', '90px');
+			$creatableActions.css('padding-right', '90px');
+			$('#controls').append($creatableActions);
+
+			bc._resize();
+
+			// Second, third, fourth and fifth crumb are hidden and everything
+			// else is 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(true);
+			expect($crumbs.eq(4).hasClass('hidden')).toEqual(true);
+			expect($crumbs.eq(5).hasClass('hidden')).toEqual(true);
+			expect($crumbs.eq(6).hasClass('hidden')).toEqual(true);
+			expect($crumbs.eq(7).hasClass('hidden')).toEqual(false);
+		});
 		it('Updates the breadcrumbs when reducing available width', function() {
 			var $crumbs;
 
-- 
cgit v1.2.3


From a37007f872df68dec9038cf43846f8bd998ff19b Mon Sep 17 00:00:00 2001
From: Daniel Calviño Sánchez <danxuliu@gmail.com>
Date: Tue, 20 Feb 2018 00:11:40 +0100
Subject: Take all visible siblings into account
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Other apps could add elements to the controls outside the creatable
actions div (for example, the button to switch to the gallery), so the
widths of all the visible siblings of the breadcrumbs have to be taken
into account in the size calculations.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
---
 apps/files/js/breadcrumb.js           | 10 ++++++-
 apps/files/tests/js/breadcrumbSpec.js | 54 ++++++++++++++++++++++++++++++++---
 2 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/apps/files/js/breadcrumb.js b/apps/files/js/breadcrumb.js
index 16762f386be..4a61188e93a 100644
--- a/apps/files/js/breadcrumb.js
+++ b/apps/files/js/breadcrumb.js
@@ -316,7 +316,15 @@
 				return;
 			}
 
-			var availableWidth = this.$el.parent().width() - this.$el.parent().find('.actions.creatable').outerWidth(true);
+			var siblingsWidth = 0;
+			this.$el.prevAll(':visible').each(function () {
+				siblingsWidth += $(this).outerWidth(true);
+			});
+			this.$el.nextAll(':visible').each(function () {
+				siblingsWidth += $(this).outerWidth(true);
+			});
+
+			var availableWidth = this.$el.parent().width() - siblingsWidth;
 
 			// If container is smaller than content
 			// AND if there are crumbs left to hide
diff --git a/apps/files/tests/js/breadcrumbSpec.js b/apps/files/tests/js/breadcrumbSpec.js
index dcb9391f6ed..bae8370cd1d 100644
--- a/apps/files/tests/js/breadcrumbSpec.js
+++ b/apps/files/tests/js/breadcrumbSpec.js
@@ -322,13 +322,35 @@ describe('OCA.Files.BreadCrumb tests', function() {
 			expect($crumbs.eq(6).hasClass('hidden')).toEqual(false);
 			expect($crumbs.eq(7).hasClass('hidden')).toEqual(false);
 
+			// Visible sibling widths add up to 200px
+			var $previousSibling = $('<div class="otherSibling"></div>');
+			// Set both the width and the min-width to even differences in width
+			// handling in the browsers used to run the tests.
+			$previousSibling.css('width', '50px');
+			$previousSibling.css('min-width', '50px');
+			$('#controls').prepend($previousSibling);
+
 			var $creatableActions = $('<div class="actions creatable"></div>');
 			// Set both the width and the min-width to even differences in width
 			// handling in the browsers used to run the tests.
-			$creatableActions.css('width', '200px');
-			$creatableActions.css('min-width', '200px');
+			$creatableActions.css('width', '100px');
+			$creatableActions.css('min-width', '100px');
 			$('#controls').append($creatableActions);
 
+			var $nextHiddenSibling = $('<div class="otherSibling hidden"></div>');
+			// Set both the width and the min-width to even differences in width
+			// handling in the browsers used to run the tests.
+			$nextHiddenSibling.css('width', '200px');
+			$nextHiddenSibling.css('min-width', '200px');
+			$('#controls').append($nextHiddenSibling);
+
+			var $nextSibling = $('<div class="otherSibling"></div>');
+			// Set both the width and the min-width to even differences in width
+			// handling in the browsers used to run the tests.
+			$nextSibling.css('width', '50px');
+			$nextSibling.css('min-width', '50px');
+			$('#controls').append($nextSibling);
+
 			bc._resize();
 
 			// Second, third, fourth and fifth crumb are hidden and everything
@@ -362,15 +384,39 @@ describe('OCA.Files.BreadCrumb tests', function() {
 			expect($crumbs.eq(6).hasClass('hidden')).toEqual(false);
 			expect($crumbs.eq(7).hasClass('hidden')).toEqual(false);
 
+			// Visible sibling widths add up to 200px
+			var $previousSibling = $('<div class="otherSibling"></div>');
+			// Set both the width and the min-width to even differences in width
+			// handling in the browsers used to run the tests.
+			$previousSibling.css('width', '10px');
+			$previousSibling.css('min-width', '10px');
+			$previousSibling.css('margin', '20px');
+			$('#controls').prepend($previousSibling);
+
 			var $creatableActions = $('<div class="actions creatable"></div>');
 			// Set both the width and the min-width to even differences in width
 			// handling in the browsers used to run the tests.
 			$creatableActions.css('width', '20px');
 			$creatableActions.css('min-width', '20px');
-			$creatableActions.css('margin-left', '90px');
-			$creatableActions.css('padding-right', '90px');
+			$creatableActions.css('margin-left', '40px');
+			$creatableActions.css('padding-right', '40px');
 			$('#controls').append($creatableActions);
 
+			var $nextHiddenSibling = $('<div class="otherSibling hidden"></div>');
+			// Set both the width and the min-width to even differences in width
+			// handling in the browsers used to run the tests.
+			$nextHiddenSibling.css('width', '200px');
+			$nextHiddenSibling.css('min-width', '200px');
+			$('#controls').append($nextHiddenSibling);
+
+			var $nextSibling = $('<div class="otherSibling"></div>');
+			// Set both the width and the min-width to even differences in width
+			// handling in the browsers used to run the tests.
+			$nextSibling.css('width', '10px');
+			$nextSibling.css('min-width', '10px');
+			$nextSibling.css('padding', '20px');
+			$('#controls').append($nextSibling);
+
 			bc._resize();
 
 			// Second, third, fourth and fifth crumb are hidden and everything
-- 
cgit v1.2.3


From acdf091f8463aadadd92be037e64102d2c8cf34c Mon Sep 17 00:00:00 2001
From: Daniel Calviño Sánchez <danxuliu@gmail.com>
Date: Tue, 20 Feb 2018 17:07:26 +0100
Subject: Compress siblings before calculating the available width for crumbs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When the parent element of the breadcrumbs was resized to a larger width
and the siblings of the breadcrumbs expanded to fill all the available
width some crumbs could be hidden even if there was enough room for
them. The reason was that the width of the siblings being used to
calculate the available width for the breadcrumbs was the expanded width
of the siblings. Now as many crumbs as possible (that is, fitting in the
parent, no matter the siblings) are first shown so the expanding
siblings are compressed before calculating the available width.

Due to the lack of support for flexboxes in PhantomJS the related unit
test is skipped; it has to be run in other browser, like Firefox.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
---
 apps/files/js/breadcrumb.js           |  9 ++++
 apps/files/tests/js/breadcrumbSpec.js | 87 +++++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+)

diff --git a/apps/files/js/breadcrumb.js b/apps/files/js/breadcrumb.js
index 4a61188e93a..347d5a5dd0d 100644
--- a/apps/files/js/breadcrumb.js
+++ b/apps/files/js/breadcrumb.js
@@ -316,6 +316,15 @@
 				return;
 			}
 
+			// Show the crumbs to compress the siblings before hidding again the
+			// crumbs. This is needed when the siblings expand to fill all the
+			// available width, as in that case their old width would limit the
+			// available width for the crumbs.
+			while (this.$el.find('.crumb.hidden').length > 0
+				&& this.getTotalWidth() < this.$el.parent().width()) {
+				this._showCrumb();
+			}
+
 			var siblingsWidth = 0;
 			this.$el.prevAll(':visible').each(function () {
 				siblingsWidth += $(this).outerWidth(true);
diff --git a/apps/files/tests/js/breadcrumbSpec.js b/apps/files/tests/js/breadcrumbSpec.js
index bae8370cd1d..392d4bf3fe1 100644
--- a/apps/files/tests/js/breadcrumbSpec.js
+++ b/apps/files/tests/js/breadcrumbSpec.js
@@ -238,6 +238,10 @@ describe('OCA.Files.BreadCrumb tests', function() {
 	describe('Resizing', function() {
 		var bc, dummyDir, widths;
 
+		// cit() will skip tests if running on PhantomJS because it does not
+		// have proper support for flexboxes.
+		var cit = window.isPhantom?xit:it;
+
 		beforeEach(function() {
 			dummyDir = '/short name/longer name/looooooooooooonger/' +
 				'even longer long long long longer long/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/last one';
@@ -451,6 +455,89 @@ describe('OCA.Files.BreadCrumb tests', function() {
 			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(false);
+			expect($crumbs.eq(6).hasClass('hidden')).toEqual(false);
+			expect($crumbs.eq(7).hasClass('hidden')).toEqual(false);
+		});
+		it('Updates the breadcrumbs when increasing available width', function() {
+			var $crumbs;
+
+			// limited space
+			$('#controls').width(850);
+			bc._resize();
+
+			$crumbs = bc.$el.find('.crumb');
+
+			// Third and fourth crumb are hidden and everything else is 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);
+
+			// simulate increase
+			$('#controls').width(1000);
+			bc._resize();
+
+			// Third crumb is hidden and everything else is 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(false);
+			expect($crumbs.eq(6).hasClass('hidden')).toEqual(false);
+			expect($crumbs.eq(7).hasClass('hidden')).toEqual(false);
+		});
+		cit('Updates the breadcrumbs when increasing available width with an expanding sibling', function() {
+			var $crumbs;
+
+			// The sibling expands to fill all the width left by the breadcrumbs
+			var $nextSibling = $('<div class="sibling"></div>');
+			// Set both the width and the min-width to even differences in width
+			// handling in the browsers used to run the tests.
+			$nextSibling.css('width', '10px');
+			$nextSibling.css('min-width', '10px');
+			$nextSibling.css('display', 'flex');
+			$nextSibling.css('flex', '1 1');
+			var $nextSiblingChild = $('<div class="siblingChild"></div>');
+			$nextSiblingChild.css('margin-left', 'auto');
+			$nextSibling.append($nextSiblingChild);
+			$('#controls').append($nextSibling);
+
+			// limited space
+			$('#controls').width(850);
+			bc._resize();
+
+			$crumbs = bc.$el.find('.crumb');
+
+			// Third and fourth crumb are hidden and everything else is 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);
+
+			// simulate increase
+			$('#controls').width(1000);
+			bc._resize();
+
+			// Third crumb is hidden and everything else is 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);
-- 
cgit v1.2.3


From 29c924f74b692a496606af1759f14d02c5dee9cc Mon Sep 17 00:00:00 2001
From: Daniel Calviño Sánchez <danxuliu@gmail.com>
Date: Tue, 27 Feb 2018 13:30:07 +0100
Subject: Use hard-coded values for paddings and margins
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This ensures that the resize tests do not depend on the values set in
the CSS files.

Note that this change causes a test to fail with Firefox, but not with
PhantomJS. This is due to a difference in the starting width used by
Firefox and by PhantomJS, and it will be fixed in a following commit.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
---
 apps/files/tests/js/breadcrumbSpec.js | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/apps/files/tests/js/breadcrumbSpec.js b/apps/files/tests/js/breadcrumbSpec.js
index 392d4bf3fe1..b1fd8d11769 100644
--- a/apps/files/tests/js/breadcrumbSpec.js
+++ b/apps/files/tests/js/breadcrumbSpec.js
@@ -236,7 +236,7 @@ describe('OCA.Files.BreadCrumb tests', function() {
 	});
 
 	describe('Resizing', function() {
-		var bc, dummyDir, widths;
+		var bc, dummyDir, widths, paddings, margins;
 
 		// cit() will skip tests if running on PhantomJS because it does not
 		// have proper support for flexboxes.
@@ -261,9 +261,15 @@ describe('OCA.Files.BreadCrumb tests', function() {
 			// results on different browsers due to font engine differences
 			// 51px is default size for menu and home
 			widths = [51, 51, 106, 112, 160, 257, 251, 91];
+			// using hard-coded paddings and margins to avoid depending on the
+			// current CSS values used in the server
+			paddings = [0, 0, 0, 0, 0, 0, 0, 0];
+			margins = [0, 0, 0, 0, 0, 0, 0, 0];
 
 			$('div.crumb').each(function(index){
 				$(this).css('width', widths[index]);
+				$(this).css('padding', paddings[index]);
+				$(this).css('margin', margins[index]);
 			});
 		});
 		afterEach(function() {
-- 
cgit v1.2.3


From 92345f2c38a4cddbca6b63d03e3031023fe92a1a Mon Sep 17 00:00:00 2001
From: Daniel Calviño Sánchez <danxuliu@gmail.com>
Date: Tue, 27 Feb 2018 13:59:09 +0100
Subject: Take padding and margins of crumbs into account
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When calculating the total width of the crumbs only its padding was
taken into account; now the margin is too. In a similar way, before
showing a crumb only its width was taken into account; now its padding
and margin are taken into account too.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
---
 apps/files/js/breadcrumb.js           |  4 +--
 apps/files/tests/js/breadcrumbSpec.js | 54 +++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/apps/files/js/breadcrumb.js b/apps/files/js/breadcrumb.js
index 347d5a5dd0d..e4951958ad9 100644
--- a/apps/files/js/breadcrumb.js
+++ b/apps/files/js/breadcrumb.js
@@ -249,7 +249,7 @@
 			for (var i = 0; i < this.breadcrumbs.length; i++ ) {
 				var $crumb = $(this.breadcrumbs[i]);
 				if(!$crumb.hasClass('hidden') || ignoreHidden === true) {
-					totalWidth += $crumb.outerWidth();
+					totalWidth += $crumb.outerWidth(true);
 				}
 			}
 			return totalWidth;
@@ -344,7 +344,7 @@
 			// If container is bigger than content + element to be shown
 			// AND if there is at least one hidden crumb
 			while (this.$el.find('.crumb.hidden').length > 0
-				&& this.getTotalWidth() + this._getCrumbElement().width() < availableWidth) {
+				&& this.getTotalWidth() + this._getCrumbElement().outerWidth(true) < availableWidth) {
 				this._showCrumb();
 			}
 
diff --git a/apps/files/tests/js/breadcrumbSpec.js b/apps/files/tests/js/breadcrumbSpec.js
index b1fd8d11769..ea912cc9fa9 100644
--- a/apps/files/tests/js/breadcrumbSpec.js
+++ b/apps/files/tests/js/breadcrumbSpec.js
@@ -313,6 +313,60 @@ describe('OCA.Files.BreadCrumb tests', function() {
 			expect($crumbs.eq(6).hasClass('hidden')).toEqual(false);
 			expect($crumbs.eq(7).hasClass('hidden')).toEqual(false);
 		});
+		it('Hides breadcrumbs to fit available width taking paddings into account', function() {
+			var $crumbs;
+
+			// Each element is 20px wider
+			paddings = [10, 10, 10, 10, 10, 10, 10, 10];
+
+			$('div.crumb').each(function(index){
+				$(this).css('padding', paddings[index]);
+			});
+
+			$('#controls').width(700);
+			bc._resize();
+
+			$crumbs = bc.$el.find('.crumb');
+
+			// Second, third and fourth crumb are hidden and everything else is
+			// 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(true);
+			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('Hides breadcrumbs to fit available width taking margins into account', function() {
+			var $crumbs;
+
+			// Each element is 20px wider
+			margins = [10, 10, 10, 10, 10, 10, 10, 10];
+
+			$('div.crumb').each(function(index){
+				$(this).css('margin', margins[index]);
+			});
+
+			$('#controls').width(700);
+			bc._resize();
+
+			$crumbs = bc.$el.find('.crumb');
+
+			// Second, third and fourth crumb are hidden and everything else is
+			// 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(true);
+			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('Hides breadcrumbs to fit available width left by siblings', function() {
 			var $crumbs;
 
-- 
cgit v1.2.3


From 1bcba56d8fecba8d8d64da0f66f51daef1140ead Mon Sep 17 00:00:00 2001
From: Daniel Calviño Sánchez <danxuliu@gmail.com>
Date: Wed, 28 Feb 2018 13:53:00 +0100
Subject: Fix setup to test the breadcrumbs menu
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The "Shows only items not in the breadcrumb" test was failing when run
on Firefox, but not on PhantomJS. This was caused by the differences in
the starting width between both browsers and an incorrect setup of the
test (the width set for the crumbs was overriden when the breadcrumbs
were rendered again, and the breadcrumb was resized to 300 from an
indeterminate initial width).

Now the crumbs are rendered and then its width, padding and margin are
set to a known value. Then it is resized to 1000px, which ensures that
there will be enough room for all the crumbs and thus the menu will be
hidden, and finally it is resized to 300, which causes the middle crumb
to be hidden and the menu to be shown.

Note, however, that the test now always fails, no matter if it is run on
PhantomJS or on Firefox; if the menu crumb is hidden when "_updateMenu"
is called it will show it, but it will also wrongly try to add the menu
itself to the menu. As the "crumb-id" of the menu crumb is "-1" this
causes the last regular crumb to be added to the menu. This will be
fixed with other related issues in the next commit.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
---
 apps/files/tests/js/breadcrumbSpec.js | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/apps/files/tests/js/breadcrumbSpec.js b/apps/files/tests/js/breadcrumbSpec.js
index ea912cc9fa9..3d154f12edd 100644
--- a/apps/files/tests/js/breadcrumbSpec.js
+++ b/apps/files/tests/js/breadcrumbSpec.js
@@ -175,10 +175,6 @@ describe('OCA.Files.BreadCrumb tests', function() {
 		beforeEach(function() {
 			dummyDir = '/one/two/three/four/five'
 
-			$('div.crumb').each(function(index){
-				$(this).css('width', 50);
-			});
-
 			bc = new BreadCrumb();
 			// append dummy navigation and controls
 			// as they are currently used for measurements
@@ -187,11 +183,22 @@ describe('OCA.Files.BreadCrumb tests', function() {
 			);
 			$('#controls').append(bc.$el);
 
+			bc.setDirectory(dummyDir);
+
+			$('div.crumb').each(function(index){
+				$(this).css('width', 50);
+				$(this).css('padding', 0);
+				$(this).css('margin', 0);
+			});
+			$('div.crumbhome').css('width', 51);
+			$('div.crumbmenu').css('width', 51);
+
+			$('#controls').width(1000);
+			bc._resize();
+
 			// Shrink to show popovermenu
 			$('#controls').width(300);
-
-			// triggers resize implicitly
-			bc.setDirectory(dummyDir);
+			bc._resize();
 
 			$crumbmenuLink = bc.$el.find('.crumbmenu > a');
 			$popovermenu = $crumbmenuLink.next('.popovermenu');
-- 
cgit v1.2.3


From 10e9eeec45773325a69c47c19266d6629e4ea3ca Mon Sep 17 00:00:00 2001
From: Daniel Calviño Sánchez <danxuliu@gmail.com>
Date: Tue, 27 Feb 2018 17:14:34 +0100
Subject: Fix menu visibility
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The crumb for the menu was shown like any other crumb when calling
"_showCrumb", but it was also shown when other crumbs were hidden
without taking into account the available width. This caused several
related problems, like the breadcrumbs taking too much space when the
menu was sometimes shown after the rest of the crumbs were adjusted to
the available width, or the menu being shown instead of the last crumb
even if there was room for it when the available width was increased.

Now the menu is always hidden before starting the resizing of the crumbs
to ensure that whether it was previously shown or not does not affect
the result. In a similar way, the menu will no longer be shown by
"_showCrumb", as it is not a regular crumb that has to be shown simply
if there is enough room. The menu is now shown as soon as any other
crumb is hidden; this ensures that the menu width will be taken into
account in further width checks. As when _updateMenu" is called it no
longer needs to take care of showing the menu this fixes the issue
revealed when fixing the test setup in the previous commit.

Finally, this implicitly fixes the failure in the breadcrumbs tests when
run on Firefox, as it was caused by the menu interfering in the
calculations of the other crumbs when increasing the width.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
---
 apps/files/js/breadcrumb.js           | 35 +++++++++++-----
 apps/files/tests/js/breadcrumbSpec.js | 75 +++++++++++++++++++++++++++++++++++
 2 files changed, 100 insertions(+), 10 deletions(-)

diff --git a/apps/files/js/breadcrumb.js b/apps/files/js/breadcrumb.js
index e4951958ad9..3e43de39354 100644
--- a/apps/files/js/breadcrumb.js
+++ b/apps/files/js/breadcrumb.js
@@ -36,6 +36,7 @@
 		this.$menu = $('<div class="popovermenu menu-center"><ul></ul></div>');
 
 		this.crumbSelector = '.crumb:not(.hidden):not(.crumbhome):not(.crumbmenu)';
+		this.hiddenCrumbSelector = '.crumb.hidden:not(.crumbhome):not(.crumbmenu)';
 		options = options || {};
 		if (options.onClick) {
 			this.onClick = options.onClick;
@@ -269,19 +270,19 @@
  		 * Get the crumb to show
  		 */
  		_getCrumbElement: function() {
-			var hidden = this.$el.find('.crumb.hidden').length;
+			var hidden = this.$el.find(this.hiddenCrumbSelector).length;
 			var shown = this.$el.find(this.crumbSelector).length;
 			// Get the outer one with priority to the highest
 			var elmt = (1 - shown % 2) * (hidden - 1);
-			return this.$el.find('.crumb.hidden:eq('+elmt+')');
+			return this.$el.find(this.hiddenCrumbSelector + ':eq('+elmt+')');
 		},
 
  		/**
  		 * Show the middle crumb
  		 */
  		_showCrumb: function() {
-			if(this.$el.find('.crumb.hidden').length === 1) {
-				this.$el.find('.crumb.hidden').removeClass('hidden');
+			if(this.$el.find(this.hiddenCrumbSelector).length === 1) {
+				this.$el.find(this.hiddenCrumbSelector).removeClass('hidden');
 			}
 			this._getCrumbElement().removeClass('hidden');
  		},
@@ -298,9 +299,7 @@
 		 * Update the popovermenu
 		 */
 		_updateMenu: function() {
-			var menuItems = this.$el.find('.crumb.hidden');
-			// Hide the crumb menu if no elements
-			this.$el.find('.crumbmenu').toggleClass('hidden', menuItems.length === 0);
+			var menuItems = this.$el.find(this.hiddenCrumbSelector);
 
 			this.$menu.find('li').addClass('in-breadcrumb');
 			for (var i = 0; i < menuItems.length; i++) {
@@ -316,12 +315,19 @@
 				return;
 			}
 
+			// Always hide the menu to ensure that it does not interfere with
+			// the width calculations; otherwise, the result could be different
+			// depending on whether the menu was previously being shown or not.
+			this.$el.find('.crumbmenu').addClass('hidden');
+
 			// Show the crumbs to compress the siblings before hidding again the
 			// crumbs. This is needed when the siblings expand to fill all the
 			// available width, as in that case their old width would limit the
 			// available width for the crumbs.
-			while (this.$el.find('.crumb.hidden').length > 0
-				&& this.getTotalWidth() < this.$el.parent().width()) {
+			// Note that the crumbs shown always overflow the parent width
+			// (except, of course, when they all fit in).
+			while (this.$el.find(this.hiddenCrumbSelector).length > 0
+				&& this.getTotalWidth() <= this.$el.parent().width()) {
 				this._showCrumb();
 			}
 
@@ -339,11 +345,20 @@
 			// AND if there are crumbs left to hide
 			while (this.getTotalWidth() > availableWidth
 				&& this.$el.find(this.crumbSelector).length > 0) {
+				// As soon as one of the crumbs is hidden the menu will be
+				// shown. This is needed for proper results in further width
+				// checks.
+				// Note that the menu is not shown only when all the crumbs were
+				// being shown and they all fit the available space; if any of
+				// the crumbs was not being shown then those shown would
+				// overflow the available width, so at least one will be hidden
+				// and thus the menu will be shown.
+				this.$el.find('.crumbmenu').removeClass('hidden');
 				this._hideCrumb();
 			}
 			// If container is bigger than content + element to be shown
 			// AND if there is at least one hidden crumb
-			while (this.$el.find('.crumb.hidden').length > 0
+			while (this.$el.find(this.hiddenCrumbSelector).length > 0
 				&& this.getTotalWidth() + this._getCrumbElement().outerWidth(true) < availableWidth) {
 				this._showCrumb();
 			}
diff --git a/apps/files/tests/js/breadcrumbSpec.js b/apps/files/tests/js/breadcrumbSpec.js
index 3d154f12edd..ebc70f35a78 100644
--- a/apps/files/tests/js/breadcrumbSpec.js
+++ b/apps/files/tests/js/breadcrumbSpec.js
@@ -529,6 +529,45 @@ describe('OCA.Files.BreadCrumb tests', function() {
 			expect($crumbs.eq(6).hasClass('hidden')).toEqual(false);
 			expect($crumbs.eq(7).hasClass('hidden')).toEqual(false);
 		});
+		it('Updates the breadcrumbs when reducing available width taking into account the menu width', function() {
+			var $crumbs;
+
+			// enough space
+			$('#controls').width(1800);
+			bc._resize();
+
+			$crumbs = bc.$el.find('.crumb');
+
+			// Menu is hidden
+			expect($crumbs.eq(0).hasClass('hidden')).toEqual(true);
+			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(false);
+			expect($crumbs.eq(5).hasClass('hidden')).toEqual(false);
+			expect($crumbs.eq(6).hasClass('hidden')).toEqual(false);
+			expect($crumbs.eq(7).hasClass('hidden')).toEqual(false);
+
+			// simulate decrease
+			// 650 is enough for all the crumbs except the third and fourth
+			// ones, but not enough for the menu and all the crumbs except the
+			// third and fourth ones; the second one has to be hidden too.
+			$('#controls').width(650);
+			bc._resize();
+
+			// Second, third and fourth crumb are hidden and everything else is
+			// 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(true);
+			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 increasing available width', function() {
 			var $crumbs;
 
@@ -564,6 +603,42 @@ describe('OCA.Files.BreadCrumb tests', function() {
 			expect($crumbs.eq(6).hasClass('hidden')).toEqual(false);
 			expect($crumbs.eq(7).hasClass('hidden')).toEqual(false);
 		});
+		it('Updates the breadcrumbs when increasing available width taking into account the menu width', function() {
+			var $crumbs;
+
+			// limited space
+			$('#controls').width(850);
+			bc._resize();
+
+			$crumbs = bc.$el.find('.crumb');
+
+			// Third and fourth crumb are hidden and everything else is 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);
+
+			// simulate increase
+			// 1030 is enough for all the crumbs if the menu is hidden.
+			$('#controls').width(1030);
+			bc._resize();
+
+			// Menu is hidden and everything else is visible
+			expect($crumbs.eq(0).hasClass('hidden')).toEqual(true);
+			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(false);
+			expect($crumbs.eq(5).hasClass('hidden')).toEqual(false);
+			expect($crumbs.eq(6).hasClass('hidden')).toEqual(false);
+			expect($crumbs.eq(7).hasClass('hidden')).toEqual(false);
+		});
 		cit('Updates the breadcrumbs when increasing available width with an expanding sibling', function() {
 			var $crumbs;
 
-- 
cgit v1.2.3


From d122d054d368f8b7084f3ff31c406867b6c0a55c Mon Sep 17 00:00:00 2001
From: Daniel Calviño Sánchez <danxuliu@gmail.com>
Date: Wed, 28 Feb 2018 14:13:38 +0100
Subject: Do not show the crumbs again after hiding them
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

After the changes in the previous commit "_showCrumb" no longer shows
the menu, only the same crumb that was hidden by the last call to
"_hideCrumb". Therefore, if the crumb was hidden because it did not fit
there is no need to try to show it again, as it will still not fit.

Moreover, the calculated width for a hidden element is not always
accurate; in some cases the calculated width is lower than the actual
width (it happens, for example, when using a background image like the
"Share" icon), which causea the crumb to be shown even if there is not
enough room, which in the end causes the siblings to overflow the
contents.

No unit tests for this one, though; you will have to trust me on this,
sorry ;-)

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
---
 apps/files/js/breadcrumb.js | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/apps/files/js/breadcrumb.js b/apps/files/js/breadcrumb.js
index 3e43de39354..b9e8e6920e9 100644
--- a/apps/files/js/breadcrumb.js
+++ b/apps/files/js/breadcrumb.js
@@ -356,12 +356,6 @@
 				this.$el.find('.crumbmenu').removeClass('hidden');
 				this._hideCrumb();
 			}
-			// If container is bigger than content + element to be shown
-			// AND if there is at least one hidden crumb
-			while (this.$el.find(this.hiddenCrumbSelector).length > 0
-				&& this.getTotalWidth() + this._getCrumbElement().outerWidth(true) < availableWidth) {
-				this._showCrumb();
-			}
 
 			this._updateMenu();
 		}
-- 
cgit v1.2.3


From 292476911fdf77a7cb8d0d1202ed05a38c5ced3b Mon Sep 17 00:00:00 2001
From: Daniel Calviño Sánchez <danxuliu@gmail.com>
Date: Wed, 28 Feb 2018 14:35:28 +0100
Subject: Improve documentation of "getTotalWidth"
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

"getTotalWidth" is not more accurate; it is simply not clamped.
Moreover, "width/outerWidth" could be used in tests too, and also even
if "getTotalWidth" could be used in tests while others not that would
not be something to be stated in the API documentation, but in a
comment.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
---
 apps/files/js/breadcrumb.js | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/apps/files/js/breadcrumb.js b/apps/files/js/breadcrumb.js
index b9e8e6920e9..2593e12d72f 100644
--- a/apps/files/js/breadcrumb.js
+++ b/apps/files/js/breadcrumb.js
@@ -241,11 +241,14 @@
 
 		/**
 		 * Calculate real width based on individual crumbs
-		 * More accurate and works with tests
 		 *
 		 * @param {boolean} ignoreHidden ignore hidden crumbs
 		 */
 		getTotalWidth: function(ignoreHidden) {
+			// The width has to be calculated by adding up the width of all the
+			// crumbs; getting the width of the breadcrumb element is not a
+			// valid approach, as the returned value could be clamped to its
+			// parent width.
 			var totalWidth = 0;
 			for (var i = 0; i < this.breadcrumbs.length; i++ ) {
 				var $crumb = $(this.breadcrumbs[i]);
-- 
cgit v1.2.3


From ad71abca6ff6d959dd5979a7f6b73c0a1cbc37fd Mon Sep 17 00:00:00 2001
From: Daniel Calviño Sánchez <danxuliu@gmail.com>
Date: Wed, 28 Feb 2018 15:00:44 +0100
Subject: Update comments in tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Menu and home are not always visible; home is always visible, but menu
is shown only when needed.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
---
 apps/files/tests/js/breadcrumbSpec.js | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/apps/files/tests/js/breadcrumbSpec.js b/apps/files/tests/js/breadcrumbSpec.js
index ebc70f35a78..14ad42a915d 100644
--- a/apps/files/tests/js/breadcrumbSpec.js
+++ b/apps/files/tests/js/breadcrumbSpec.js
@@ -290,7 +290,8 @@ describe('OCA.Files.BreadCrumb tests', function() {
 
 			$crumbs = bc.$el.find('.crumb');
 
-			// Menu and home are always visible
+			// Second, third, fourth and fifth crumb are hidden and everything
+			// else is visible
 			expect($crumbs.eq(0).hasClass('hidden')).toEqual(false);
 			expect($crumbs.eq(1).hasClass('hidden')).toEqual(false);
 
@@ -309,7 +310,7 @@ describe('OCA.Files.BreadCrumb tests', function() {
 
 			$crumbs = bc.$el.find('.crumb');
 
-			// Menu and home are always visible
+			// Third and fourth crumb are hidden and everything else is visible
 			expect($crumbs.eq(0).hasClass('hidden')).toEqual(false);
 			expect($crumbs.eq(1).hasClass('hidden')).toEqual(false);
 
@@ -518,7 +519,7 @@ describe('OCA.Files.BreadCrumb tests', function() {
 			$('#controls').width(950);
 			bc._resize();
 
-			// Menu and home are always visible
+			// Third crumb is hidden and everything else is visible
 			expect($crumbs.eq(0).hasClass('hidden')).toEqual(false);
 			expect($crumbs.eq(1).hasClass('hidden')).toEqual(false);
 
-- 
cgit v1.2.3