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

Datawrapper iframe #202

Open
CXAdi opened this issue Jun 8, 2020 · 10 comments
Open

Datawrapper iframe #202

CXAdi opened this issue Jun 8, 2020 · 10 comments
Assignees
Labels

Comments

@CXAdi
Copy link

CXAdi commented Jun 8, 2020

This Github issue is synchronized with Zendesk:

Ticket ID: #8957
Priority: normal
Group: Support
Requester: [email protected]
Organization: Vikatan
Assignee: Rachana TR
Issue escalated by: Rachana TR
Assigned Team: backend
Bug Related To: open_issue

Original ticket description:

Hi Team, adding below datawrapper iframe AMP is getting failed. Pls advise.

 

</script>

 

Regards,

Ismail Anees

@StInGDeaDlY
Copy link

@sramalin @Rachana-t Could you please add steps to reproduce?

@rashmirmittal
Copy link

@CXAdi Is this even Platform ticket? Or for the AMP team? HAve you guys done any analysis on this? There is no information in the ticket.

@sinimerlin
Copy link

@sramalin Please add in the details why this needs to be added from Platform

@sramalin
Copy link
Collaborator

sramalin commented Jun 9, 2020

@sinimerlin @Rachana-t @SpandanaPatnam @StInGDeaDlY @rashmirmittal
Please find my comments below

  • When a ' Data wrapper ' is provided as a JS embed in a story, sketches mark this story with " is-amp-supported": false". This is an existing behavior and not sure the reason for the same. (Refer this BQ story

  • BQ is using a custom AMP ignores platform's "is-amp-supported" flag and renders Data wrapper embedded strories,

  • However AMP lib (Vikatan is the first publisher with AMP powered by AMP lib) is currently depending Platform's "is-amp-supported" flag .

@rashmirmittal @StInGDeaDlY @sharangj We have two options

  1. Implement "is-amp-support" logic in AMP lib and continue enhancing it. We could fix this issue as part of this activity

  2. Continue depending on Platform's flag and provide override options in AMP to skip this flag.

Thoughts?

@StInGDeaDlY
Copy link

Spoke to @sramalin . This will be handled in AMP library which in the future will not be looking at platform's is-amp-supported flag. Platform will be deprecating it going forward.

@StInGDeaDlY
Copy link

@sinimerlin
This ticket will be handled by AMP library team.

@sinimerlin sinimerlin transferred this issue from another repository Jun 17, 2020
@sinimerlin
Copy link

Transferring issue to Amp-library

@sinimerlin
Copy link

@sramalin Please provide an update on this

@SpandanaPatnam
Copy link

@sramalin Please help us with an update on this

@sramalin
Copy link
Collaborator

@SpandanaPatnam We are going with Option 1. This is already planned for this quarter (July - Sep)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants