Laterpay validator config #6974
Conversation
| tag_name: "SCRIPT" | ||
| spec_name: "amp-access-laterpay extension .js script" | ||
| mandatory_parent: "HEAD" | ||
| unique_warning: true |
There was a problem hiding this comment.
Since this is new, this shouldn't be a warning. Set as unique: true.
| spec_name: "amp-access-laterpay extension .js script" | ||
| mandatory_parent: "HEAD" | ||
| unique_warning: true | ||
| also_requires_tag: "amp-access extension .js script" |
There was a problem hiding this comment.
The markup from #5920 mentions amp-analytics as also being a required script. If so then we should add another also_requires_tag here.
also_requires_tag: "amp-analytics extension .js script"
There was a problem hiding this comment.
Well. The dependency chain is amp-access-layerpay -> amp-access -> amp-analytics. Should I expand transitive dependencies here? Also, we're trying to remove amp-access -> amp-analytics dependency.
| @@ -0,0 +1,71 @@ | |||
| <!-- | |||
There was a problem hiding this comment.
I added couple things, such as amp-access-laterpay script and vendor config. There's not much in terms of requirements here. Also, I imagine we may add a couple more things. One thing, for instance, we could add id=amp-access-laterpay-dialog requirement. Do you think it'd be good to add it in validator?
On the other hand, laterpay vendor config might be hard to enforce since we don't have JSON rules, do we?
There was a problem hiding this comment.
Sorry for not responding earlier. We currently don't have a mechanism to enforce an attribute across all tags given if another tag exists. It would be possible if the attribute was limited to a single or small set of elements.
| error_message: "contents" | ||
| } | ||
| } | ||
| spec_url: "https://www.ampproject.org/docs/reference/extended/amp-access-laterpay.html" |
There was a problem hiding this comment.
This url 404s. When ampproject.org doesn't have an appropriate page we tend to link directly to the github markup that would have similar content to what shows up eventually on ampproject.org. In this instance that should end up being https://github.com/ampproject/amphtml/blob/master/extensions/amp-access-laterpay/amp-access-laterpay.md
There was a problem hiding this comment.
We don't have that one yet either :) Will be added by #5920 and should be auto-deployed to ampproject.org. I switched to github url for now, since it will be available sooner.
|
@honeybadgerdontcare PTAL. |
* master: (310 commits) Update csa.md to remove non-required parameters (ampproject#6902) Add notes about requesting ads ATF and link to demo (ampproject#7037) Remove whitelist for lightbox scrollable validator (ampproject#7034) Delegate submit events until amp-form is loaded (ampproject#6929) Moves closure sha384 into a new extension amp-crypto-polyfill for lazy load (ampproject#7006) Refactor observables in viewer-impl into a map object (ampproject#7004) resizing of margins (ampproject#6824) Use URL replacer from embed for pixel (ampproject#7029) adds support for Gemius analytics (ampproject#6558) Avoid duplicating server-layout (ampproject#7021) Laterpay validator config (ampproject#6974) Validator rollup (ampproject#7023) skeleton for amp-tabs (ampproject#7003) Upgrade post-css and related packages to latest (ampproject#7020) handle unload (ampproject#7001) viewer-integr.js -> amp-viewer-integration (ampproject#6989) dev().info()->dev().fine() (ampproject#7017) Turned on experiment flag (ampproject#6774) Unlaunch ios-embed-wrapper for iOS8 to avoid scroll freezing issues (ampproject#7018) Add some A4A ad request parameters (ampproject#6643) ...
* Laterpay validator config * test files * review fixes
* Laterpay validator config * test files * review fixes
/cc @trodrigues