search

ampproject / amphtml

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

amp-carousel-0.2 changes slide on tab change / resize in amp-list #33476

Open p2er opened 3 years ago

p2er commented 3 years ago

What's the issue?

If the amp carousel component is used inside an amp-list it often occurs that the initial slide displayed is the last slide instead of the first slide. On mobile safari the issue can be triggered by switching to a different tab and returning to the tab with the amp-list & amp-carousel combination. On other browsers, e.g desktop chrome, the issue occurs when changing the window size and slides formerly out of viewport become visible.

How do we reproduce the issue?

Safari Mobile

  1. open https://www.cu-camper.com/en/results/?phrase1=Kanada&phrase2=Kanada&date1=25.11.2021&date2=04.12.2021&adults=2&children=0
  2. Wait for results to show up
  3. IS: list of vehicle attributes is visible, SHOULD: show vehicle image (first slide)

In case the images are shown initially:

  1. Scroll down
  2. Switch tabs
  3. Open tab with result list again
  4. IS: list of vehicle attributes is visible, SHOULD: show vehicle image (first slide)

Chrome desktop

  1. open with small viewport https://www.cu-camper.com/en/results/?phrase1=Kanada&phrase2=Kanada&date1=25.11.2021&date2=04.12.2021&adults=2&children=0
  2. Quickly change page dimension, e.g. close inspect window
  3. IS: list of vehicle attributes is visible, SHOULD: show vehicle image (first slide)

What browsers are affected?

Safari mobile 14, Chrome desktop Version 89.0.4389.90

Which AMP version is affected?

AMP ⚡ HTML – Version 2103122145004

https://user-images.githubusercontent.com/12771905/112468626-ca97e700-8d68-11eb-8cbf-506d4067ea6c.mov @sebastianbenz I have also attached a video of the problem.

sebastianbenz commented 3 years ago

Can you please check if the problem still persists with amp-carousel-1.0? //cc @caroqliu

p2er commented 3 years ago

What script tag would i need to use instead of <script src="https://cdn.ampproject.org/v0/amp-carousel-0.2.js" async="" custom-element="amp-carousel"> ? I would assume https://cdn.ampproject.org/v1/amp-carousel-1.0.js but that url does not seem to be the correct one. Neither does https://cdn.ampproject.org/v0/amp-carousel-1.0.js work for me.

sebastianbenz commented 3 years ago

Sorry, my fault. Can you please try amp-base-carousel (the successor of amp-carousel)? The prod version is 0.1, but if that doesn't work please try 1.0. Thanks!

Sidenote: @caroqliu & @CrystalOnScript should we recommend using amp-base-carousel over amp-carousel in the docs?

p2er commented 3 years ago

Sounds good. I'll try it. The only thing that kept me from testing with amp-base-carousel was the experimental flag: https://amp.dev/documentation/components/amp-base-carousel/?format=websites If thats no longer the case I would suggest removing the flag :) Keep you posted about the results.

p2er commented 3 years ago

The base-carousel behaves differently but still unexpected. Instead of jumping to the last slide a random slide is being displayed. In the video you will see the floorplans and other images instead of the exterior shots.

https://user-images.githubusercontent.com/12771905/112480764-a216e980-8d76-11eb-93a7-fe112f3459f0.mov