Skip to content

Commit

Permalink
refactor(ui5-carousel): rework component behaviour (#1486)
Browse files Browse the repository at this point in the history 
- [ Refactor ] Previously entire page of items is being moved in and out of the view during navigation. Now, the items are moved in and out the visible area one by one, in other words all the items are shifted to the left and to the right by one.
- [ New Implementation ] Initial accessibility spec is implemented (as in openui5), but can be further improved as it does not handle the multiple items per page case the best way.
- [ Bug Fix ] Previously the navigation arrows were remaining displayed, although no previous or next item is available and pressing the arrows didn't perform any action. Now, the arrows remain hidden if no previous/next item is available to navigate towards.
- [ Bug Fix ] Previously the navigation arrows have been misplaced in IE, making the component not usable at all 
as the next arrow is out of the viewport ([reproduce in IE](https://sap.github.io/ui5-webcomponents/master/playground/main/pages/Carousel/)). Now the arrows are displayed correctly in IE too.
- [ Bug Fix ] Previously setting "selectedIndex" that is out of range brings the component to unacceptable/ broken state. Now, the "selectedIndex" is validated and changed to its default value when not valid.

FIXES: #1277
  • Loading branch information
@ilhan007
ilhan007 authored Apr 22, 2020
1 parent df2671f commit f337d47
Show file tree
Hide file tree
Showing 7 changed files with 364 additions and 248 deletions.
37 changes: 22 additions & 15 deletions packages/main/src/Carousel.hbs
View file
Original file line number Diff line number Diff line change
@@ -1,32 +1,37 @@
<section
class="ui5-carousel-root"
tabindex="0"
role="listbox"
aria-activedescendant="{{ariaActiveDescendant}}"
@keydown={{_onkeydown}}
>
<div class="ui5-carousel-overflow-hidden">
<div class="ui5-carousel-viewport {{classes.viewport}}">
<div class="{{classes.content}}" style="{{styles.content}}">
{{#each pages}}
<div class="{{../classes.page}}">
{{#each this}}
<div class="ui5-carousel-item" style="width: {{this.width}}%">
<slot name="{{this.item._individualSlot}}" tabindex="{{this.tabIndex}}"></slot>
</div>
{{/each}}
{{#each items}}
<div id="{{id}}"
class="ui5-carousel-item {{classes}}"
style="width: {{this.width}}px;"
role="option"
aria-posinset="{{posinset}}"
aria-setsize="{{setsize}}"
>
<slot name="{{this.item._individualSlot}}" tabindex="{{this.tabIndex}}"></slot>
</div>
{{/each}}
</div>
</div>

{{#if arrows.content}}
<div class="ui5-carousel-navigation-arrows">
<ui5-button
class="ui5-carousel-navigation-button"
class="ui5-carousel-navigation-button {{classes.navPrevButton}}"
icon="slim-arrow-left"
tabindex="-1"
@click={{navigateLeft}}
@keydown={{_onbuttonkeydown}}
></ui5-button>
<ui5-button
class="ui5-carousel-navigation-button"
class="ui5-carousel-navigation-button {{classes.navNextButton}}"
icon="slim-arrow-right"
tabindex="-1"
@click={{navigateRight}}
Expand All @@ -39,7 +44,7 @@
<div class="{{classes.navigation}}">
{{#if arrows.navigation}}
<ui5-button
class="ui5-carousel-navigation-button"
class="ui5-carousel-navigation-button {{classes.navPrevButton}}"
icon="slim-arrow-left"
tabindex="-1"
@click={{navigateLeft}}
Expand All @@ -51,18 +56,20 @@
{{#if isPageTypeDots}}
{{#each dots}}
<div
?active="{{this.active}}"
role="img"
aria-label="{{ariaLabel}}"
?active="{{active}}"
class="ui5-carousel-navigation-dot"
></div>
{{/each}}
{{else}}
<ui5-label>{{selectedIndexToShow}}&nbsp;{{ofText}}&nbsp;{{pages.length}}</ui5-label>
<ui5-label>{{selectedIndexToShow}}&nbsp;{{ofText}}&nbsp;{{pagesCount}}</ui5-label>
{{/if}}
</div>

{{#if arrows.navigation}}
<ui5-button
class="ui5-carousel-navigation-button"
class="ui5-carousel-navigation-button {{classes.navNextButton}}"
icon="slim-arrow-right"
tabindex="-1"
@click={{navigateRight}}
Expand All @@ -72,4 +79,4 @@
</div>
{{/if}}
</div>
</section>
</section>
149 changes: 100 additions & 49 deletions packages/main/src/Carousel.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import AnimationMode from "@ui5/webcomponents-base/dist/types/AnimationMode.js";
import { getAnimationMode } from "@ui5/webcomponents-base/dist/config/AnimationMode.js";
import {
CAROUSEL_OF_TEXT,
CAROUSEL_DOT_TEXT,
} from "./generated/i18n/i18n-defaults.js";
import CarouselArrowsPlacement from "./types/CarouselArrowsPlacement.js";
import CarouselTemplate from "./generated/templates/CarouselTemplate.lit.js";
Expand Down Expand Up @@ -127,6 +128,14 @@ const metadata = {
_width: {
type: Integer,
},

/**
* Defines the carousel item width in pixels
* @private
*/
_itemWidth: {
type: Integer,
},
},
managedSlots: true,
slots: /** @lends sap.ui.webcomponents.main.Carousel.prototype */ {
Expand Down Expand Up @@ -209,10 +218,16 @@ class Carousel extends UI5Element {

this.i18nBundle = getI18nBundle("@ui5/webcomponents");
this._onResizeBound = this._onResize.bind(this);
this._resizing = false; // indicates if the carousel is in process of resizing
}

onBeforeRendering() {
this.validateSelectedIndex();
}

onAfterRendering() {
this._scrollEnablement.scrollContainer = this.getDomRef();
this._resizing = false; // not invalidating
}

onEnterDOM() {
Expand All @@ -223,21 +238,32 @@ class Carousel extends UI5Element {
ResizeHandler.deregister(this, this._onResizeBound);
}

validateSelectedIndex() {
if (!this.isIndexInRange(this.selectedIndex)) {
this.selectedIndex = 0;
console.warn(`The "selectedIndex" is out of range, changed to: ${0}`); // eslint-disable-line
}
}

_onResize() {
const oldItemsPerPage = this.effectiveItemsPerPage;
const previousItemsPerPage = this.effectiveItemsPerPage;

// Set the resizing flag to suppress animation while resizing
this._resizing = true;

// Change transitively effectiveItemsPerPage by modifying _width
this._width = this.offsetWidth;
this._itemWidth = Math.floor(this._width / this.effectiveItemsPerPage);

// Items per page did not change, therefore page index does not need to be re-adjusted
if (this.effectiveItemsPerPage === oldItemsPerPage) {
// Items per page did not change or the current,
// therefore page index does not need to be re-adjusted
if (this.effectiveItemsPerPage === previousItemsPerPage) {
return;
}

// 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;
this.selectedIndex = Math.round(this.selectedIndex * adjustment);
if (this.selectedIndex > this.pagesCount - 1) {
this.selectedIndex = this.pagesCount - 1;
}
}

_updateScrolling(event) {
Expand Down Expand Up @@ -267,15 +293,15 @@ class Carousel extends UI5Element {
navigateLeft() {
if (this.selectedIndex - 1 < 0) {
if (this.cyclic) {
this.selectedIndex = this.pages.length - 1;
this.selectedIndex = this.pagesCount - 1;
}
} else {
--this.selectedIndex;
}
}

navigateRight() {
if (this.selectedIndex + 1 > this.pages.length - 1) {
if (this.selectedIndex + 1 > this.pagesCount - 1) {
if (this.cyclic) {
this.selectedIndex = 0;
}
Expand All @@ -284,37 +310,22 @@ class Carousel extends UI5Element {
}
}

get shouldAnimate() {
return getAnimationMode() === AnimationMode.None;
}

/**
* Assuming that all items have the same width
* @private
*/
get pages() {
const result = [],
pagesCount = Math.ceil(this.content.length / this.effectiveItemsPerPage);

for (let pageIdx = 0; pageIdx < pagesCount; pageIdx++) {
result.push([]);
for (let itemIdx = 0; itemIdx < this.effectiveItemsPerPage; itemIdx++) {
const item = this.content[(pageIdx * this.effectiveItemsPerPage) + itemIdx];
if (item) {
result[pageIdx].push({
item,
tabIndex: pageIdx === this.selectedIndex ? "0" : "-1",
});
}
}
const itemsOnThisPage = result[pageIdx].length;
const itemWidth = Math.floor(100 / itemsOnThisPage);
result[pageIdx].forEach(item => {
item.width = itemWidth;
});
}

return result;
get items() {
return this.content.map((item, idx) => {
return {
id: `${this._id}-carousel-item-${idx + 1}`,
item,
tabIndex: idx === this.selectedIndex ? "0" : "-1",
posinset: idx + 1,
setsize: this.content.length,
width: this._itemWidth,
classes: this.isItemInViewport(idx) ? "" : "ui5-carousel-item--hidden",
};
});
}

get effectiveItemsPerPage() {
Expand All @@ -329,43 +340,67 @@ class Carousel extends UI5Element {
return this.itemsPerPageL;
}

isItemInViewport(index) {
return index >= this.selectedIndex && index <= this.selectedIndex + this.effectiveItemsPerPage - 1;
}

isIndexInRange(index) {
return index >= 0 && index <= this.pagesCount - 1;
}

get styles() {
return {
content: {
transform: `translateX(-${this.selectedIndex * 100}%)`,
transform: `translateX(-${this.selectedIndex * this._itemWidth}px`,
},
};
}

get classes() {
return {
viewport: {
"ui5-carousel-viewport--single": this.pagesCount === 1,
},
content: {
"ui5-carousel-content": true,
"ui5-carousel-content-no-animation": this.shouldAnimate,
"ui5-carousel-content-no-animation": this.supressAimation,
"ui5-carousel-content-has-navigation": this.showNavigationArrows,
"ui5-carousel-content-has-navigation-and-buttons": this.showNavigationArrows && this.arrowsPlacement === CarouselArrowsPlacement.Navigation,
},
navigation: {
"ui5-carousel-navigation-wrapper": true,
"ui5-carousel-navigation-with-buttons": this.showNavigationArrows && this.arrowsPlacement === CarouselArrowsPlacement.Navigation,
},
page: {
"ui5-carousel-page": true,
"ui5-carousel-page-multiple": this.effectiveItemsPerPage > 1,
navPrevButton: {
"ui5-carousel-navigation-button--hidden": !this.hasPrev,
},
navNextButton: {
"ui5-carousel-navigation-button--hidden": !this.hasNext,
},
};
}

get pagesCount() {
const items = this.content.length;
return items > this.effectiveItemsPerPage ? items - this.effectiveItemsPerPage + 1 : 1;
}

get isPageTypeDots() {
return this.pages.length < Carousel.pageTypeLimit;
return this.pagesCount < Carousel.pageTypeLimit;
}

get dots() {
return this.pages.map((item, index) => {
return {
const dots = [];
const pages = this.pagesCount;

for (let index = 0; index < pages; index++) {
dots.push({
active: index === this.selectedIndex,
};
});
ariaLabel: this.i18nBundle.getText(CAROUSEL_DOT_TEXT, [index + 1], [pages]),
});
}

return dots;
}

get arrows() {
Expand All @@ -377,16 +412,32 @@ class Carousel extends UI5Element {
};
}

get ofText() {
return this.i18nBundle.getText(CAROUSEL_OF_TEXT);
get hasPrev() {
return this.cyclic || this.selectedIndex - 1 >= 0;
}

get hasNext() {
return this.cyclic || this.selectedIndex + 1 <= this.pagesCount - 1;
}

get supressAimation() {
return this._resizing || getAnimationMode() === AnimationMode.None;
}

get selectedIndexToShow() {
return this.selectedIndex + 1;
}

get showNavigationArrows() {
return !this.hideNavigation && this.pages.length > 1;
return !this.hideNavigation && this.pagesCount > 1;
}

get ofText() {
return this.i18nBundle.getText(CAROUSEL_OF_TEXT);
}

get ariaActiveDescendant() {
return this.content.length ? `${this._id}-carousel-item-${this.selectedIndex + 1}` : undefined;
}

static async onDefine() {
Expand Down
3 changes: 3 additions & 0 deletions packages/main/src/i18n/messagebundle.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ DATEPICKER_DATE_ACC_TEXT=Date
# Carousel of text
CAROUSEL_OF_TEXT=of

#Carousel dots text
CAROUSEL_DOT_TEXT=Item {0} of {1} displayed

#XACT: DatePicker 'Open Picker' icon title
DATEPICKER_OPEN_ICON_TITLE=Open Picker

Expand Down
Loading

0 comments on commit f337d47

Please sign in to comment.