✨Create new extension - AMP-smartlinks #20967
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Gregable
previously requested changes
Feb 20, 2019
|
Thank you @PhilWinchester Have tested locally with compiled file |
|
@zhouyx Yep! I tested with those commands and |
|
@c-nichols I didn't notice that this was previously merged. Apologies for the moving goalposts. As @honeybadgerdontcare mentioned, it might be difficult to make these changes later if there is observed usage by publishers, as we wouldn't want to make a breaking change. I'd lean towards making the one change I recommended as my guess is that the modification is small. However, If you feel comfortable with the risk that you will be stuck with the current syntax, I'm OK with that. Consider my comment advisory. I can't speak for @zhouyx |
Gregable
dismissed
their stale review
My comments can be considered advisory.
|
Thanks @Gregable - we will make that change and then hopefully final approvals will come in quickly ;) |
alabiaga
suggested changes
Feb 25, 2019
zhouyx
suggested changes
Feb 25, 2019
zhouyx
reviewed
Feb 25, 2019
zhouyx
reviewed
Feb 25, 2019
Gregable
approved these changes
Feb 25, 2019
alabiaga
reviewed
Feb 25, 2019
zhouyx
reviewed
Feb 25, 2019
alabiaga
approved these changes
Feb 25, 2019
zhouyx
approved these changes
Feb 25, 2019
|
@PhilWinchester let's pull from master and do another push for this PR to retrigger that failure. |
added 4 commits
February 26, 2019 16:21
* adding example page and amp-smartlinks to bundles.config * creating amp-smartlinks scaffolding * BAM-2584 adding hardcoded POC implementation of amp-smartlinks * adding new constants file and linkmate file/class * adding linkmate call and association working with link-rewrite service * add amp config call and update workflow to use those values * setup initial amp-smartlinks and linkmate workflow * adding new config variables for link attribute and selector * add tests for amp-smartlinks * add test for linkmate.js * add more thorough anchorList check in runSmartlinks * adding code comments, new constants structure and better options validation * update amp-smartlinks to have helpful information * updating global vars and cleaning up main files * clean up tests and add more explicit type assertions * clean up jsdoc tags
…ith userAssert (#4) * BAM-2585 whitelist navigation import and fix type errors * replace user.assert with userAssert
…impression request (#12)
…mes in linkmate-options
PhilWinchester
force-pushed
the
amp-smartlinks-0.1
branch
from
8605851
to
d2e6bec
Compare
|
@alabiaga tests passed 🎆 is this good to merge? |
twifkak
added a commit
to twifkak/amphtml
that referenced
this pull request
Mar 7, 2019
Merged
twifkak
added a commit
that referenced
this pull request
Mar 7, 2019
* cl/235984006 Revision bump for #21124 * cl/236141405 Validating ssr class values and attributes for transformed AMP * cl/236207670 Validating SSR layout should use the SSR CalculateLayout * cl/236240396 Revision bump for #20967 * cl/236242005 Revision bump for #20992 * cl/236357124 Allow `meta name=amp-recaptcha-input` * cl/236374806 Allow `meta name=amp-list-load-more` * cl/236486003 Limit depth of recursion in CSS parsing. * cl/236922121 n/a * cl/236944297 allow i-amphtml-sizer for transformed amp-story * eslint fixes
noranazmy
pushed a commit
to noranazmy/amphtml
that referenced
this pull request
Mar 22, 2019
* creating new amp-smartlinks extension * BAM-2584 AMP-Smartlinks (#3) * adding example page and amp-smartlinks to bundles.config * creating amp-smartlinks scaffolding * BAM-2584 adding hardcoded POC implementation of amp-smartlinks * adding new constants file and linkmate file/class * adding linkmate call and association working with link-rewrite service * add amp config call and update workflow to use those values * setup initial amp-smartlinks and linkmate workflow * adding new config variables for link attribute and selector * add tests for amp-smartlinks * add test for linkmate.js * add more thorough anchorList check in runSmartlinks * adding code comments, new constants structure and better options validation * update amp-smartlinks to have helpful information * updating global vars and cleaning up main files * clean up tests and add more explicit type assertions * clean up jsdoc tags * BAM-2585 Whitelist import, fix type errors, and replace user.assert with userAssert (ampproject#4) * BAM-2585 whitelist navigation import and fix type errors * replace user.assert with userAssert * BAM-2585 fix validator and type check (ampproject#5) * more validator fixes (ampproject#6) * BAM-2585 Fix validator, copyright, and whitespace (ampproject#7) * alphabetize validator, fix whitespace, and add valid tag * update year in copyright statement * BAM-2585 move `link-rewriter` to import statements and updating types (ampproject#8) * BAM-2585 remove link-rewriter and switch to importing from skimlinks extension * clean up promise chain and more descriptive API comments * update xhr to pull from ampdoc.win and add types to constants.js * fix type in buildPageImpressionPayload_ * update validator with new empty value check (ampproject#9) * update validator for exclusive-links * switch page-impression API request to customEventReporter (ampproject#11) * BAM-2585 fix jsdoc in linkmate-options and unnecessary param in page_impression request (ampproject#12) * fix jsdoc for linkmate params and make runLinkmate more readable (ampproject#13) * add try/catch on amp_config fetch and updated constants.js * remove bad type in constants.js and add check for existing shop-links (ampproject#14) * add check for auction_id in mapLinks and add jsdoc for SMARTLINKS_REWRITER_ID * fix type notation in constants.js and linkmate-options.js * fix indentation in example file * add note to README describing link-rewriter priority queue behavior (ampproject#15) * add exception to compile.js for amp-skimlinks * update validator to use empty value as indicator linkmate param * fix validator and linkmate-options to use new config style (ampproject#16) * updating tests for linkmate-options.js * remove redundant userAssert in linkmate-options * update tests to reflect config changes * update tests to send accurate config params * update readme to refelct config changes and more accurate function names in linkmate-options
noranazmy
pushed a commit
to noranazmy/amphtml
that referenced
this pull request
Mar 22, 2019
* cl/235984006 Revision bump for ampproject#21124 * cl/236141405 Validating ssr class values and attributes for transformed AMP * cl/236207670 Validating SSR layout should use the SSR CalculateLayout * cl/236240396 Revision bump for ampproject#20967 * cl/236242005 Revision bump for ampproject#20992 * cl/236357124 Allow `meta name=amp-recaptcha-input` * cl/236374806 Allow `meta name=amp-list-load-more` * cl/236486003 Limit depth of recursion in CSS parsing. * cl/236922121 n/a * cl/236944297 allow i-amphtml-sizer for transformed amp-story * eslint fixes
bramanudom
pushed a commit
to bramanudom/amphtml
that referenced
this pull request
Mar 22, 2019
* creating new amp-smartlinks extension * BAM-2584 AMP-Smartlinks (ampproject#3) * adding example page and amp-smartlinks to bundles.config * creating amp-smartlinks scaffolding * BAM-2584 adding hardcoded POC implementation of amp-smartlinks * adding new constants file and linkmate file/class * adding linkmate call and association working with link-rewrite service * add amp config call and update workflow to use those values * setup initial amp-smartlinks and linkmate workflow * adding new config variables for link attribute and selector * add tests for amp-smartlinks * add test for linkmate.js * add more thorough anchorList check in runSmartlinks * adding code comments, new constants structure and better options validation * update amp-smartlinks to have helpful information * updating global vars and cleaning up main files * clean up tests and add more explicit type assertions * clean up jsdoc tags * BAM-2585 Whitelist import, fix type errors, and replace user.assert with userAssert (ampproject#4) * BAM-2585 whitelist navigation import and fix type errors * replace user.assert with userAssert * BAM-2585 fix validator and type check (ampproject#5) * more validator fixes (ampproject#6) * BAM-2585 Fix validator, copyright, and whitespace (ampproject#7) * alphabetize validator, fix whitespace, and add valid tag * update year in copyright statement * BAM-2585 move `link-rewriter` to import statements and updating types (ampproject#8) * BAM-2585 remove link-rewriter and switch to importing from skimlinks extension * clean up promise chain and more descriptive API comments * update xhr to pull from ampdoc.win and add types to constants.js * fix type in buildPageImpressionPayload_ * update validator with new empty value check (ampproject#9) * update validator for exclusive-links * switch page-impression API request to customEventReporter (ampproject#11) * BAM-2585 fix jsdoc in linkmate-options and unnecessary param in page_impression request (ampproject#12) * fix jsdoc for linkmate params and make runLinkmate more readable (ampproject#13) * add try/catch on amp_config fetch and updated constants.js * remove bad type in constants.js and add check for existing shop-links (ampproject#14) * add check for auction_id in mapLinks and add jsdoc for SMARTLINKS_REWRITER_ID * fix type notation in constants.js and linkmate-options.js * fix indentation in example file * add note to README describing link-rewriter priority queue behavior (ampproject#15) * add exception to compile.js for amp-skimlinks * update validator to use empty value as indicator linkmate param * fix validator and linkmate-options to use new config style (ampproject#16) * updating tests for linkmate-options.js * remove redundant userAssert in linkmate-options * update tests to reflect config changes * update tests to send accurate config params * update readme to refelct config changes and more accurate function names in linkmate-options
bramanudom
pushed a commit
to bramanudom/amphtml
that referenced
this pull request
Mar 22, 2019
* cl/235984006 Revision bump for ampproject#21124 * cl/236141405 Validating ssr class values and attributes for transformed AMP * cl/236207670 Validating SSR layout should use the SSR CalculateLayout * cl/236240396 Revision bump for ampproject#20967 * cl/236242005 Revision bump for ampproject#20992 * cl/236357124 Allow `meta name=amp-recaptcha-input` * cl/236374806 Allow `meta name=amp-list-load-more` * cl/236486003 Limit depth of recursion in CSS parsing. * cl/236922121 n/a * cl/236944297 allow i-amphtml-sizer for transformed amp-story * eslint fixes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #19470
History
This PR was initially merged here (#20494) and then reverted here (#20891) because it broke master due to it's reliance on another extension. The changes in this PR are the same as
20494, with the addition tocompile.jsdescribed here: #20494 (comment)What it does
Create an extension that lets Narrativ offer our publishers our Linkmate service in AMP pages. The extension scans the page for any outbound links the publisher wants targeted and sends them to our API for monetization. Upon API response we use the
link-rewriterservice to map that response to the links in the initial payload. When a user clicks one of the monetized links we swap the href with the monetized link. More about Linkmate here.