Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions all.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
*/
import './buttons/elevated-button.js'
import './buttons/button.js'
import './carousel/carousel.js'
import './carousel/carousel-item.js'
Comment on lines +9 to +10
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 imports for the carousel component are currently interspersed with the button component imports (e.g., between button.js and filled-tonal-button.js). It is recommended to group imports by component type to maintain a clean and organized structure. Consider moving these imports after the complete buttons group.

import './buttons/filled-tonal-button.js'
import './buttons/outlined-button.js'
import './buttons/text-button.js'
Expand Down Expand Up @@ -48,6 +50,8 @@ import './text/text-field.js'
// LINT.IfChange(exports)
// go/keep-sorted start
export * from './buttons/button.js'
export * from './carousel/carousel.js'
export * from './carousel/carousel-item.js'
export * from './checkbox/checkbox.js'
export * from './chips/chip.js'
export * from './chips/chip-set.js'
Expand Down
32 changes: 32 additions & 0 deletions carousel/carousel-item.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { html, LitElement, css } from 'lit'

export class CarouselItem extends LitElement {
static properties = {}

constructor() {
super()
}
Comment on lines +4 to +8
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 static properties object and the constructor are currently empty and do not contain any logic. They can be removed to simplify the component definition and reduce boilerplate.


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

static styles = css`
:host {
display: block;
scroll-snap-align: start;
flex-shrink: 0;
}
.item {
display: block;
border-radius: var(--md-carousel-item-shape, 24px);
overflow: hidden;
}
`
}

customElements.define('md-carousel-item', CarouselItem)
41 changes: 41 additions & 0 deletions carousel/carousel.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { html, LitElement, css } from 'lit'

export class Carousel extends LitElement {
static properties = {}

constructor() {
super()
}
Comment on lines +4 to +8
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 empty constructor can be removed. Additionally, consider adding an ariaLabel property to allow users to provide a localized or custom label for the carousel, which is important for accessibility and internationalization.

  static properties = {
    /**
     * The aria-label of the carousel.
     */
    ariaLabel: {attribute: 'aria-label'},
  };

  ariaLabel = 'Carousel';


render() {
return html`
<div class="carousel" role="list" aria-label="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

Use the ariaLabel property to set the aria-label attribute on the internal carousel container. This allows the component to support dynamic and localized labels instead of a hardcoded string.

Suggested change
<div class="carousel" role="list" aria-label="Carousel">
<div class="carousel" role="list" aria-label=${this.ariaLabel || 'Carousel'}>

<slot></slot>
</div>
`
}

static styles = css`
:host {
display: block;
}
.carousel {
display: flex;
overflow-x: auto;
scroll-snap-type: x mandatory;
scrollbar-width: none; /* Firefox */
-ms-overflow-style: none; /* IE and Edge */
gap: 8px; /* Optional gap */
padding: 16px;
Comment on lines +28 to +29
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

Using hardcoded values for gap and padding limits the flexibility of the component. It is better to use CSS custom properties to allow for easier theming and consistency with other Material components in the project.

Suggested change
gap: 8px; /* Optional gap */
padding: 16px;
gap: var(--md-carousel-gap, 8px);
padding: var(--md-carousel-padding, 16px);

scroll-behavior: smooth;
}
.carousel::-webkit-scrollbar {
display: none; /* Chrome, Safari and Opera */
}
::slotted(*) {
scroll-snap-align: start;
}
`
}

customElements.define('md-carousel', Carousel)
4 changes: 4 additions & 0 deletions common.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
* for production.
*/
import './buttons/button.js'
import './carousel/carousel.js'
import './carousel/carousel-item.js'
import './checkbox/checkbox.js'
import './chips/chip.js'
import './chips/chip-set.js'
Expand All @@ -27,6 +29,8 @@ import './tabs/tabs.js'
import './text/text-field.js'

export * from './buttons/button.js'
export * from './carousel/carousel.js'
export * from './carousel/carousel-item.js'
export * from './checkbox/checkbox.js'
export * from './chips/chip.js'
export * from './chips/chip-set.js'
Expand Down
44 changes: 44 additions & 0 deletions demo/components/expressive-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,50 @@ class ExpressiveComponent extends LitElement {
value-end="75"></md-slider>
</div>

<h3>Carousel</h3>
<md-carousel style="max-width: 600px; border-radius: 12px; background: var(--md-sys-color-surface-container);">
<md-carousel-item style="width: 300px;">
<md-card type="outlined">
<div class="flex col">
<img src="./images/img1.jpg" />
</div>
<div class="flex col g12 p16">
<div class="card-title">Carousel Item 1</div>
</div>
</md-card>
</md-carousel-item>
<md-carousel-item style="width: 300px;">
<md-card type="outlined">
<div class="flex col">
<img src="./images/img2.jpg" />
</div>
<div class="flex col g12 p16">
<div class="card-title">Carousel Item 2</div>
</div>
</md-card>
</md-carousel-item>
<md-carousel-item style="width: 300px;">
<md-card type="outlined">
<div class="flex col">
<img src="./images/img1.jpg" />
</div>
<div class="flex col g12 p16">
<div class="card-title">Carousel Item 3</div>
</div>
</md-card>
</md-carousel-item>
<md-carousel-item style="width: 300px;">
<md-card type="outlined">
<div class="flex col">
<img src="./images/img2.jpg" />
</div>
<div class="flex col g12 p16">
<div class="card-title">Carousel Item 4</div>
</div>
</md-card>
</md-carousel-item>
</md-carousel>

<h3>Cards</h3>
<div class="flexw g12">
<md-card type="outlined">
Expand Down
2 changes: 2 additions & 0 deletions demo/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@
import 'material/snackbar/snackbar.js'
import 'material/app/bar.js'
import 'material/progress/progress.js'
import 'material/carousel/carousel.js'
import 'material/carousel/carousel-item.js'
import './components/expressive-component.js'
</script>

Expand Down