[TRAVEL - Home] Bugs intro'd by fixes

ampproject / ampstart

AMP Start source code and templates .

https://ampstart.com/

Apache License 2.0

419 stars 152 forks source link

[TRAVEL - Home] Bugs intro'd by fixes #795

Open spacedino opened 6 years ago

spacedino commented 6 years ago

Can @ericlindley-g , @mattludwig confirm these?

[ ] container line for "email / sign up" is 1 px off. White is extending beyond container. [safari, ios 11.1.2]

[ ] tiny weird white space, exacerbated in landscape view [safari, ios 11.1.2] - its possible this isnt worth fixing - i leave it to @camelburrito to decide. right now it looks way better than it did!!

[ ] hero image extends past the container - is that new or was already like that? OK if it was already like that. i think its a know amp bug.

[ ] Mobile scrolling is buggy / jumpy [safari, ios 11.1.2, portrait] - i actually cant repo upon refresh but just want to triple check

thanks!! otherwise GTG for me. just need @ericlindley-g and @mattludwig to confirm its ready!!!

cc/ @camelburrito

camelburrito commented 6 years ago

@spacedino - did you forget to attach screenshots?

spacedino commented 6 years ago

didnt forget - switching over to mobile to do it :) hold pls.

mattludwig commented 6 years ago

Just checked and I can only reproduce the second issue ("tiny weird white space, exacerbated in landscape view [safari, ios 11.1.2]"). I'm on Chrome, Android, 8.1.0.

88kbbq commented 6 years ago

The horizontal scroll in for the travel template in iOS is a bug. Should not have horizontal panning or scrolling.

spacedino commented 6 years ago

That’s a known bug in a different issue. I don’t thing there is a way to fix it currently.

--

abbybeck@ AMP Design Lead www.ampproject.org / www.ampstart.com / www.ampbyexample.com

ericlindley-g commented 6 years ago

I think we can just drop it into an <amp-carousel type="carousel"> — I thought we had done this before launch across all the templates, but looks like it got missed.

@camelburrito @aghassemi @pbakaus — does anyone have bandwidth to put this horizontally scrolling element into a carousel so it doesn't break in AMP viewer contexts?

camelburrito commented 6 years ago

I can , I thought it was in a carousel too. I’ll take a look

On Wed, Apr 18, 2018 at 11:16 AM ericlindley-g notifications@github.com wrote:

I think we can just drop it into an — I thought we had done this before launch across all the templates, but looks like it got missed.

@camelburrito https://github.com/camelburrito @aghassemi https://github.com/aghassemi @pbakaus https://github.com/pbakaus — does anyone have bandwidth to put this horizontally scrolling element into a carousel so it doesn't break in AMP viewer contexts?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ampproject/ampstart/issues/795#issuecomment-382423589, or mute the thread https://github.com/notifications/unsubscribe-auth/AA_T_7AIsWEoc5OO7bFRbSjrt1b6gnLdks5tp1jjgaJpZM4RJFpo .

-- -- Sriram

moyvera commented 6 years ago

somebody working on a page for the detail of the trips ?

ericlindley-g commented 6 years ago

@moyvera — I'm not aware of anyone working on additional page types for the travel theme (no one on the existing AMP Start team is), though like AMP HTML, AMP Start is open to contributions.