Skip to content

Make sidebar the highest z-index possible#6711

Merged
camelburrito merged 1 commit intoampproject:masterfrom
camelburrito:sidebar1
Dec 16, 2016
Merged

Make sidebar the highest z-index possible#6711
camelburrito merged 1 commit intoampproject:masterfrom
camelburrito:sidebar1

Conversation

@camelburrito
Copy link
Copy Markdown
Contributor

Fixes #6705

@aghassemi
Copy link
Copy Markdown
Contributor

These conflict with lightbox-viewer. I believe they need to be lower than lightbox-viewer's z-index in case the sidebar opens something like an image in the lightbox-viewer. Please see and update Z_INDEX.md as well.

@camelburrito
Copy link
Copy Markdown
Contributor Author

@aghassemi - sidebar should have the highest zindex on the page. when we open a lbv from a sidebar we need to make sure it closes. isn't the z-index file autogenerated?

@aghassemi
Copy link
Copy Markdown
Contributor

@camelburrito created #6712. Z_INDEX.md is not auto-generated AFAIK.

@camelburrito
Copy link
Copy Markdown
Contributor Author

@aghassemi i think @erwinmombay has a script that generates it

@camelburrito camelburrito merged commit b5da32d into ampproject:master Dec 16, 2016
@jpettitt
Copy link
Copy Markdown
Contributor

Not fixed: see https://www.google.com/amp/cdn.relaymedia.com/amp/www.civilbeat.org/2016/12/ige-hawaiis-food-sustainability-a-priority-in-his-budget-plan/#amph=1 and https://www.google.com/amp/amp.gospelherald.com/articles/68178/20161122/christian-forces-retake-ancient-monastery-from-isis-vow-to-rebuild-we-are-stronger-than-they-imaged.htm#amph=1 for examples of how the sidebar is obscured by the fixed header when in a viewer. We really need to get the !important removed from the sidebar z-index so we can make this choice ourselves.

@camelburrito camelburrito deleted the sidebar1 branch December 16, 2016 22:01
Lith pushed a commit to Lith/amphtml that referenced this pull request Dec 22, 2016
Lith pushed a commit to Lith/amphtml that referenced this pull request Dec 22, 2016
torch2424 pushed a commit to torch2424/amphtml that referenced this pull request Jan 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants