Skip to content

✨ [amp-story] [amp-story-page-outlink] Element child#34532

Merged
processprocess merged 45 commits intoampproject:mainfrom
processprocess:outlink-cta
Jun 2, 2021
Merged

✨ [amp-story] [amp-story-page-outlink] Element child#34532
processprocess merged 45 commits intoampproject:mainfrom
processprocess:outlink-cta

Conversation

@processprocess
Copy link
Copy Markdown
Contributor

@processprocess processprocess commented May 25, 2021

Context / Fixes #34507

demo
Paste AMP.toggleExperiment('amp-story-page-attachment-ui-v2', true) into the console to activate the experiment.

Updates amp-story-page-outlink to use an anchor child instead of an href attribute.
Outlink navigation is triggered by a programatic click on this tag to be fully compliant with all analytics or other tracking mechanisms.

  • Adds anchor child to amp-story-page-outlink.
  • Removes cta-text from validation in favor of a's text content.
  • Removes title from validation in favor of child a's title attribute.
  • Removes href from validation in favor of child a's href attribute.
  • Updates REMOTE to OUTLINK in AttachmentType enum to keep semantics consistent.
  • Removes new outlink feature validation from amp-story-page-attachment[href].
  • Updates validation.
  • Updates visual test template.
  • Updates example template.
  • Updates comments.
  • Adds visual test for legacy amp-story-page-attachment with href.

@processprocess processprocess marked this pull request as ready for review May 26, 2021 16:25
@processprocess processprocess requested a review from gmajoulet May 26, 2021 16:25
@amp-owners-bot amp-owners-bot bot requested a review from mszylkowski May 26, 2021 16:25
@amp-owners-bot
Copy link
Copy Markdown

amp-owners-bot bot commented May 26, 2021

Hey @gmajoulet, @newmuis, @Enriqe! These files were changed:

extensions/amp-story/1.0/amp-story-open-page-attachment.js
extensions/amp-story/1.0/amp-story-page-attachment.css
extensions/amp-story/1.0/amp-story-page-attachment.js
extensions/amp-story/1.0/test/test-amp-story-page.js
extensions/amp-story/1.0/test/validator-amp-story-amp-experiment-error.out
extensions/amp-story/1.0/test/validator-amp-story-amp-twitter-error.out
extensions/amp-story/1.0/test/validator-amp-story-page-attachment.html
extensions/amp-story/1.0/test/validator-amp-story-page-attachment.out
extensions/amp-story/validator-amp-story.protoascii

Hey @ampproject/wg-caching! These files were changed:

extensions/amp-story/1.0/test/validator-amp-story-amp-experiment-error.out
extensions/amp-story/1.0/test/validator-amp-story-amp-twitter-error.out
extensions/amp-story/1.0/test/validator-amp-story-page-attachment.html
extensions/amp-story/1.0/test/validator-amp-story-page-attachment.out
extensions/amp-story/validator-amp-story.protoascii

Copy link
Copy Markdown
Member

@Gregable Gregable left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review for validator changes only. Does not imply that the semantics of the change are OK.

@processprocess processprocess requested a review from Gregable May 26, 2021 17:14
@processprocess processprocess merged commit 195fb51 into ampproject:main Jun 2, 2021
@processprocess processprocess deleted the outlink-cta branch June 2, 2021 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[amp-story] [amp-story-page-outlink] Child anchor element

4 participants