Amphtml: Remove requirement that amp-sidebar be the direct child of the body
Describe the new feature or change to an existing feature you'd like to see
Some more context on this Issue: https://github.com/ampproject/amp-by-example/issues/16
There has been a requirement for years that amp-sidebar must be the direct child of the body component. This seems to be because of bugs in iOS safari, but as it's been a couple of years, I would like this assumption to be revisited.
It's not that I want to put the amp-sidebar willy nilly anywhere in the page -- it's that sometimes I don't have choice and cannot place it as a child of the body. If this could be loosened, it would make the component easier to use.
What's more, your docs contain an example of the sidebar component being contained within a div -- which is incorrect.
Please see the comments at the above issue for more context and an example repo where you can see the problem described.
Thanks! cc @andrewwatterson (he mentioned at AMP conf that it was okay to tag him 馃槵 )
maddoxnelson
All 2 comments
Thanks @maddoxnelson for the FR. I agree the restriction here is a bit too much, and I am up for relaxing it.
It is worth mentioning the primary reasons why this restriction was added however:
1- Accessibility. It enforces the sidebar to be almost in the right DOM order. Putting the sidebar DOM in a random place will result in same visual representation but very different location for screen readers. Arguably this is not a strong enforcement since someone can still put sidebar as the last child of body which is still bad.
2- Avoiding unexpected z-index issues. For example in am-sidebar ends up in a div that creates its own stacking context, it can break amp-sidebar. That being said, same is true for amp-lightbox but we did not restrict it that way. So not a strong argument here.
Both of the reasons above add some value but I agree it is not enough to justify the drawbacks. So let's remove the restriction!
aghassemi
on 18 Apr 2019
I solemnly swear to take accessibility into account when using this component, good point!
maddoxnelson
on 30 Apr 2019
Related issues
sebastianbenz
路
48Comments
lisotton
路
52Comments
choumx
路
50Comments
zhouyx
路
103Comments
ericlindley-g
路
60Comments
Most helpful comment
Thanks @maddoxnelson for the FR. I agree the restriction here is a bit too much, and I am up for relaxing it.
It is worth mentioning the primary reasons why this restriction was added however:
1- Accessibility. It enforces the sidebar to be almost in the right DOM order. Putting the sidebar DOM in a random place will result in same
visualrepresentation but very different location for screen readers. Arguably this is not a strong enforcement since someone can still put sidebar as the last child of body which is still bad.2- Avoiding unexpected z-index issues. For example in
am-sidebarends up in adivthat creates its own stacking context, it can breakamp-sidebar. That being said, same is true foramp-lightboxbut we did not restrict it that way. So not a strong argument here.Both of the reasons above add some value but I agree it is not enough to justify the drawbacks. So let's remove the restriction!