Skip to content

Commit

Permalink
fix(esl-carousel): fix esl-carousel DOM manipulation and slides livec…
Browse files Browse the repository at this point in the history
…ycle
  • Loading branch information
ala-n committed Aug 21, 2024
1 parent 22900a0 commit 9ab2b6b
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 6 deletions.
5 changes: 3 additions & 2 deletions src/modules/esl-carousel/core/esl-carousel.slide.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ export class ESLCarouselSlide extends ESLMixinElement {
return this.$host.closest(carouselTag) as ESLCarousel | undefined;
}

@ready
protected override connectedCallback(): void {
if (!this.$carousel) return;
this.$carousel?.addSlide && this.$carousel.addSlide(this.$host);
Expand All @@ -59,7 +58,9 @@ export class ESLCarouselSlide extends ESLMixinElement {
}

protected override disconnectedCallback(): void {
this.$carousel?.removeSlide && this.$carousel.removeSlide(this.$host);
// A disconnected callback is not directly related to slide removal from the carousel
// e.g. carousel itself can be removed from the DOM so child slides behaves accordingly
this.$carousel?.update() && this.$carousel?.update();
memoize.clear(this, '$carousel');
super.disconnectedCallback();
}
Expand Down
5 changes: 1 addition & 4 deletions src/modules/esl-carousel/core/esl-carousel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,17 +183,14 @@ export class ESLCarousel extends ESLBaseElement {

/** Appends slide instance to the current carousel */
public addSlide(slide: HTMLElement): void {
if (!slide) return;
slide.setAttribute(this.slideAttrName, '');
if (slide.parentNode === this.$slidesArea) return this.update();
console.debug('[ESL]: ESLCarousel moves slide to correct location', slide);
if (slide.parentNode) slide.remove();
Promise.resolve().then(() => this.$slidesArea.appendChild(slide));
this.$slidesArea.appendChild(slide);
}

/** Remove slide instance from the current carousel */
public removeSlide(slide: HTMLElement): void {
if (!slide) return;
if (slide.parentNode === this.$slidesArea) this.$slidesArea.removeChild(slide);
if (this.$slides.includes(slide)) this.update();
}
Expand Down
102 changes: 102 additions & 0 deletions src/modules/esl-carousel/test/core/esl-carousel.manip.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import {ESLCarousel} from '../../core/esl-carousel';
import {ESLCarouselSlide} from '../../core/esl-carousel.slide';
import {ESLCarouselDummyRenderer} from '../common/esl-carousel.dummy.renderer';

describe('ESLCarousel: DOM manipulation', () => {
const twoTicks = () => Promise.resolve().then(() => Promise.resolve());

beforeAll(() => {
ESLCarousel.register();
ESLCarouselDummyRenderer.register();
});

describe('ESLCarouselSlide ensure connection to the carousel inside the slide area', () => {
const $carousel = ESLCarousel.create();

beforeEach(async () => {
document.body.appendChild($carousel);
await ESLCarousel.registered;
});
afterAll(() => document.body.innerHTML = '');

test('DOM element marked as slide outside carousel does not move itself', async () => {
const $slide = document.createElement('div');
$slide.setAttribute('esl-carousel-slide', '');
document.body.appendChild($slide);
await Promise.resolve();
expect($slide.parentElement).toBe(document.body);
});

test('DOM element marked as slide inside carousel moves itself to the carousel area', async () => {
jest.spyOn(console, 'debug').mockImplementationOnce(() => {});
const $slide = document.createElement('div');
$slide.setAttribute('esl-carousel-slide', '');
$carousel.appendChild($slide);
await twoTicks();
expect($slide.parentElement).toBe($carousel.$slidesArea);
});

test('DOM element marked as slide inside carousel area does not process twice', async () => {
const $slide = document.createElement('div');
$slide.setAttribute('esl-carousel-slide', '');
$carousel.$slidesArea.appendChild($slide);
const connectedCallbackSpy = jest.spyOn(ESLCarouselSlide.prototype, 'connectedCallback' as any);
await twoTicks();
expect($carousel.$slides).toContain($slide);
expect($slide.parentElement).toBe($carousel.$slidesArea);
expect(connectedCallbackSpy).toHaveBeenCalledTimes(1);
});

test('Usage of the ESLCarousel.addSlide method creates proper slide', async () => {
const $slide = document.createElement('div');
$carousel.addSlide($slide);
await twoTicks();
expect($carousel.$slides).toContain($slide);
expect($slide.parentElement).toBe($carousel.$slidesArea);
});

test('Removal of the slide ensures carousel be aware of it', async () => {
const $slide = document.createElement('div');
$carousel.addSlide($slide);
await twoTicks();
expect($carousel.$slides).toContain($slide);
expect($slide.parentElement).toBe($carousel.$slidesArea);
$slide.remove();
await twoTicks();
expect($carousel.$slides).not.toContain($slide);
expect($slide.parentElement).toBe(null);
});
});

describe('ESLCarousel manipulation does not affect slides', () => {
const $carousel = ESLCarousel.create();
const $slides = Array.from({length: 2}, () => {
const $slide = document.createElement('div');
$slide.setAttribute('esl-carousel-slide', '');
return $slide;
});

beforeEach(async () => {
document.body.appendChild($carousel);
await ESLCarousel.registered;
$slides.forEach(($slide) => $carousel.addSlide($slide));
await twoTicks();
});
afterAll(() => document.body.innerHTML = '');

test('Carousel removal does not affect slides', async () => {
$carousel.remove();
await twoTicks();
$slides.forEach(($slide) => expect($slide.parentElement).toBe($carousel.$slidesArea));
});

test('Carousel movement does not affect slides', async () => {
const $someParent = document.createElement('div');
document.body.appendChild($someParent);
$someParent.appendChild($carousel);
await twoTicks();
expect($carousel.parentElement).toBe($someParent);
expect($carousel.$slides).toEqual($slides);
});
});
});

0 comments on commit 9ab2b6b

Please sign in to comment.