From c310006a62c93cca051e401fd3720d3768e89dcb Mon Sep 17 00:00:00 2001 From: ilhan Date: Thu, 9 Apr 2020 12:47:44 +0300 Subject: [PATCH 01/10] feat(ui5-carousel): add selectedIndexChange event --- packages/main/src/Carousel.hbs | 4 ++++ packages/main/src/Carousel.js | 29 ++++++++++++++++++++++- packages/main/test/pages/Carousel.html | 10 ++++++++ packages/main/test/specs/Carousel.spec.js | 24 +++++++++++++++++++ 4 files changed, 66 insertions(+), 1 deletion(-) diff --git a/packages/main/src/Carousel.hbs b/packages/main/src/Carousel.hbs index 07a08152073e..9aaab3583d33 100644 --- a/packages/main/src/Carousel.hbs +++ b/packages/main/src/Carousel.hbs @@ -19,6 +19,7 @@ {{#if arrows.content}}
{{#if arrows.navigation}} selectedIndex changes due to user interaction. + * Note: it changes when the user clicks on the navigation arrows, or upon screen resize. + * s the user can define how many items to be displayed on different screen sizes, the selectedIndex might change on resize. + * + * + * @event + * @public + */ + selectedIndexChange: { + }, }, }; @@ -237,7 +247,12 @@ class Carousel extends UI5Element { // Whenever the number of items per page changes, the selected index needs to be re-adjusted so that the items // that were visible before, can be visible as much as possible afterwards. const adjustment = oldItemsPerPage / this.effectiveItemsPerPage; + const peviousSelectedIndex = this.selectedIndex; this.selectedIndex = Math.round(this.selectedIndex * adjustment); + + if (peviousSelectedIndex !== this.selectedIndex) { + this.fireEvent("selectedIndexChange", { selectedIndex: this.selectedIndex }); + } } _updateScrolling(event) { @@ -265,6 +280,8 @@ class Carousel extends UI5Element { } navigateLeft() { + const peviousSelectedIndex = this.selectedIndex; + if (this.selectedIndex - 1 < 0) { if (this.cyclic) { this.selectedIndex = this.pages.length - 1; @@ -272,9 +289,15 @@ class Carousel extends UI5Element { } else { --this.selectedIndex; } + + if (peviousSelectedIndex !== this.selectedIndex) { + this.fireEvent("selectedIndexChange", { selectedIndex: this.selectedIndex }); + } } navigateRight() { + const peviousSelectedIndex = this.selectedIndex; + if (this.selectedIndex + 1 > this.pages.length - 1) { if (this.cyclic) { this.selectedIndex = 0; @@ -282,6 +305,10 @@ class Carousel extends UI5Element { } else { ++this.selectedIndex; } + + if (peviousSelectedIndex !== this.selectedIndex) { + this.fireEvent("selectedIndexChange", { selectedIndex: this.selectedIndex }); + } } get shouldAnimate() { diff --git a/packages/main/test/pages/Carousel.html b/packages/main/test/pages/Carousel.html index f3040c3c52ba..4a41183d8610 100644 --- a/packages/main/test/pages/Carousel.html +++ b/packages/main/test/pages/Carousel.html @@ -358,6 +358,8 @@ Button 7 Button 8 + + Carousel with one page The arrows and dots are not displayed @@ -369,4 +371,12 @@ + + diff --git a/packages/main/test/specs/Carousel.spec.js b/packages/main/test/specs/Carousel.spec.js index f2f975b5cc1a..32385e2ff191 100644 --- a/packages/main/test/specs/Carousel.spec.js +++ b/packages/main/test/specs/Carousel.spec.js @@ -64,4 +64,28 @@ describe("Carousel general interaction", () => { assert.ok(!navigationArrows.isExisting(), "Navigation is rendered"); assert.strictEqual(pages, 1, "There are only 3 pages."); }); + + it("Event selectedIndexChange fired when pressing navigation arrows", () => { + const carousel = browser.$("#carousel5"); + const selectedIndex = browser.$("#result"); + const eventCounter = browser.$("#resultCounter"); + const navigationArrowForward = carousel.shadow$("ui5-button[arrow-forward]"); + const navigationArrowsBack = carousel.shadow$("ui5-button[arrow-back]"); + + navigationArrowForward.click(); // forward + assert.strictEqual(selectedIndex.getProperty("value"), "1", "The selectedIndex is correct."); + assert.strictEqual(eventCounter.getProperty("value"), "1", "The selectedIndexChange is fired."); + + navigationArrowForward.click(); // forward + assert.strictEqual(selectedIndex.getProperty("value"), "1", "The selectedIndex remains the same as this is the last page."); + assert.strictEqual(eventCounter.getProperty("value"), "1", "The selectedIndexChange not fired as this is last page."); + + navigationArrowsBack.click(); // back + assert.strictEqual(selectedIndex.getProperty("value"), "0", "The selectedIndex is correct"); + assert.strictEqual(eventCounter.getProperty("value"), "2", "The selectedIndexChange is fired."); + + navigationArrowsBack.click(); // back + assert.strictEqual(selectedIndex.getProperty("value"), "0", "The selectedIndex remains the same as this is the first page."); + assert.strictEqual(eventCounter.getProperty("value"), "2", "The selectedIndexChange is not fired as this is the first page."); + }); }); From 2eee90deaa18e793acdf329324cc65cabb595257 Mon Sep 17 00:00:00 2001 From: ilhan Date: Thu, 9 Apr 2020 12:50:45 +0300 Subject: [PATCH 02/10] docs typo and lint --- packages/main/src/Carousel.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/main/src/Carousel.js b/packages/main/src/Carousel.js index aa301cd503ea..ef530a65a909 100644 --- a/packages/main/src/Carousel.js +++ b/packages/main/src/Carousel.js @@ -146,8 +146,8 @@ const metadata = { /** * Fired when the currently selectedIndex changes due to user interaction. * Note: it changes when the user clicks on the navigation arrows, or upon screen resize. - * s the user can define how many items to be displayed on different screen sizes, the selectedIndex might change on resize. - * + * As the user can define how many items to be displayed on different screen sizes, + * the number of pages and the selectedIndex might change. * * @event * @public @@ -251,7 +251,7 @@ class Carousel extends UI5Element { this.selectedIndex = Math.round(this.selectedIndex * adjustment); if (peviousSelectedIndex !== this.selectedIndex) { - this.fireEvent("selectedIndexChange", { selectedIndex: this.selectedIndex }); + this.fireEvent("selectedIndexChange", { selectedIndex: this.selectedIndex }); } } @@ -291,7 +291,7 @@ class Carousel extends UI5Element { } if (peviousSelectedIndex !== this.selectedIndex) { - this.fireEvent("selectedIndexChange", { selectedIndex: this.selectedIndex }); + this.fireEvent("selectedIndexChange", { selectedIndex: this.selectedIndex }); } } @@ -307,7 +307,7 @@ class Carousel extends UI5Element { } if (peviousSelectedIndex !== this.selectedIndex) { - this.fireEvent("selectedIndexChange", { selectedIndex: this.selectedIndex }); + this.fireEvent("selectedIndexChange", { selectedIndex: this.selectedIndex }); } } From 1b095caaea84f9fc1946d3ecc09e2499b79538b4 Mon Sep 17 00:00:00 2001 From: ilhan Date: Thu, 9 Apr 2020 12:57:44 +0300 Subject: [PATCH 03/10] change event name --- packages/main/src/Carousel.js | 17 +++++------------ packages/main/test/pages/Carousel.html | 2 +- packages/main/test/specs/Carousel.spec.js | 10 +++++----- 3 files changed, 11 insertions(+), 18 deletions(-) diff --git a/packages/main/src/Carousel.js b/packages/main/src/Carousel.js index ef530a65a909..a047d2ecaaa5 100644 --- a/packages/main/src/Carousel.js +++ b/packages/main/src/Carousel.js @@ -144,15 +144,13 @@ const metadata = { }, events: /** @lends sap.ui.webcomponents.main.Carousel.prototype */ { /** - * Fired when the currently selectedIndex changes due to user interaction. - * Note: it changes when the user clicks on the navigation arrows, or upon screen resize. - * As the user can define how many items to be displayed on different screen sizes, - * the number of pages and the selectedIndex might change. + * Fired when the currently selected page changes, + * when the user clicks on the navigation arrows, or upon screen resize. * * @event * @public */ - selectedIndexChange: { + selectedPageChange: { }, }, }; @@ -247,12 +245,7 @@ class Carousel extends UI5Element { // Whenever the number of items per page changes, the selected index needs to be re-adjusted so that the items // that were visible before, can be visible as much as possible afterwards. const adjustment = oldItemsPerPage / this.effectiveItemsPerPage; - const peviousSelectedIndex = this.selectedIndex; this.selectedIndex = Math.round(this.selectedIndex * adjustment); - - if (peviousSelectedIndex !== this.selectedIndex) { - this.fireEvent("selectedIndexChange", { selectedIndex: this.selectedIndex }); - } } _updateScrolling(event) { @@ -291,7 +284,7 @@ class Carousel extends UI5Element { } if (peviousSelectedIndex !== this.selectedIndex) { - this.fireEvent("selectedIndexChange", { selectedIndex: this.selectedIndex }); + this.fireEvent("selectedPageChange", { selectedIndex: this.selectedIndex }); } } @@ -307,7 +300,7 @@ class Carousel extends UI5Element { } if (peviousSelectedIndex !== this.selectedIndex) { - this.fireEvent("selectedIndexChange", { selectedIndex: this.selectedIndex }); + this.fireEvent("selectedPageChange", { selectedIndex: this.selectedIndex }); } } diff --git a/packages/main/test/pages/Carousel.html b/packages/main/test/pages/Carousel.html index 4a41183d8610..9927080b61d3 100644 --- a/packages/main/test/pages/Carousel.html +++ b/packages/main/test/pages/Carousel.html @@ -374,7 +374,7 @@