Add support for sticky ad #62
Conversation
rtBot
left a comment
rtBot
left a comment
There was a problem hiding this comment.
phpcs scanning turned up:
🚫 5 errors
hashes-api-scanning skipped
rtBot
left a comment
rtBot
left a comment
There was a problem hiding this comment.
phpcs scanning turned up:
🚫 1 error
hashes-api-scanning skipped
rtBot
left a comment
rtBot
left a comment
There was a problem hiding this comment.
phpcs scanning turned up:
🚫 1 error
hashes-api-scanning skipped
deepaklalwani97
changed the title
classes/class-amp-admanager.php
Outdated
| 'desktop-sizes' => '600x90,728x90', | ||
| 'tablet-sizes' => '320x50,468x60', | ||
| 'mobile-sizes' => '320x50,468x60', |
There was a problem hiding this comment.
sizes should be dynamic instead of static, right? I see the discussion is going on #59
cc: @pradeep910
There was a problem hiding this comment.
Yes sizes should be dynamic, and override with these *-sizes parameters.
rtBot
left a comment
rtBot
left a comment
There was a problem hiding this comment.
phpcs scanning turned up:
🚫 1 error
hashes-api-scanning skipped
template-parts/amp-resources.php
Outdated
|
|
||
| <?php //phpcs:disable ?> | ||
| <script type="text/javascript" src="https://cdn.ampproject.org/v0.js" async></script> | ||
| <script async custom-element="amp-sticky-ad" src="https://cdn.ampproject.org/v0/amp-sticky-ad-1.0.js"></script> |
There was a problem hiding this comment.
There should be a check here if sticky ad is enabled, then only include this script.
rtBot
left a comment
rtBot
left a comment
There was a problem hiding this comment.
phpcs scanning turned up:
🚫 1 error
hashes-api-scanning skipped
|
Addressed feedbacks |
deepaklalwani97
changed the title
rtBot
left a comment
rtBot
left a comment
There was a problem hiding this comment.
phpcs scanning turned up:
🚫 6 errors
hashes-api-scanning skipped
deepaklalwani97
changed the title
Fixes: #59