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

Update the TikTok embed handler to use amp-tiktok component #6060

Closed
pierlon opened this issue Apr 8, 2021 · 7 comments · Fixed by #6558
Closed

Update the TikTok embed handler to use amp-tiktok component #6060

pierlon opened this issue Apr 8, 2021 · 7 comments · Fixed by #6558
Assignees
Labels
Embeds Enhancement New feature or improvement of an existing one WS:Core Work stream for Plugin core
Milestone

Comments

@pierlon
Copy link
Contributor

pierlon commented Apr 8, 2021

Feature description

Now that ampproject/amphtml#32651 has been merged we can begin the process of updating AMP_TikTok_Embed_Handler to use the amp-tiktok component instead of amp-embedly-card.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • Inserting a TikTok embed should result in the amp-tiktok component being used on the AMP page.

Implementation brief

  • Update the AMP_TikTok_Embed_Handler to use the amp-tiktok component rather than the amp-embedly-card one.

QA testing instructions

Demo

Changelog entry

@pierlon pierlon added Enhancement New feature or improvement of an existing one Embeds WS:Core Work stream for Plugin core labels Apr 8, 2021
@westonruter westonruter added this to the v2.1 milestone Apr 8, 2021
@westonruter
Copy link
Member

Looks like we're still waiting on ampproject/amphtml#32219 to be closed. The validator spec hasn't yet been updated.

@westonruter westonruter modified the milestones: v2.1, v2.1.1 Apr 27, 2021
@westonruter westonruter modified the milestones: v2.1.1, v2.1.2 May 7, 2021
@westonruter westonruter modified the milestones: v2.1.2, v2.2 May 11, 2021
@westonruter
Copy link
Member

This can now be worked on as a PR off of #6436.

@westonruter
Copy link
Member

This can now be worked on as a PR off of #6436.

The PR has been merged into develop. This work can proceed.

@westonruter westonruter self-assigned this Aug 24, 2021
@westonruter westonruter modified the milestones: v2.2, v2.1.4 Aug 24, 2021
@westonruter westonruter removed their assignment Aug 25, 2021
@delawski delawski self-assigned this Aug 27, 2021
@delawski
Copy link
Collaborator

QA Passed

I've pasted a TikTok link into a post that got automatically converted into an embed. I've then visited the AMP version of the post and confirmed that - as expected - the amp-tiktok element has been used to render the embed:

Screenshot 2021-08-27 at 15 55 25

Tested against Version 2.2.0-alpha-20210825T212353Z-ab5a1e7

@westonruter
Copy link
Member

(To re-check on 2.1.4)

@westonruter
Copy link
Member

I've fixed the 2.1 build errors via #6574. That should allow a new 2.1 branch build to be created, although there is a JS unit test error still. See #6235 (comment).

@delawski
Copy link
Collaborator

Confirmed to be working as expected on 2.1.4-alpha:

Screenshot 2021-08-30 at 18 03 58

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Embeds Enhancement New feature or improvement of an existing one WS:Core Work stream for Plugin core
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants