search

ampproject / amphtml

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

New amp-carousel jitters when changing slides #5407

Closed poscar closed 7 years ago

poscar commented 8 years ago

How do we reproduce the issue?

Observe the carousel included in the following example: http://output.jsbin.com/mojuqakosu

  1. Switch slides by clicking the next or prev buttons.

Result: Jitter as slides are transitioned. Expected Result: No jitter.

What browsers are affected?

Specifically tested: Chrome: 53.0.2785.116 Safari: 9.0.3 (11601.4.4)

Which AMP version is affected?

Powered by AMP ⚡ HTML – Version 1475106123549

Note that we did not observe any jitter with this same markup under the old amp-carousel. Any ideas?

erwinmombay commented 8 years ago

/to @sriramkrish85

camelburrito commented 8 years ago

@poscar is this on a linux machine?

camelburrito commented 8 years ago

if this is on a linux machine - possibly a dupe of - https://github.com/ampproject/amphtml/issues/5092

poscar commented 8 years ago

@sriramkrish85 I'm under MacOS 10.11.3

camelburrito commented 8 years ago

Okay ! Could you upload a video of what you are seeing ? That would be very helpful

poscar commented 8 years ago

@sriramkrish85 yessir, here it is: amp-carousel-jitter.zip

camelburrito commented 8 years ago

Ugh. That looks ugly - (im still not able to repro on my mac :) )

Chrome: Version 53.0.2785.116 (64-bit) OSX - 10.11.6 (15G31)

Will dig into this.

camelburrito commented 8 years ago

@poscar i could not see images on your js bin may be thats why im seeing the jitter - could you post a direct url if you have one.

poscar commented 8 years ago

@sriramkrish85 ah yes, sorry. Here it is: http://www.wmagazine.com/story/why-alice-lancaster-and-other-young-artists-are-making-their-names-with-fashion-collabs-first/amp

jaygray0919 commented 8 years ago

Per previous issue, can also be seen here: https://www.ampproject.org/

jaygray0919 commented 8 years ago

Seems to us that this is a severity-level-1 problem. We see this problem on our carousel's, on Google AMP carousels, and on publisher carousels such as the Guardian.

camelburrito commented 8 years ago

@poscar - https://www.youtube.com/watch?v=NYQvNFiXG7Q - this is what i am seeing on

Chrome: Version 53.0.2785.116 (64-bit) OSX - 10.11.6 (15G31)

On an external monitor. Looks like the issue is on smaller screens.

Works perfectly fine on all Cellphones too.

@jaygray0919 - this has nothing to do with Google search carousel

Will try and see if can isolate the problem and do a quick fix.

camelburrito commented 8 years ago

Works well on my 13" mac pro - but was able to repro on my colleague's 15" pro. Will keep digging

poscar commented 8 years ago

@camelburrito

This is very weird.

I see this: https://youtu.be/dFPX5pQeKOM

camelburrito commented 8 years ago

@poscar @jaygray0919 can you post your monitor sizes (and makes if using external monitor) and mention if you are using retina displays.

@poscar could you try this out on some other machine/monitor.

@jaygray0919 -- understood - the issue is with the carousel so we will see it on all sites -- also post your browser/os versions

poscar commented 8 years ago

@camelburrito it doesn't happen on my machine if I simulate an iPhone 6 with the Chrome Dev Tools. Leads me to believe that this might some inconsistent animation math within the new amp-carousel component.

NOTE: This is on a 15" MacBook Pro Retina with a HP Z27S (4k) external monitor.

jaygray0919 commented 8 years ago

Am running dual HP w2007 monitors, Windows 10 Pro. 1050 x 1680 and reverse. Other team members are running smaller external monitors on Windows 7 and Windows 10 platforms - they too see the same problem. One team member runs a MacBook (uncertain version) and does not see the problem.

jaygray0919 commented 8 years ago

We believe this problem began 4 to 6 weeks ago. We know all of our implementations worked before then. We began to see the problem ~ 4 weeks ago but didn't take it seriously. A week ago we revisited previous work and realized we had a problem. When we reproduced the results on Google pages and Guardian pages, we were somewhat relieved because it no longer seemed to be just our problem. No snarking here, just the day-to-day reality of software development.

camelburrito commented 8 years ago

the only difference i see between the machines is the model of the graphics cards :) This is hard to

@jaygray0919 - We did launch an improvement on carousels last week (now you can have good swiping experience on your mobile devices and now we can swipe through iframes/embeds). not sure what you saw 4-6 weeks ago.

camelburrito commented 8 years ago

This will be on prod along with next Week's release

poscar commented 8 years ago

Great, thank you @camelburrito !!

eike-hass commented 8 years ago

Should the fix have landed by now? We still see that issue

poscar commented 8 years ago

@eike-hass we haven't seen this issue after the fix went live weeks ago.

sebastianbenz commented 8 years ago

@eike-hass which platforms/browsers are affected? Which AMP runtime version are you using?

cramforce commented 8 years ago

@eike-hass Could you make a video of the problem? Seems like this is something else from what @poscar is seeing.

eike-hass commented 8 years ago

https://gfycat.com/HeartfeltSecondaryGermanpinscher this is what i am seeing on https://www.ampproject.org/ with Chrome Version 54.0.2840.71 m and AMP Version 1477334765771

cramforce commented 8 years ago

@eike-hass Wow :)

cramforce commented 8 years ago

@eike-hass Are you seeing this on other pages? I can repro a less dramatic effect, that seems to be related to buggy border-radius render in Chrome (pretty bizarre, compare how the page looks on Safari).

Are you seeing the same problem on other carousels? Just want to make sure this is not just a Chrome bug related to this particular HTML setup.

cramforce commented 8 years ago

@pbakaus for the border-radius Chrome bug. I think we should just remove the radius (it never renders in Chrome anyway), but should likely also file a crbug.com

eike-hass commented 8 years ago

@cramforce i see the same jittering on https://ampbyexample.com/components/amp-carousel/ on the second and third carousel

camelburrito commented 8 years ago

What is the OS ?

eike-hass commented 8 years ago

@camelburrito Windows 10.0.14393

eike-hass commented 8 years ago

@camelburrito could you reproduce the issue?

eike-hass commented 8 years ago

The Issue seems to persist with Chrome 56.0.2915.0 canary and AMP 1478272520861 on Windows 10

jridgewell commented 8 years ago

Ping @camelburrito

camelburrito commented 8 years ago

@eike-hass - i am running around using different machines to reproduce this - no luck yet - ill fire up my old windows laptop and try it out. So far no luck in reproducing -

sebastianbenz commented 8 years ago

@eike-hass you've probably done this, but just to be sure: does it still happen if you disable all Chrome extensions, clear all caches and serviceworkers?

camelburrito commented 8 years ago

I revived my pentium 4 desktop and installed windows7 and latest chrome. I did not see the jitter. But when i inspected the page and then went to mobile mode and then came back to desktop mode, i found a visible jitter, and the jitter is gone when i reload the page.

Other than this i did not see jitters anywhere.

eike-hass commented 8 years ago

I investigated with a different machine and a fresh chrome install. No Jitter. Will report back as soon as i have the "jitter"-machine on my hands again.

eike-hass commented 8 years ago

Back at the jitter machine. The issue persists in fresh canary install. My best guest is that has something to do with it having "retina" resolution. Will try to verify with another device.

eike-hass commented 8 years ago

@camelburrito we can reproduce this issue on other windows 10 "retina" devices like the surface pro.

eike-hass commented 7 years ago

Tested against latest master. Seems to work great!