ampproject / amp.dev

The AMP Project Website.
https://amp.dev
Other
582 stars 694 forks source link

New homepage / about page nits #5854

Open sebastianbenz opened 3 years ago

sebastianbenz commented 3 years ago
Screenshot 2021-05-17 at 12 36 08
sebastianbenz commented 3 years ago

Added another one

lluerich commented 3 years ago

As for the carousel arrows, since they are custom, I don't think there is a way of disabling/hiding them once the end of the carousel is reached, is there?

sebastianbenz commented 3 years ago

Is this possible via amp-bind? We know how many elements there are in the list and what the current index is.

lluerich commented 3 years ago

Oh yeah, that would be a way of doing it. I was trying to solve it with amp-base-carousel functions. I will look into that.

lluerich commented 3 years ago

Is this possible via amp-bind? We know how many elements there are in the list and what the current index is.

So far, I couldn't find a proper way to achieve this. Problem is that slideChange is a low trust action and thus it cannot update AMP state with the current index.

lluerich commented 3 years ago

Still no way to hide next/prev arrow. We are using controls that are outside of the amp-base-carousel component so that's why the regular mechanism does not seem to work. I don't think this is a bug in the amp-base-carousel. On the other hand I could not find a way to position the arrows outside of the carousel using the standard ones as the container has position: relative; and overflow: hidden;.

Any other ideas? 😄

DerekNonGeneric commented 3 years ago

Looks like this is the explanation for the bug I just opened (https://github.com/ampproject/amp.dev/issues/6133) regarding the carousel buttons.

I was under the impression that this component was an ordinary amp-carousel when I volunteered to fix it.

Seems like we are going to have to put on wetsuite and jump in. :)