Lullabot / amp-library

Convert HTML to AMP HTML and report HTML compliance with the AMP HTML specification
Other
382 stars 182 forks source link

amp-sticky-ad is being removed #218

Open karltud123 opened 6 years ago

karltud123 commented 6 years ago

Hi,

I wondering why amp-sticky-ad is being removed on my page.

Here is the log i'm getting.

image

Basically, im adding the script on the head as it is not added automatically, then i'm adding the amp-sticky-ad at the body content

`

`

renzit commented 6 years ago

i fixed it in a pull request, the changes are accepted but wasnt merged yet. Heres is the link: https://github.com/Lullabot/amp-library/pull/196

I Dont know when would be a new tag release or if these would be merged soon, meanwhile you can make a patch for this in the composer, you need:

"cweagans/composer-patches": "^1.6"

And

 "extra": {
                "patches":{
                    "lullabot/amp": {
                        "Fix for amp-sticky": "https://github.com/Lullabot/amp-library/commit/4d49f5884eb180ee356e47b016e1d7158b973651.patch"
                    }
                }
            }
karltud123 commented 6 years ago

Hi,

Will this work also in Drupal 7?

renzit commented 6 years ago

Yes! all of this is implemented on drupal 7. Take a look: https://www.vix.com/pt/comportamento/556372/estudo-descobriu-o-salario-ideal-para-ser-feliz-concorda?amp In this site, for now, all the amp are articles converted with this library, so everithyng you see there could be done whit this repo and some little fixes and maintainence.

karltud123 commented 6 years ago

Great! I asked because I tried it and still gives me the same issue... will check more..

maybe you have any ideas?

renzit commented 6 years ago

mmm, here are the validations, for sure it need a child AMP-AD: https://github.com/ampproject/amphtml/blob/master/extensions/amp-sticky-ad/validator-amp-sticky-ad.protoascii

show your implementation!