ampproject / amphtml

The AMP web component framework.
https://amp.dev
Apache License 2.0
14.9k stars 3.88k forks source link

`amp-analytics` Multiple slideshows does only track one of them #37323

Closed cabaalexander closed 2 years ago

cabaalexander commented 2 years ago

Description

When there are more than one slideshow on a page I expect that each one send a track call when clicking previous or next arrow every time user clicks on it.

Right now the current behavior is that when you click on a slideshow's next arrow and then click on the second slideshow's next arrow, the second slideshow wont trigger the track call and vice versa.

Note: This is the current implementation we have in place to track slideshow changes πŸ‘‡

...
'carouselChange' => [
    'selector'       => '.amp-wp-article-content',
    'on'             => 'amp-carousel-change',
    'request'        => 'track',
    'vars'           => [
        'event' => 'View Photo',
    ],
    'extraUrlParams' => [
        'properties.category' => 'AMP Slideshows',
        'properties.label'    => 'Photo ${toSlide}',
    ],
],
...

The class .amp-wp-article-content is the parent containing both/all slideshows.

Reproduction Steps

Test Link: https://nxsttv-stage.go-vip.net/wcmh/news/local-news/double-slideshow/amp/

  1. open devtools -> network
  2. filter the calls by segment
  3. click on the first slideshow next arrow (see a new track call in the console)
  4. click on the second slideshow next arrow (now there's no track call now, but is should)

Relevant Logs

No response

Browser(s) Affected

No response

OS(s) Affected

No response

Device(s) Affected

No response

AMP Version Affected

No response

alanorozco commented 2 years ago

From amp-analytics documentation:

In the case where a single configuration applies to multiple elements, instead of creating separate configuration for each, it can be simplified by specifying all the selectors at once. To do so, specify an array of selectors that are comma separated and individually enclosed in quote marks.

In order to match multiple elements using the same selector, you'll have to wrap the selector in an array ([]):

            "carouselChange": {
-             "selector": ".amp-wp-article-content",
+             "selector": [".amp-wp-article-content"],