-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
amp-sidebar should add class to body #3106
Comments
@Massa84 - we did consider that initially , this version of sidebar was built mainly for the mobile web. We did consider adding a state class to the body and then later rule it out due to implementation complexities and also to avoid excessive spamming of classes on the body. However , we would definitely consider implementing this as a feature (may be like - adding an attribute would trigger this and hopefully will be configurable) CC - @ericlindley-g - to help prioritize this into a milestone as appropriate. |
@Massa84 I've spun off a feature request for the specific animation behavior you describe in #3170—I've made some assumptions about the behavior, so feel free to correct or elaborate on that issue. @sriramkrish85 — closing the request in this issue to add state classes to the body, due to the concerns you mention above. Please correct/re-open if this is possible without those risks, somehow |
For our website we would like to 'slide' the content of the article to the right simultaneously with the sidebar opening animation, but, to do so, it would be extremely useful if the amp-sidebar adds some configurable class to the body.
Otherwise we should rely on possibly complicated sibling selectors to achieve the same result but would make everyone life a bit harder.
Would it be possible to add this feature ?
The text was updated successfully, but these errors were encountered: