Skip to content

Commit 34a70fa

Browse files
ala-nala-n
committed
fix(esl-carousel): fix incorrect canNavigate behaviour for undefined direction navigation (equal distance for both direction to reach the target slide)
1 parent b02207a commit 34a70fa

File tree

3 files changed

+73
-63
lines changed

3 files changed

+73
-63
lines changed

‎packages/esl/src/esl-carousel/core/esl-carousel.utils.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,17 @@ import type {
1010
/** @returns sign of the value */
1111
export const sign = (value: number): -1 | 1 | 0 => value > 0 ? 1 : value < 0 ? -1 : 0;
1212

13+
const bounds =
14+
(value: number, min: number, max: number): number => Math.max(min, Math.min(max, value));
15+
1316
/** @returns normalized slide index in bounds of [0, count] range */
1417
export function normalize(index: number, size: number): number {
1518
return (size + (index % size)) % size;
1619
}
1720

1821
/** @returns normalize first slide index according to the carousel mode */
1922
export function normalizeIndex(index: number, {size, count, loop}: ESLCarouselStaticState): number {
20-
return loop && count < size ? normalize(index, size) : Math.max(0, Math.min(size - count, index));
23+
return loop && count < size ? normalize(index, size) : bounds(index, 0, size - count);
2124
}
2225

2326
/** @returns normalized sequence of slides starting from the current index */
@@ -129,5 +132,5 @@ export function canNavigate(target: ESLCarouselSlideTarget, cfg: ESLCarouselStat
129132
if (cfg.size <= cfg.count) return false;
130133
const {direction, index} = toIndex(target, cfg);
131134
if (!cfg.loop && direction && index < direction * cfg.activeIndex) return false;
132-
return !!direction && index !== cfg.activeIndex;
135+
return index !== cfg.activeIndex;
133136
}

‎packages/esl/src/esl-carousel/test/core/esl-carousel.utils.test.ts renamed to ‎packages/esl/src/esl-carousel/test/core/esl-carousel.utils.base.test.ts

Lines changed: 2 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,8 @@
1-
import {
2-
normalize,
3-
groupToIndex,
4-
indexToGroup,
5-
indexToDirection,
6-
toIndex, canNavigate
7-
} from '../../core/esl-carousel.utils';
1+
import {normalize, groupToIndex, indexToGroup, indexToDirection, toIndex} from '../../core/esl-carousel.utils';
82
import {ESLCarouselDirection} from '../../core/esl-carousel.types';
9-
103
import type {ESLCarouselSlideTarget, ESLCarouselState} from '../../core/esl-carousel.types';
114

12-
describe('ESLCarousel: Nav Utils', () => {
5+
describe('ESLCarousel: Nav Utils (Core)', () => {
136
describe('normalize', () => {
147
test.each([
158
// [index, size, result]
@@ -285,56 +278,4 @@ describe('ESLCarousel: Nav Utils', () => {
285278
);
286279
});
287280
});
288-
289-
describe('canNavigate', () => {
290-
test.each([
291-
// Loop for complete
292-
['next', {activeIndex: 0, size: 5, count: 1, loop: true}],
293-
['prev', {activeIndex: 0, size: 5, count: 1, loop: true}],
294-
['next', {activeIndex: 4, size: 5, count: 1, loop: true}],
295-
// Non loop case with free space
296-
['next', {activeIndex: 0, size: 5, count: 1, loop: false}],
297-
['prev', {activeIndex: 4, size: 5, count: 1, loop: false}],
298-
['next', {activeIndex: 2, size: 5, count: 1, loop: false}],
299-
['prev', {activeIndex: 2, size: 5, count: 1, loop: false}],
300-
// Group cases
301-
['group: next', {activeIndex: 0, size: 5, count: 2, loop: true}],
302-
['group: prev', {activeIndex: 0, size: 5, count: 2, loop: true}],
303-
['group: next', {activeIndex: 4, size: 5, count: 2, loop: true}],
304-
['group: prev', {activeIndex: 4, size: 5, count: 2, loop: true}],
305-
// Non loop case with free space
306-
['group: next', {activeIndex: 0, size: 5, count: 2, loop: false}],
307-
['group: prev', {activeIndex: 2, size: 5, count: 2, loop: false}],
308-
['group: prev', {activeIndex: 4, size: 5, count: 2, loop: false}]
309-
])(
310-
'Target %s is allowed for %p configuration',
311-
(target: ESLCarouselSlideTarget, cfg: ESLCarouselState) => expect(canNavigate(target, cfg)).toBe(true)
312-
);
313-
314-
test.each([
315-
// Non loop case with no free space
316-
['next', {activeIndex: 4, size: 5, count: 1, loop: false}],
317-
['prev', {activeIndex: 0, size: 5, count: 1, loop: false}],
318-
// Incomplete cases
319-
['next', {activeIndex: 0, size: 1, count: 1, loop: false}],
320-
['prev', {activeIndex: 0, size: 1, count: 1, loop: false}],
321-
['next', {activeIndex: 0, size: 1, count: 1, loop: true}],
322-
['prev', {activeIndex: 0, size: 1, count: 1, loop: true}],
323-
['next', {activeIndex: 0, size: 2, count: 2, loop: true}],
324-
['prev', {activeIndex: 0, size: 2, count: 2, loop: true}],
325-
['next', {activeIndex: 0, size: 1, count: 2, loop: true}],
326-
['prev', {activeIndex: 0, size: 1, count: 2, loop: true}],
327-
// Same slide
328-
['0', {activeIndex: 0, size: 5, count: 1, loop: false}],
329-
['0', {activeIndex: 0, size: 5, count: 1, loop: true}],
330-
['1', {activeIndex: 1, size: 5, count: 2, loop: false}],
331-
['1', {activeIndex: 1, size: 5, count: 2, loop: true}],
332-
// Group cases with no loop
333-
['group: next', {activeIndex: 4, size: 5, count: 2, loop: false}],
334-
['group: prev', {activeIndex: 0, size: 5, count: 2, loop: false}]
335-
])(
336-
'Target %s is not allowed for %p configuration',
337-
(target: ESLCarouselSlideTarget, cfg: ESLCarouselState) => expect(canNavigate(target, cfg)).toBe(false)
338-
);
339-
});
340281
});

‎packages/esl/src/esl-carousel/test/core/esl-carousel.utils.nav.test.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import {canNavigate} from '../../core/esl-carousel.utils';
2+
import type {ESLCarouselSlideTarget, ESLCarouselState} from '../../core/esl-carousel.types';
3+
4+
describe('ESLCarousel: Nav Utils', () => {
5+
describe('canNavigate', () => {
6+
test.each([
7+
// Loop for complete
8+
['next', {activeIndex: 0, size: 5, count: 1, loop: true}],
9+
['prev', {activeIndex: 0, size: 5, count: 1, loop: true}],
10+
['next', {activeIndex: 4, size: 5, count: 1, loop: true}],
11+
// Non loop case with free space
12+
['next', {activeIndex: 0, size: 5, count: 1, loop: false}],
13+
['prev', {activeIndex: 4, size: 5, count: 1, loop: false}],
14+
['next', {activeIndex: 2, size: 5, count: 1, loop: false}],
15+
['prev', {activeIndex: 2, size: 5, count: 1, loop: false}],
16+
// Group cases
17+
['group: next', {activeIndex: 0, size: 5, count: 2, loop: true}],
18+
['group: prev', {activeIndex: 0, size: 5, count: 2, loop: true}],
19+
['group: next', {activeIndex: 4, size: 5, count: 2, loop: true}],
20+
['group: prev', {activeIndex: 4, size: 5, count: 2, loop: true}],
21+
// Non loop case with free space
22+
['group: next', {activeIndex: 0, size: 5, count: 2, loop: false}],
23+
['group: prev', {activeIndex: 2, size: 5, count: 2, loop: false}],
24+
['group: prev', {activeIndex: 4, size: 5, count: 2, loop: false}],
25+
// Indexes
26+
['slide:1', {activeIndex: 0, size: 4, count: 1, loop: true}],
27+
['slide:2', {activeIndex: 0, size: 4, count: 1, loop: true}],
28+
['slide:3', {activeIndex: 0, size: 4, count: 1, loop: true}],
29+
['slide:0', {activeIndex: 1, size: 4, count: 1, loop: true}],
30+
['slide:2', {activeIndex: 1, size: 4, count: 1, loop: true}],
31+
['slide:3', {activeIndex: 1, size: 4, count: 1, loop: true}],
32+
])(
33+
'Target %s is allowed for %p configuration',
34+
(target: ESLCarouselSlideTarget, cfg: ESLCarouselState) => expect(canNavigate(target, cfg)).toBe(true)
35+
);
36+
37+
test.each([
38+
// Non loop case with no free space
39+
['next', {activeIndex: 4, size: 5, count: 1, loop: false}],
40+
['prev', {activeIndex: 0, size: 5, count: 1, loop: false}],
41+
// Incomplete cases
42+
['next', {activeIndex: 0, size: 1, count: 1, loop: false}],
43+
['prev', {activeIndex: 0, size: 1, count: 1, loop: false}],
44+
['next', {activeIndex: 0, size: 1, count: 1, loop: true}],
45+
['prev', {activeIndex: 0, size: 1, count: 1, loop: true}],
46+
['next', {activeIndex: 0, size: 2, count: 2, loop: true}],
47+
['prev', {activeIndex: 0, size: 2, count: 2, loop: true}],
48+
['next', {activeIndex: 0, size: 1, count: 2, loop: true}],
49+
['prev', {activeIndex: 0, size: 1, count: 2, loop: true}],
50+
// Same slide
51+
['0', {activeIndex: 0, size: 5, count: 1, loop: false}],
52+
['0', {activeIndex: 0, size: 5, count: 1, loop: true}],
53+
['1', {activeIndex: 1, size: 5, count: 2, loop: false}],
54+
['1', {activeIndex: 1, size: 5, count: 2, loop: true}],
55+
// Group cases with no loop
56+
['group: next', {activeIndex: 4, size: 5, count: 2, loop: false}],
57+
['group: prev', {activeIndex: 0, size: 5, count: 2, loop: false}],
58+
// Indexes
59+
['slide:0', {activeIndex: 0, size: 4, count: 1, loop: true}],
60+
['slide:1', {activeIndex: 1, size: 4, count: 1, loop: true}]
61+
])(
62+
'Target %s is not allowed for %p configuration',
63+
(target: ESLCarouselSlideTarget, cfg: ESLCarouselState) => expect(canNavigate(target, cfg)).toBe(false)
64+
);
65+
});
66+
});

0 commit comments

Comments
 (0)