Add <ol-carousel> Lit web component - #13077
Merged
Merged
Conversation
lokesh
changed the title
lokesh
force-pushed
the
feat/ol-carousel-component
branch
from
July 1, 2026 05:08
255af84 to
80d4f89
Compare
lokesh
marked this pull request as ready for review
July 1, 2026 05:13
Ports the transform-based OlCarousel web component into the Lit bundle and registers it, as a reusable foundation for rendering book carousels without the legacy Slick dependency. - OlCarousel.js: paged, transform-based horizontal carousel with keyboard/pointer paging, inert offscreen items, and an ol-carousel-page-change event for incremental loading. Debug console logging stripped. - Export it from components/lit/index.js (importing registers the <ol-carousel> custom element). - Regenerate custom-elements.json to include the component. No consumer yet — carousels opt in via a later change; the rest of the site is untouched. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_011xofHmwP8tooS5FMshu1DJ
Surfaces the component on /developers/design alongside the other Lit components: - design.html: a "Carousel" nav entry and a demo article with basic usage, hide-indicators, and page-change-event examples. - Document the default slot and the --ol-carousel-* CSS custom properties with @slot/@cssprop JSDoc so the auto-generated API tables (built from the Custom Elements Manifest in design.py) list them; regenerate custom-elements.json. - page-design.css: a placeholder card style for the demo items. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_011xofHmwP8tooS5FMshu1DJ
Add a tactile press-down (transform: scale(0.92)) to the prev/next arrow buttons on :active, matching the existing pattern in ol-button, ol-chip, ol-pagination, and ol-toast. Snaps with no transition, like the others. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_011xofHmwP8tooS5FMshu1DJ
Match the design-page convention: move the inline <ol-carousel> demo
article into design/carousel.html and render it via
render_template("design/carousel", ...), like tooltip/popover/button/etc.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
Claude-Session: https://claude.ai/code/session_011xofHmwP8tooS5FMshu1DJ
Rename the ol-carousel hideIndicators prop to showIndicators so page indicators are hidden by default and opt-in via the show-indicators attribute. Update the design-page demo and regenerate the manifest.
RayBB
force-pushed
the
feat/ol-carousel-component
branch
from
July 1, 2026 10:16
cd76b9c to
34a33da
Compare
RayBB
approved these changes
Jul 1, 2026
RayBB
left a comment
RayBB
left a comment
Collaborator
There was a problem hiding this comment.
I rebased from master and pushed up some small tweaks to use the new jinja templates.
Tested this on my computer and phone and it works beautifully on both.
AI seems to think scroll-snap can get us most of this with less code but I'm guessing you already considered that and there's something we're losing?
The only bug I see is this:
… total - Clear _draggedPastThreshold in _onPointerUp so a stale flag no longer eats the next arrow/indicator click after an off-element or touch drag-release (deferred in rAF so the drag's own click stays suppressed). - Use --focus-width/--color-focus-ring tokens for focus outlines instead of a hardcoded hex, matching sibling components. - Correct documented peek/gap defaults (0.03/8) in JSDoc + manifest. - Expose page/totalPages getters and seed the demo's page counter on updateComplete so it shows the real total instead of '?'.
Collaborator
Author
|
Fixed the page number CSS |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related to #13062 — the first extracted, independently-reviewable slice of that (too-large) subject-page redesign draft.
carousel.mp4
[feature] Adds the
<ol-carousel>Lit web component — a paged, transform-based horizontal carousel — registers it, and demos it on the developer design page.Why it matters. Carousels are everywhere on the site (home, subject, author, list pages), and today every one is powered by
slick-carousel1.6.0 — an unmaintained jQuery plugin (archived upstream; pinned to a ~2016 build).<ol-carousel>is a single accessible, framework-agnostic primitive the site can converge on, and the first step toward removing slick-carousel + jQuery from a core UI pattern — done incrementally behind an opt-in flag (a later PR) so there's no big-bang swap.<ol-carousel>slick-carousel1.6.0inert+aria-hidden;region/tablistrolesol-carousel-page-changeevent + attributesTechnical
Foundation only, so low-risk: the component is registered but has no production consumer — nothing on the live site renders it; it appears only on
/developers/design. The change is purely additive (no existing component, template, or route is modified —index.jsgains oneexportline). A follow-up PR threads an opt-inweb_componentflag through the carousel macros to render real book rails with it.Testing
npm run lintclean; Lit bundle builds; existing JS suite unaffected (this PR adds no JS tests — see below).make lit-components && docker compose restart web, then/developers/design→ Carousel — arrows page the rail,hide-indicatorsdrops the bar, the page-change demo updates, and arrows scale down on press.No unit tests here by design. The component's value is its interaction (paging, the page-change event, animation, drag), which a jsdom unit test can't exercise — that coverage belongs in a browser-based (Playwright) suite and will land alongside the first real consumer in the follow-up PR, rather than bolting a repo-wide Jest/Lit test-harness change onto this foundation PR.
Stakeholders
@mekarpeles @cdrini