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 @@ -9,6 +9,8 @@ import './buttons/button.js'
import './buttons/filled-tonal-button.js'
import './buttons/outlined-button.js'
import './buttons/text-button.js'
import './carousel/carousel.js'
import './carousel/carousel-item.js'
import './checkbox/checkbox.js'
import './chips/assist-chip.js'
import './chips/chip-set.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
46 changes: 46 additions & 0 deletions carousel/carousel-item.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { html, LitElement, css, isServer } from 'lit'

export class CarouselItem extends LitElement {
constructor() {
super()
this.internals = this.attachInternals()
if (!isServer) {
this.internals.role = 'listitem'
}
}

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

static styles = [
css`
:host {
display: block;
border-radius: var(--md-carousel-item-border-radius, 28px);
overflow: hidden;
height: 100%;
width: var(--md-carousel-item-width, 240px); /* Multi-browse width by default */
}
.carousel-item {
display: flex;
flex-direction: column;
justify-content: center;
align-items: center;
width: 100%;
height: 100%;
}
::slotted(img) {
width: 100%;
height: 100%;
object-fit: cover;
}
`,
]
}

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

export class Carousel extends LitElement {
constructor() {
super()
this.internals = this.attachInternals()
if (!isServer) {
this.internals.role = 'list'
}
}

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

The aria-label="carousel" is a hardcoded string, which is problematic for internationalization. Additionally, aria-label should generally not be used on a div without a semantic role (like region or list), as it may be ignored by assistive technologies. Since the host element already has role="list", any descriptive label should be provided by the consumer on the host element itself.

Suggested change
<div class="carousel" aria-label="carousel">
<div class="carousel">

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

static styles = [
css`
:host {
display: block;
width: 100%;
overflow: hidden;
border-radius: var(--md-carousel-border-radius, 28px);
}
.carousel {
display: flex;
overflow-x: auto;
scroll-snap-type: x mandatory;
scrollbar-width: none; /* Firefox */
-ms-overflow-style: none; /* Internet Explorer 10+ */
gap: var(--md-carousel-gap, 8px);
padding: var(--md-carousel-padding, 0);
height: 100%;
}
.carousel::-webkit-scrollbar {
display: none; /* WebKit */
}
::slotted(*) {
flex: 0 0 auto;
scroll-snap-align: start;

/* M3 dynamic item sizing using CSS Scroll-driven Animations */
animation: material-carousel-item-resize linear both;
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 animation shorthand is missing a duration. While animation-timeline is used to drive the progress, many browsers still require a non-zero duration (or the auto keyword) for the animation to be considered active and to function correctly across different implementations of the Scroll-driven Animations spec.

Suggested change
animation: material-carousel-item-resize linear both;
animation: material-carousel-item-resize auto linear both;

animation-timeline: view(inline);
}

@keyframes material-carousel-item-resize {
0% {
width: var(--md-carousel-small-width, 40px);
}
15% {
width: var(--md-carousel-large-width, 240px);
}
85% {
width: var(--md-carousel-large-width, 240px);
}
Comment on lines +54 to +59
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 variable name --md-carousel-large-width is inconsistent with --md-carousel-item-width used in carousel-item.js. Using a single variable for the item's standard width improves maintainability and ensures that customizations to the item size are correctly reflected in the carousel's dynamic resizing animation.

Suggested change
15% {
width: var(--md-carousel-large-width, 240px);
}
85% {
width: var(--md-carousel-large-width, 240px);
}
15% {
width: var(--md-carousel-item-width, 240px);
}
85% {
width: var(--md-carousel-item-width, 240px);
}

100% {
width: var(--md-carousel-small-width, 40px);
}
}
`,
]
}

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