Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ [Story bookend] Created amp-story-social-share #33077

Merged
merged 9 commits into from
Mar 16, 2021

Conversation

mszylkowski
Copy link
Contributor

@mszylkowski mszylkowski commented Mar 4, 2021

Contributes to #26957

Part of the deprecation of the bookend is to create a substitute, which will allow creators to specify the share providers in a separate component.

Steps in this PR:

  • Create amp-story-social-share as a validated tag where the configuration can be added.
  • Call loadShareConfig instead of callBookendConfig when deciding what the social share providers will be. This falls back on loadBookendConfig if there is no amp-story-social-share tag present in the story.
  • Validate amp-story-social-share as a valid tag that can have a script inside it or the src attribute.
  • Created validation test for it.
  • Created example story to verify functionality.

image

@amp-owners-bot
Copy link

amp-owners-bot bot commented Mar 4, 2021

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

extensions/amp-story/1.0/amp-story-request-service.js
extensions/amp-story/1.0/amp-story-share.js
extensions/amp-story/1.0/test/test-amp-story-request-service.js
extensions/amp-story/1.0/test/validator-amp-story-amp-experiment-error.out
extensions/amp-story/1.0/test/validator-amp-story-cta-layer-error.out
extensions/amp-story/1.0/test/validator-amp-story-social-share.html
extensions/amp-story/1.0/test/validator-amp-story-social-share.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-cta-layer-error.out
extensions/amp-story/1.0/test/validator-amp-story-social-share.html
extensions/amp-story/1.0/test/validator-amp-story-social-share.out
extensions/amp-story/validator-amp-story.protoascii

@mszylkowski mszylkowski self-assigned this Mar 4, 2021
Copy link
Contributor

@honeybadgerdontcare honeybadgerdontcare left a comment

Choose a reason for hiding this comment

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

LGTM for validator, would like a test as well.

@mszylkowski mszylkowski added this to In progress in wg-stories Sprint via automation Mar 5, 2021
@mszylkowski mszylkowski merged commit d94427b into ampproject:master Mar 16, 2021
wg-stories Sprint automation moved this from In progress to Done Mar 16, 2021
@mszylkowski mszylkowski deleted the ampshare_component branch March 16, 2021 18:22
processprocess pushed a commit to processprocess/amphtml that referenced this pull request Mar 17, 2021
* Created social-share

* Remove empty social share extension

* Fixed validation

* Updated validation

* Updated validation rule to pass test

* Remove comment

* Updated tests

* Added tests
MichaelRybak pushed a commit to MichaelRybak/amphtml that referenced this pull request Mar 18, 2021
@MichaelRybak MichaelRybak mentioned this pull request Mar 18, 2021
MichaelRybak added a commit that referenced this pull request Mar 18, 2021
cl/363456177 Revision bump for #33077
@Gregable authored and @MichaelRybak committed

cl/363067094 Allow link rel=modulepreload in AMP
@honeybadgerdontcare authored and @MichaelRybak committed
@mszylkowski mszylkowski removed this from Done in wg-stories Sprint Apr 12, 2021
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.

None yet

4 participants