Skip to content

feat(carousel): mp-carousel web component + ng/react/vue wrappers + no-JS SSR#388

Open
PieterjanDeClippel wants to merge 7 commits into
masterfrom
feat/carousel-web-component
Open

feat(carousel): mp-carousel web component + ng/react/vue wrappers + no-JS SSR#388
PieterjanDeClippel wants to merge 7 commits into
masterfrom
feat/carousel-web-component

Conversation

@PieterjanDeClippel

Copy link
Copy Markdown
Member

Migrates the carousel to the cross-framework WC + per-framework wrappers model, and makes it fully usable with JavaScript disabled.

What's in this branch

  • @mintplayer/web-components/swiper-core — a framework-agnostic SwipeEngine (gesture math, direction-lock, index/wraparound state machine, keyboard), DOM/animation delegated to a host.
  • mp-carousel Lit web component on swiper-coreslide/fade/none, horizontal/vertical, indicators, auto-advance + play/pause, swipe, keyboard, reduced-motion. Slides are projected children (any content, not just images).
  • Angular / React / Vue wrappers over the same element.
  • Demo pages across the three demo apps.

No-JavaScript / SSR tier

The carousel stays usable with scripting off, mirroring the configured mode:

  • slide/none → native scroll-snap strip
  • fade → in-shadow radio + indicator-label crossfade

Both gated by :host(:not(:defined)), so the SwipeEngine cleanly takes over on upgrade.

Delivery (duplication-minimized):

  • One carousel.styles.scss feeds both tiers — the client adopts it via static styles, the DSD inlines the same .cssText.
  • New framework-agnostic @mintplayer/web-components/carousel/ssr: buildMpCarouselDsd (a pure string builder — no lit-ssr at request time, so it runs safely inside @angular/ssr with no Domino clash) + injectMpCarouselDsd, called by all three SSR servers alongside injectMpShellDsd. The only count-dependent CSS (per-index :checked fade rules) is generated in the builder.
  • Wrappers supply slide-count (the builder can't count slotted children) and reflect animation/orientation/aria-label as attributes so the builder can size the DSD. The React wrapper is hand-rolled rather than @lit/react createComponent, because createComponent sets known props as client-only properties (absent from SSR HTML); React 19 serializes the hand-rolled attributes into SSR markup.
  • createRenderRoot is destructive and adopts styles itself — the DSD is the no-JS tier and is intentionally non-hydratable.

Hydrated-path fixes

  • Horizontal track align-items: flex-start — no more stretched images; the viewport height tracks the active slide (height changes after the slide animation completes).
  • A ResizeObserver per slide re-measures once images load.
  • The active fade slide stays in-flow (position: relative) so the carousel doesn't collapse to its controls in a shrink-to-fit container.
  • createRenderRoot adopts styles so the element is styled under the @lit-labs/ssr-client hydrate shim (React/Vue).

Demos

  • bs-select (animation, orientation) + bs-checkbox (indicators) — no raw <input>/<select>.
  • A server-rendered "Without JavaScript" section (a fade + a slide example).
  • Vue wrapper now forwards $attrs (it was silently dropping style/class).

Verification

  • nx build of mintplayer-web-components + all three wrapper libs ✅
  • nx test mintplayer-web-components — 773 passed ✅
  • New Playwright javaScriptEnabled:false specs (carousel-nojs.spec.ts) in all three e2e apps — fade dots crossfade via pure CSS, slide degrades to a scroll-snap strip. Green on Vue + React.
  • JS path verified live on Angular/React/Vue (styles, height, navigation, 0 console errors).

🤖 Generated with Claude Code

PieterjanDeClippel and others added 7 commits June 19, 2026 13:16
Pure-TS engine (gesture math, 3px direction-lock / Firefox-Android PTR, index/wraparound/offside state machine, offset computation, keyboard) with DOM/animation/measurement delegated via SwipeEngineHost. Consumed by the carousel WC and the future full-page swiper. 16 unit tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Slotted slides + imperatively-cloned offside slides for seamless looping (flexbox cell track), WAAPI slide animation, autoplay + prefers-reduced-motion, indicators, prev/next, paused/index + slide-change/paused-change/animation events, horizontal+vertical with equal-height (object-fit:contain, no stretch).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bs-carousel rewritten as a thin wrapper (plain content-projected slides; drops the *bsCarouselImage/*bsCarouselPlayPause/bsCarouselImg directives — breaking, BC-ok). New @lit/react and Vue SFC wrappers with controlled paused/index + events.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Carousel demo pages, routes and sidebar links in all three demo apps; migrated the existing ng carousel + swiper sample pages to the new projection API. Shared animal images added to react/vue demos, tracked via Git LFS (.gitattributes).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Switch ng-bootstrap-demo build/dev-server/extract-i18n to @angular/build (the @angular-devkit/build-angular shims are deprecated and slated for removal). @angular-devkit/build-angular stays as a devDep so the @nx/angular peer override keeps npm install flag-free.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the mode-mirroring no-JavaScript fallback for mp-carousel and fix
several hydrated-path rendering bugs.

No-JS / SSR tier:
- slide/none degrade to a native scroll-snap strip; fade to an in-shadow
  radio + indicator-label crossfade, gated by :host(:not(:defined)).
- New framework-agnostic carousel/ssr: buildMpCarouselDsd (pure string,
  no lit-ssr so it is safe inside @angular/ssr) + injectMpCarouselDsd,
  called by all three SSR servers alongside injectMpShellDsd.
- One carousel.styles.scss feeds both tiers; per-index fade CSS is the
  only count-dependent piece and is generated in the builder.
- Wrappers supply slide-count and reflect animation/orientation/aria-label
  as attributes so the builder can size the DSD. React wrapper rewritten
  off @lit/react createComponent to a hand-rolled tag (createComponent
  sets known props as client-only properties, absent from SSR HTML).
- createRenderRoot is destructive and adopts styles itself (the DSD is the
  no-JS tier, intentionally non-hydratable).

Hydrated-path fixes:
- Horizontal track align-items:flex-start (no stretch; height tracks the
  active slide); vertical centers content.
- ResizeObserver per slide so height re-measures after images load.
- Active fade slide stays in-flow so the carousel does not collapse in a
  shrink-to-fit container.

Demos use bs-select (animation, orientation) and bs-checkbox (indicators),
plus a server-rendered "Without JavaScript" section. Vue wrapper forwards
$attrs. Playwright javaScriptEnabled:false specs added in all three e2e apps.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request migrates the carousel component to a framework-agnostic Lit web component (<mp-carousel>) powered by a new core swipe engine (SwipeEngine), with corresponding wrappers for Angular, React, and Vue. It also introduces server-side rendering (SSR) support via Declarative Shadow DOM (DSD) to ensure the carousel remains functional with JavaScript disabled. The code review identified several critical issues: the web component needs to handle reconnection lifecycle events properly by re-initializing the swipe engine and re-registering event listeners, the SSR regex parser needs to be hardened against attributes containing > characters to prevent malformed HTML, and the Vue wrapper's slide-counting logic requires an array guard to prevent runtime TypeError crashes.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

];
}

private readonly engine: SwipeEngine;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Remove the readonly modifier from the engine property. Since the carousel needs to be fully re-initialized upon reconnection in connectedCallback to prevent frozen state/broken gestures, the engine property must be mutable.

Suggested change
private readonly engine: SwipeEngine;
private engine: SwipeEngine;

Comment on lines +217 to +221
override connectedCallback(): void {
super.connectedCallback();
this.reducedMotionQuery = window.matchMedia?.('(prefers-reduced-motion: reduce)') ?? null;
this.reducedMotionQuery?.addEventListener('change', this.onReducedMotionChange);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Upon element disconnection and reconnection, firstUpdated() does not run again. This causes several critical issues: the SwipeEngine remains permanently destroyed (frozen state), the ResizeObserver is never re-observed, and the touch event listeners on this.trackEl are never re-registered. Re-initialize the engine, re-register event listeners, and re-observe the slides when reconnecting (i.e., when this.hasUpdated is true).

  override connectedCallback(): void {
    super.connectedCallback();
    this.reducedMotionQuery = window.matchMedia?.('(prefers-reduced-motion: reduce)') ?? null;
    this.reducedMotionQuery?.addEventListener('change', this.onReducedMotionChange);

    if (this.hasUpdated) {
      this.trackEl?.addEventListener('touchstart', this.onTouchStart, { passive: true });
      this.trackEl?.addEventListener('touchmove', this.onTouchMove, { passive: false });
      this.trackEl?.addEventListener('touchend', this.onTouchEnd, { passive: false });

      this.engine = new SwipeEngine(
        this,
        {
          onIndexChange: (i) => this.handleIndexChange(i),
          onAnimationStart: () => {
            this.isAnimating = true;
            this.dispatchEvent(new CustomEvent('animation-start', { bubbles: true, composed: true }));
          },
          onAnimationEnd: () => {
            this.isAnimating = false;
            this.dispatchEvent(new CustomEvent('animation-end', { bubbles: true, composed: true }));
            this.requestUpdate();
          },
        },
        this.readConfig(),
      );
      this.refreshSlides();
      this.engine.markReady();
      this.restartAutoAdvance();
    }
  }

// template (the negative lookahead makes a second pass a no-op, and — unlike a
// page-global `shadowrootmode` check — won't be tripped by some *other* element
// emitting DSD). Capture group 1 is the tag's raw attribute text.
const OPEN_TAG = /<mp-carousel\b([^>]*)>(?!\s*<template\b[^>]*shadowrootmode)/g;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The OPEN_TAG regular expression uses [^>]* to match attributes. If any attribute (such as aria-label or style) contains a > character (e.g., aria-label="Next >"), the regex will prematurely terminate at that character, splitting the tag and rendering malformed HTML on the page. Use a more robust regex that correctly handles single and double quotes within attributes.

Suggested change
const OPEN_TAG = /<mp-carousel\b([^>]*)>(?!\s*<template\b[^>]*shadowrootmode)/g;
const OPEN_TAG = /<mp-carousel\b((?:[^>"']|"[^"]*"|'[^']*')*)>(?!\s*<template\b[^>]*shadowrootmode)/g;

Comment on lines +51 to +57
function countSlides(vnodes: VNode[] | undefined): number {
return (vnodes ?? []).reduce((n, v) => {
if (v.type === Fragment) return n + countSlides(v.children as VNode[]);
if (v.type === Comment || v.type === Text) return n;
return n + 1;
}, 0);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If vnodes is not an array (for example, if a fragment contains only a single text node or is empty, where v.children can be a string or undefined), calling .reduce() on it will throw a runtime TypeError and crash the component. Guard the reduction by ensuring vnodes is a valid array.

function countSlides(vnodes: VNode[] | undefined): number {
  if (!Array.isArray(vnodes)) return 0;
  return vnodes.reduce((n, v) => {
    if (v.type === Fragment) return n + countSlides(v.children as VNode[]);
    if (v.type === Comment || v.type === Text) return n;
    return n + 1;
  }, 0);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant