Repository: exadel-inc/esl
Pull request #2349 (2024-04-16 17:23:42)

[carousel]: refactoring for default renderer and utils


Patch:
diff --git a/src/modules/esl-carousel/core/esl-carousel.renderer.ts b/src/modules/esl-carousel/core/esl-carousel.renderer.ts
index ca3ff8e1b..106219aeb 100644
--- a/src/modules/esl-carousel/core/esl-carousel.renderer.ts
+++ b/src/modules/esl-carousel/core/esl-carousel.renderer.ts
@@ -2,7 +2,7 @@ import {memoize} from '../../esl-utils/decorators';
 import {isEqual} from '../../esl-utils/misc/object';
 import {SyntheticEventTarget} from '../../esl-utils/dom';
 import {ESLCarouselSlideEvent} from './esl-carousel.events';
-import {calcDirection, normalizeIndex} from './nav/esl-carousel.nav.utils';
+import {calcDirection, normalize} from './nav/esl-carousel.nav.utils';
 
 import type {ESLCarousel, ESLCarouselActionParams} from './esl-carousel';
 import type {ESLCarouselConfig, ESLCarouselDirection} from './nav/esl-carousel.nav.types';
@@ -129,7 +129,7 @@ export abstract class ESLCarouselRenderer implements ESLCarouselConfig {
     const count = Math.min(this.count, this.size);
 
     for (let i = 0; i < this.size; i++) {
-      const position = normalizeIndex(i + current, this.size);
+      const position = normalize(i + current, this.size);
       this.$slides[position].active = i < count;
       this.$slides[position].next = i === count && (this.loop || position !== 0);
       this.$slides[position].prev = i === this.size - 1 && i >= count && (this.loop || position !== this.size - 1);
diff --git a/src/modules/esl-carousel/core/esl-carousel.ts b/src/modules/esl-carousel/core/esl-carousel.ts
index 5d90e0b85..63bfeb99e 100644
--- a/src/modules/esl-carousel/core/esl-carousel.ts
+++ b/src/modules/esl-carousel/core/esl-carousel.ts
@@ -8,7 +8,7 @@ import {parseBoolean} from '../../esl-utils/misc';
 import {ESLMediaRuleList} from '../../esl-media-query/core';
 import {ESLResizeObserverTarget} from '../../esl-event-listener/core';
 
-import {normalizeIndex, toIndex, canNavigate} from './nav/esl-carousel.nav.utils';
+import {normalize, toIndex, canNavigate} from './nav/esl-carousel.nav.utils';
 
 import {ESLCarouselSlide} from './esl-carousel.slide';
 import {ESLCarouselRenderer} from './esl-carousel.renderer';
@@ -226,7 +226,7 @@ export class ESLCarousel extends ESLBaseElement {
     if (actives.length === this.$slides.length) return this.$slides[0];
 
     for (const slide of actives) {
-      const prevIndex = normalizeIndex(slide.index - 1, this.size);
+      const prevIndex = normalize(slide.index - 1, this.size);
       if (!this.$slides[prevIndex].active) return slide;
     }
     return this.$slides[0];
@@ -272,7 +272,7 @@ export class ESLCarousel extends ESLBaseElement {
 
   /** @returns slide by index (supports not normalized indexes) */
   public slideAt(index: number): ESLCarouselSlide {
-    return this.$slides[normalizeIndex(index, this.$slides.length)];
+    return this.$slides[normalize(index, this.$slides.length)];
   }
 
   /** @returns if the passed slide target can be reached */
diff --git a/src/modules/esl-carousel/core/nav/esl-carousel.nav.utils.ts b/src/modules/esl-carousel/core/nav/esl-carousel.nav.utils.ts
index c550aeb7a..4507fece4 100644
--- a/src/modules/esl-carousel/core/nav/esl-carousel.nav.utils.ts
+++ b/src/modules/esl-carousel/core/nav/esl-carousel.nav.utils.ts
@@ -1,19 +1,25 @@
-import type {ESLCarouselDirection, ESLCarouselNavIndex, ESLCarouselSlideTarget, ESLCarouselState} from './esl-carousel.nav.types';
-
-/** @returns bounded slide index in bounds of [0, count] range */
-export function boundIndex(value: number, max: number): number {
-  return Math.max(0, Math.min(max, value));
-}
+import type {
+  ESLCarouselDirection,
+  ESLCarouselNavIndex,
+  ESLCarouselSlideTarget,
+  ESLCarouselState,
+  ESLCarouselStaticState
+} from './esl-carousel.nav.types';
 
 /** @returns normalized slide index in bounds of [0, count] range */
-export function normalizeIndex(index: number, size: number): number {
+export function normalize(index: number, size: number): number {
   return (size + (index % size)) % size;
 }
 
+/** @returns normalize slide index according to the carousel mode */
+export function normalizeIndex(index: number, {size, count, loop}: ESLCarouselStaticState): number {
+  return loop ? normalize(index, size) : Math.max(0, Math.min(size - count, index));
+}
+
 /** Gets count of slides between active and passed considering given direction. */
 export function getDistance(from: number, direction: ESLCarouselDirection, {activeIndex, size}: ESLCarouselState): number {
-  if (direction === 'prev') return normalizeIndex(activeIndex - from, size);
-  if (direction === 'next') return normalizeIndex(from - activeIndex, size);
+  if (direction === 'prev') return normalize(activeIndex - from, size);
+  if (direction === 'next') return normalize(from - activeIndex, size);
   return 0;
 }
 
@@ -30,14 +36,14 @@ export function calcDirection(from: number, to: number, size: number): ESLCarous
 /** @returns normalized numeric index from group index */
 export function groupToIndex(group: number, count: number, size: number): number {
   const groupCount = Math.ceil(size / count);
-  const value = normalizeIndex(group, groupCount);
+  const value = normalize(group, groupCount);
   const index = value + 1 === groupCount ? size - count : count * value;
-  return normalizeIndex(index, size);
+  return normalize(index, size);
 }
 
 /** @returns numeric group index from slide index */
 export function indexToGroup(index: number, count: number, size: number): number {
-  const value = normalizeIndex(index, size);
+  const value = normalize(index, size);
   const groupCount = Math.ceil(size / count);
   const firstGroupIndex = value + count >= size ? groupCount - 1 : Math.floor(value / count);
   const secondGroupIndex = (firstGroupIndex + 1) % groupCount;
@@ -82,7 +88,7 @@ function parseIndex(index: string | ESLCarouselNavIndex): {value: number, isRela
 function resolveSlideIndex(indexStr: string | ESLCarouselNavIndex, cfg: ESLCarouselState): {index: number, dir: ESLCarouselDirection | null} {
   const {value, isRelative, dir} = parseIndex(indexStr);
   const target = value + (isRelative ? cfg.activeIndex : 0);
-  const index = cfg.loop ? normalizeIndex(target, cfg.size) : boundIndex(target, cfg.size - cfg.count);
+  const index = normalizeIndex(target, cfg);
   return {index, dir: dir || indexToDirection(index, cfg)};
 }
 
@@ -97,17 +103,17 @@ function resolveGroupIndex(indexStr: string | ESLCarouselNavIndex, cfg: ESLCarou
   if (value === -1 && cfg.activeIndex < cfg.count && cfg.activeIndex > 0) {
     return {index: 0, dir: dir || 'prev'};
   }
-  if (value === 1 && normalizeIndex(cfg.activeIndex + cfg.count, cfg.size) > cfg.size - cfg.count) {
+  if (value === 1 && normalize(cfg.activeIndex + cfg.count, cfg.size) > cfg.size - cfg.count) {
     return {index: cfg.size - cfg.count, dir: dir || 'next'};
   }
-  const index = normalizeIndex(cfg.activeIndex + value * cfg.count, cfg.size);
+  const index = normalize(cfg.activeIndex + value * cfg.count, cfg.size);
   return {index, dir: dir || indexToDirection(index, cfg)};
 }
 
 /** @returns normalized index from target definition and current state */
 export function toIndex(target: ESLCarouselSlideTarget, cfg: ESLCarouselState): {index: number, dir: ESLCarouselDirection | null} {
   if (typeof target === 'number') {
-    const index = cfg.loop ? normalizeIndex(target, cfg.size) : boundIndex(target, cfg.size - cfg.count);
+    const index = normalizeIndex(target, cfg);
     return {index, dir: indexToDirection(index, cfg)};
   }
   const {type, index} = splitTarget(target);
diff --git a/src/modules/esl-carousel/renderers/esl-carousel.default.renderer.ts b/src/modules/esl-carousel/renderers/esl-carousel.default.renderer.ts
index 4af838fb5..5de8fa131 100644
--- a/src/modules/esl-carousel/renderers/esl-carousel.default.renderer.ts
+++ b/src/modules/esl-carousel/renderers/esl-carousel.default.renderer.ts
@@ -1,5 +1,5 @@
 import {promisifyTransition} from '../../esl-utils/async';
-import {boundIndex, normalizeIndex} from '../core/nav/esl-carousel.nav.utils';
+import {normalize, normalizeIndex} from '../core/nav/esl-carousel.nav.utils';
 import {ESLCarouselRenderer} from '../core/esl-carousel.renderer';
 
 import type {ESLCarouselDirection} from '../core/nav/esl-carousel.nav.types';
@@ -12,6 +12,9 @@ export class ESLDefaultCarouselRenderer extends ESLCarouselRenderer {
   /** CSS variable name for slide auto size */
   public static SIZE_PROP = '--esl-slide-size';
 
+  /** Tolerance to treat offset enough to move to the next slide. Relative (0-1) to slide width */
+  public static readonly NEXT_SLIDE_TOLERANCE = 0.25;
+
   /** Min slides to position from both sides if possible */
   protected reserve: number = 1;
   /** Slides gap size */
@@ -80,77 +83,57 @@ export class ESLDefaultCarouselRenderer extends ESLCarouselRenderer {
     const {activeIndex, $slidesArea} =  this.$carousel;
     this.currentIndex = activeIndex;
     if (!$slidesArea) return;
-    while (this.currentIndex !== nextIndex) {
-      await this.onBeforeStepAnimate(direction);
-      await this.onAfterStepAnimate(direction);
-    }
+    while (this.currentIndex !== nextIndex) await this.onStepAnimate(direction);
   }
 
   /** Post-processing animation action. */
   public override async onAfterAnimate(): Promise<void> {
+    // Make sure we end up in a defined state on transition end
+    this.reorder(this.currentIndex);
     this.setTransformOffset(-this.getOffset(this.currentIndex));
     this.$carousel.$$attr('active', false);
   }
 
   /** Makes pre-processing the transition animation of one slide. */
-  protected async onBeforeStepAnimate(direction: ESLCarouselDirection): Promise<void> {
-    this.reorder(this.currentIndex, direction === 'prev');
+  protected async onStepAnimate(direction: ESLCarouselDirection): Promise<void> {
+    const index = normalize(this.currentIndex + (direction === 'next' ? 1 : -1), this.size);
 
-    const offsetIndex = normalizeIndex(this.currentIndex + (direction === 'next' ? 1 : -1), this.size);
-    const offsetFrom = -this.getOffset(this.currentIndex);
+    // Make sure there is a slide in required direction
+    this.reorder(index, direction !== 'prev');
 
+    const offsetFrom = -this.getOffset(this.currentIndex);
     this.setTransformOffset(offsetFrom);
-    const offsetTo = -this.getOffset(offsetIndex);
 
+    // here is the final reflow before transition
+    const offsetTo = -this.getOffset(index);
+    // Allow animation and move to the target slide
     this.$carousel.$$attr('animating', true);
     this.setTransformOffset(offsetTo);
+    if (offsetTo !== offsetFrom) {
+      await promisifyTransition(this.$area, 'transform');
+    }
 
-    if (offsetTo === offsetFrom) return;
-    // TODO: still not catches cases with no animation (
-    await promisifyTransition(this.$area, 'transform');
-  }
-
-  /** Makes post-processing the transition animation of one slide. */
-  protected async onAfterStepAnimate(direction: ESLCarouselDirection): Promise<void> {
-    this.currentIndex = direction === 'next' ? this.currentIndex + 1 : this.currentIndex - 1;
-    this.currentIndex = normalizeIndex(this.currentIndex, this.size);
-
-    this.reorder(this.currentIndex);
-
-    this.setTransformOffset(-this.getOffset(this.currentIndex));
+    this.currentIndex = index;
     this.$carousel.$$attr('animating', false);
-    +this.$carousel.offsetLeft;
-  }
-
-  protected _checkNonLoop(offset: number): boolean {
-    // TODO: get rid of this method by revisiting methods below
-    if (this.loop) return true;
-
-    const sign = offset < 0 ? 1 : -1;
-    const count = Math.floor(Math.abs(offset) / (this.slideSize + this.gap));
-    const nextIndex = this.$carousel.activeIndex + count * sign;
-    const currentIndex = normalizeIndex(this.$carousel.activeIndex + count * sign, this.size);
-
-    // check non-loop state
-    if (nextIndex >= this.$carousel.size || nextIndex < 0) return false;
-    // check left border of non-loop state
-    if (offset > 0 && currentIndex - 1 < 0) return false;
-    // check right border of non-loop state
-    return !(offset < 0 && currentIndex + 1 + this.count > this.$carousel.size);
   }
 
   /** Handles the slides transition. */
   public onMove(offset: number): void {
     this.$carousel.toggleAttribute('active', true);
 
-    if (!this._checkNonLoop(offset)) return;
-
     const sign = offset < 0 ? 1 : -1;
     const slideSize = this.slideSize + this.gap;
     const count = Math.floor(Math.abs(offset) / slideSize);
-    const currentIndex = normalizeIndex(this.$carousel.activeIndex + count * sign, this.size);
+    const index = this.$carousel.activeIndex + count * sign;
+
+    // check left border of non-loop state
+    if (!this.loop && offset > 0 && index - 1 < 0) return;
+    // check right border of non-loop state
+    if (!this.loop && offset < 0 && index + 1 + this.count > this.$carousel.size) return;
+
+    const currentIndex = normalize(index, this.size);
+    const orderIndex = offset < 0 ? currentIndex : normalize(currentIndex - 1, this.size);
 
-    const orderIndex = offset < 0 ? currentIndex : normalizeIndex(currentIndex - 1, this.size);
     this.reorder(orderIndex);
     this.currentIndex = currentIndex;
 
@@ -164,24 +147,22 @@ export class ESLDefaultCarouselRenderer extends ESLCarouselRenderer {
     const sign = offset < 0 ? 1 : -1;
     const slideSize = this.slideSize + this.gap;
 
-    const count = Math.abs(offset) % slideSize >= slideSize / 4 ?
-      Math.ceil(Math.abs(offset) / this.slideSize) : Math.floor(Math.abs(offset) / this.slideSize);
-    const nextIndex = this.$carousel.activeIndex + count * sign;
-    if (this.loop) {
-      this.currentIndex = normalizeIndex(nextIndex, this.size);
-    } else {
-      this.currentIndex = boundIndex(nextIndex, this.size - this.count);
-    }
+    const amount = Math.abs(offset) / slideSize;
+    const tolerance = ESLDefaultCarouselRenderer.NEXT_SLIDE_TOLERANCE;
+    const count = (amount - Math.floor(amount)) > tolerance ? Math.ceil(amount) : Math.floor(amount);
+    const index = this.$carousel.activeIndex + count * sign;
+
+    this.currentIndex = normalizeIndex(index, this);
 
+    // Hm ... that's what actually happens on slide step
     this.$carousel.$$attr('animating', true);
     const stageOffset = -this.getOffset(this.currentIndex);
-
     this.setTransformOffset(stageOffset);
     if (stageOffset !== this.getTransformOffset()) {
       await promisifyTransition(this.$area, 'transform');
     }
-
     this.$carousel.$$attr('animating', false);
+
     this.reorder(this.currentIndex);
     this.setTransformOffset(-this.getOffset(this.currentIndex));
     this.setActive(this.currentIndex, {direction: sign > 0 ? 'next' : 'prev'});


Test patch:
diff --git a/src/modules/esl-carousel/test/core/esl-carousel.nav.test.ts b/src/modules/esl-carousel/test/core/esl-carousel.nav.test.ts
index 11ce29487..b8ec5fda7 100644
--- a/src/modules/esl-carousel/test/core/esl-carousel.nav.test.ts
+++ b/src/modules/esl-carousel/test/core/esl-carousel.nav.test.ts
@@ -1,5 +1,5 @@
 import {
-  normalizeIndex,
+  normalize,
   calcDirection,
   groupToIndex,
   indexToGroup,
@@ -13,7 +13,7 @@ import type {
 } from '../../core/nav/esl-carousel.nav.types';
 
 describe('ESLCarousel: Nav Utils', () => {
-  describe('normalizeIndex', () => {
+  describe('normalize', () => {
     test.each([
       // [index, size, result]
       [0, 5, 0],
@@ -26,7 +26,7 @@ describe('ESLCarousel: Nav Utils', () => {
       [-21, 10, 9]
     ])(
       '(index = %d, size = %d) => %d',
-      (i: number, size: number, result: number) => expect(normalizeIndex(i, size)).toBe(result)
+      (i: number, size: number, result: number) => expect(normalize(i, size)).toBe(result)
     );
   });