Skip to content

feat: implement Material 3 Carousel components#111

Draft
treeder wants to merge 1 commit into
mainfrom
feat-carousel-8823521121697351188
Draft

feat: implement Material 3 Carousel components#111
treeder wants to merge 1 commit into
mainfrom
feat-carousel-8823521121697351188

Conversation

@treeder
Copy link
Copy Markdown
Member

@treeder treeder commented May 12, 2026

Implemented Material Design 3 carousel components.

Includes md-carousel and md-carousel-item written as Lit elements leveraging modern CSS capabilities like flexbox and scroll snapping to achieve native-feeling interactions. Tested visually via Playwright.

Components:

  • md-carousel: The horizontal scroll container (role="list").
  • md-carousel-item: The individual snap targets (role="listitem").

Added demo showcasing usage with md-card and documented customizable CSS variables in a new README.md.


PR created automatically by Jules for task 8823521121697351188 started by @treeder

Added Material Design 3 image carousel components using modern web standards (LitElement, shadow DOM, flexbox, CSS scroll snap).
Includes demo updates and comprehensive component documentation.

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

Copy link
Copy Markdown
Contributor

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

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 introduces a new Material Design 3 carousel component, including the md-carousel and md-carousel-item elements, along with documentation and a demo. Feedback focuses on enhancing accessibility and component robustness. Key recommendations include moving the listitem role to the host element for better screen reader support, internationalizing the default ARIA label, and ensuring scroll-padding matches container padding for accurate scroll snapping. Additionally, the reviewer suggests reconsidering the hidden scrollbar to improve usability and removing development notes from the production code.

Comment thread carousel/carousel-item.js
Comment on lines +8 to +15
export class CarouselItem extends LitElement {
render() {
return html`
<div class="item-container" role="listitem">
<slot></slot>
</div>
`
}
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 role="listitem" should be applied to the host element rather than an internal div. This ensures that the component is correctly identified as a list item by assistive technologies when placed inside a role="list" container (like the one in md-carousel).

export class CarouselItem extends LitElement {
  connectedCallback() {
    super.connectedCallback()
    if (!this.hasAttribute('role')) {
      this.setAttribute('role', 'listitem')
    }
  }

  render() {
    return html`
      <div class="item-container">
        <slot></slot>
      </div>
    `
  }

Comment thread carousel/carousel.js

constructor() {
super()
this.ariaLabel = 'Carousel'
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

The default value for ariaLabel is a hardcoded English string. This should be internationalized using the project's localization system (e.g., @lit/localize) to support multiple languages and improve accessibility for non-English users.

Comment thread carousel/carousel.js
:host {
display: block;
/* Carousel styling based on Material Design 3 */
/* Note: specific width/height may need to be configurable */
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

This comment appears to be a development note. If the component is intended to be production-ready, this note should be removed or the suggested configurability should be implemented.

Comment thread carousel/carousel.js
scrollbar-width: none; /* Firefox */
-ms-overflow-style: none; /* IE and Edge */

padding: var(--md-carousel-padding, 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

When using scroll-snap-type, it is important to set scroll-padding to match the container's padding. This ensures that items snap correctly relative to the visible area of the carousel, preventing them from being partially obscured by the padding.

Suggested change
padding: var(--md-carousel-padding, 0);
padding: var(--md-carousel-padding, 0);
scroll-padding: var(--md-carousel-padding, 0);

Comment thread carousel/carousel.js
scroll-snap-type: x mandatory;
overscroll-behavior-x: contain;

/* Hide scrollbar for cleaner look, but keep functionality */
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

Hiding the scrollbar can negatively impact accessibility and usability, as it removes a visual cue for the amount of content and a means of navigation for some users. Consider allowing the scrollbar to be visible or providing alternative navigation indicators (like dots or arrows) as per Material Design 3 guidelines.

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