Amphtml: Sidebar is AutoScrolling Page when Toggled

[randallbpotter15]

When applying a top margin to the amp-sidebar list in order to preserver our head branding and links we noticed upon toggling the sidebar to the active state that the page is scrolling down thereby cutting off part of our header, brand and links.

Please see video here: http://www.screencast.com/t/7FeIuFBim6z

Desired behavior is that toggle state does nothing but show and hide sidebar without page movement. Taking off the margin I added to the menu list again, makes it to where the menu hides the branding and other options available to the user in the header.

This is happening in the mobile simulator and phone devices.

Thank you,
-R
@cramforce @jmadler

[rudygalfi]

/to @ericlindley-g

[jridgewell]

You're likely not updating the height style, then. Since you've added some X pixels to the margin-top, and the default height is 100vh, you need to subtract the margin-top from that:

```css
sidebar {
margin-top: 200px;
height: calc(100vh - 200px);
}

[ericlindley-g]

@randallbpotter15 If the fix @jridgewell suggests doesn't work, you can send over a link and we can take a look to determine if it's an AMP bug or implementation bug. Thanks!

[randallbpotter15]

@ericlindley-g and @jridgewell : The fix didn't work in this case. It moved the nav menu further down within it's context of the sidebar. I've uploaded a video with the safari simulator debug combo to illustrate the styles and behavior with the proposed fix. I'll see if I can get this on an environment you all can access. Let me know if you see something in the video though.

Edit: 12/01/2016: 9:11AM EST: Better video: http://www.screencast.com/t/7AwJKWdO9z

Thank you,
-Randall

[jridgewell]

You're putting the height on the wrong element. From your screencast, you've put it on <ul>, but that won't affect the sidebar itself.

You have a .nav-menu class on the <amp-sidebar> with:

.nav-menu {
  padding: 15px 0px 1px 10px;
  margin-top: 25px;
}

You need to add a height style to that class to fix.

[randallbpotter15]

That worked - thank you!

[randallbpotter15]

Reared it's head again once in the prod carousel.
http://www.screencast.com/t/AccW0XhdynH3

can replicate by googling "cnn" on mobile.

is there a way to simulate this prod carousel so we can catch these things before prod?

[cramforce]

@randallbpotter15 Yes, you can opt into "Dev Channel" here https://cdn.ampproject.org/experiments.html to get all changes one week early.

[randallbpotter15]

Thanks @cramforce - does that apply to the actual amp search carousel viewer?
In the screencast above the top amp bar with the bullets that lets you move from result to result?

One of my goals here is to be able to view what my page will look like in that container before I release to prod. (besides getting a prod ready fix)

Has this code in it:
<g-fixed-header id="amp-hdr" class="_Gog fp-w _vmh _tlo"><div class="_fpg"><span class="_hpg _Ewo _wtf _Dtf" role="button" tabindex="0" jsaction="ampfp.cl" aria-label="close" data-ved="0ahUKEwi30Ma2xefQAhWBMyYKHZkNBhcQ8owBCAI"><svg focusable="false" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24"><path d="M19 6.41L17.59 5 12 10.59 6.41 5 5 6.41 10.59 12 5 17.59 6.41 19 12 13.41 17.59 19 19 17.59 13.41 12z"></path></svg></span><span class="_gpg amp-ttlctr amp-wibbl" role="heading"><span class="amp-ttltxt">amp.cnn.com</span>

[jpettitt]

+1 for a search carousel simulator we can drop pages into - that would be really nice.

[randallbpotter15]

Also for reference here is the same code in our pre-prod environment - working with the original fix from this ticket.
http://www.screencast.com/t/Ay4a5nm03e

[cramforce]

I can bring up such a demo viewer. It unfortunately comes with a significant security dimension, making it not all that easy.

[randallbpotter15]

@cramforce I'm down with it even if it has large security.

[ericlindley-g]

Reopening and passing to @camelburrito for the regression (specifically in the prod viewer)

[randallbpotter15]

Thank you @ericlindley-g :) I'm still seeing interplay between the carousel's injected header and the top of the actual page content. Scrolling doesn't seem to be happening - but it's hard to tell what to adjust top distances based on how the carousel is injected and can change. (hence the ask for the test harness up above)

[camelburrito]

I think there are multiple things being discussed here. Can someone clarify what is the fix we are looking for here.

[jpettitt]

BTW figured out how to force a viewer -http://www.google.com/amp/<amp article domain>/<amp article path> does the trick, but only on mobile or in mobile emulation. Not sure if this is a bug or a feature but I like it.

Ex https://www.google.com/amp/dailydot.relaymedia.com/amp/entertainment/only-in-hella-webseries-interview

[jridgewell]

BTW figured out how to force a viewer -http://www.google.com/amp//

Yup, that'll get you into a viewer context. It's a little different than what happens on a normal search, though, since there's no left/right swipe to next article.

but only on mobile or in mobile emulation. Not sure if this is a bug or a feature but I like it.

The forwarding is on purpose for shares that end up being clicked on Desktop.

[randallbpotter15]

@jridgewell By "a little different" - does that mean it's code is different or just that it's a single view and not a true swiping carousel?

@cramforce Were there any more thoughts around a demo viewer? Or could you elaborate on the security dimension you mentioned?

@camelburrito Did you get the information you needed for regression?

[camelburrito]

@randallbpotter15
"Sidebar is AutoScrolling Page when Toggled " --> is this the issue you are seeing?

[cramforce]

@randallbpotter15 Does https://search.google.com/search-console/amp work for you (it has a built-in viewer preview). It currently doesn't work for non-indexable content. I'm trying to get that fixed.

[ryanpoplin]

Hey Team,

(Disclaimer: I'm working with @randallbpotter15 and I'd like to continue on this thread despite the topic title conflict because of the Google AMP Viewer conversations mentioned here. Let me know if there's a more appropriate issue thread available.)

Currently we're having an issue for example where our navigation icon renders correctly during local testing (no CSS barking from the Google AMP validator).

However, when the content is indexed and can be viewed officially within the Google AMP Viewer, this icon fails to render 9 times out of 10 (no AMP validator issues).

In general, this leaves us to conclude that our CSS is most likely being conflicted somewhere along the way within the Google AMP viewer. How are we to locally test within the official AMP Viewer?

Thanks!

[camelburrito]

@ryanpoplin - could you try and see it in the web-inspector on chrome or safari (or paste a search link - i can debug for you) - also i think you should open a new issue for this to gain more/right traction on this issue.

[ryanpoplin]

https://www.google.com/amp/s/amp.cnn.com/cnn/2016/07/15/world/live-blog-turkey/index.html

The element 'nav-menu__hamburger' is the element that's not appearing in the AMP Viewer, but is visible during local testing.

For any Devs running across what I have mentioned in the previous comments, I've opened an issues here: #7066.

[camelburrito]

@ryanpoplin - does this mean we can close this issue?

[jridgewell]

The element 'nav-menu__hamburger' is the element that's not appearing in the AMP Viewer, but is visible during local testing.

This is because your using a amp-sidebar ~ nav CSS selector, and the amp-sidebar is being reparented to avoid a position: fixed bug.

[adelinamart]

@ryanpoplin seems the issue here was answered. If there are more open questions, feel free to reopen. Thanks!