ampproject / ampproject/amphtml

[amp-carousel:1.0] Bento tracking issue

Open
#28,284 1 comment 0 reactions 0 assignees View on GitHub
Stale WG: bento
Dominant language
JavaScript
Stars
14.9k
Forks
4.1k
PR merge metrics
No merged PRs in 30d

Description

## High-level requirements for `amp-base-carousel`
| Consideration | Ready when | Status |
| ------------- | ------------- | ------- |
| Component compatibility | [Conformance with 0.1 (see below)](https://amp.dev/documentation/components/amp-base-carousel/) | ✅ |
| Testing | Unit tests and e2e tests from 0.1 | ✅ |
| Internationalization | RTL supported | ✅ |
| Analytics | `slideChange` supported | ✅ |
| Accessibility | Audit performed, [0.1 for reference](https://github.com/ampproject/amphtml/issues?q=is%3Aissue+is%3Aopen+label%3A%22UI%3A+Component%3A+amp-carousel%22+label%3A%22Related+to%3A+Accessibility%22) |✅ |
| Page experience | Audit performed |✅ |
| Documentation | Written |✅ |
| Storybook | Samples written | ✅ |

### Open tasks

- [x] Initial design and implementation (See #28704 for Preact layer, See #29303 for AMP layer)
- [x] Expose a basic subset of the [documented AMP attributes](https://amp.dev/documentation/components/amp-base-carousel/) as `props` for `BaseCarousel`; existing attributes:
- [x] `loop`
- [x] `controls`
- [x] `mixed-length`
- [x] `visible-count`
- [x] `advance-count`
- [x] `auto-advance`
- [x] `auto-advance-count`
- [x] `auto-advance-interval`
- [x] `auto-advance-loops`
- [x] `snap`
- [x] `snap-align`
- [x] `snap-by`
- [x] `slide`
- [x] `horizontal` -> `orientation`
- [x] Style default arrows similarly to existing ones on `amp-base-carousel`, possibly also [removing `dir`](https://github.com/ampproject/amphtml/pull/28704#discussion_r451800639) (See #29338)
- [x] RTL support
- [x] Resolve in-slide layout (ex. user styled `height: 50%`)
- [x] Use `forwardRef` to [dedup multiple refs](https://github.com/ampproject/amphtml/pull/28704#discussion_r454696809) in `Scroller` and `BaseCarousel` (See #29505, #29650)
- [x] Watch for mutations
- [x] Decide on [boolean=false](https://docs.google.com/document/d/1G_WMXyyKGoHHta6iyXr0dF2B-kLGm_cAoArdVcn0VNg/edit#) attributes
- [x] ~Investigate use of `order` to reorder slides~
- [ ] Position arrows on top/bottom of carousel when `orientation="vertical"` - make sure that this does not cause CLS for `outsetArrows={true}`. [See comment.](https://github.com/ampproject/amphtml/pull/31286#issuecomment-736130609)
- [x] Expose DOM events #32194
- [ ] Expose `actionTrust` or `autoplay` boolean information with event data for `slideChange`

### Migration notes
- `"horizontal"="true"|"false"` in 0.1 is replaced by `"orientation"="horizontal"|"vertical"` in 1.0

## High-level requirements for `amp-inline-gallery`
| Consideration | Ready when | Status |
| ------------- | ------------- | ------- |
| Component compatibility | [Conformance with 0.1 (see below)](https://amp.dev/documentation/components/amp-inline-gallery/) | ✅ |
| Testing | Unit tests | ✅ |
| Internationalization | RTL supported | ✅ |
| Analytics | n/a | n/a |
| Accessibility | Audit performed |✅ |
| Page experience | Audit performed |✅ |
| Documentation | Written |✅ |
| Storybook | Samples written | ✅ |

### Open tasks
- [x] Preact layer (#29674 preferred to #29545)
- [x] Support `Pagination` and optional attribute `inset` (See #29545)
- [x] Support `Thumbnails` and optional attribute `loop`. The following `amp-base-carousel` features must be supported as a prerequisite:
- [x] `snap` and `snapAlign`
- [x] `controls`
- [x] `mixedLength`
- [x] [responsive attributes](https://amp.dev/documentation/components/amp-base-carousel/#using-media-queries)
- [ ] Support `Captions` and `Slide` components

## High-level requirements for `amp-stream-gallery`
| Consideration | Ready when | Status |
| ------------- | ------------- | ------- |
| Component compatibility | n/a (0.1 is experimental) | n/a |
| Testing | Unit tests | ✅ |
| Internationalization | RTL supported | ✅ |
| Analytics | `slideChange` supported | ✅ |
| Accessibility | Audit performed |✅ |
| Page experience | Audit performed |✅ |
| Documentation | Written |✅ |
| Storybook | Samples written | ✅ |

### Open tasks
- [x] Attributes:
- [x] `min-item-width`
- [x] `max-item-width`
- [x] `max-visible-count`
- [x] `inset-arrow-visibility`
- [x] `outset-arrows`
- [x] `loop`
- [x] `snap`
- [x] Watch for mutations

Contributor guide

Open the contributing guide

Research direction

Start by reviewing the remaining unchecked tasks for amp-base-carousel and amp-inline-gallery, especially vertical arrow positioning, slideChange event data, and Captions and Slide support. The issue names no source files or tests, so locate the relevant component entry points and existing unit or end-to-end tests first. Done means the remaining checklist items are implemented and covered by the project’s tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.