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

[AMP] Multiple style amp-custom elements per page #1116

Closed
kiransg89 opened this issue Jul 22, 2020 · 5 comments
Closed

[AMP] Multiple style amp-custom elements per page #1116

kiransg89 opened this issue Jul 22, 2020 · 5 comments
Labels
bug Unexpected problem or unintended behavior that impairs normal functioning of the product.
Milestone

Comments

@kiransg89
Copy link

Bug Report

Current Behavior
With AMP selector none of the example site are working as expected

Expected behavior/code
Should render AMP page

Environment
AEM 6.4

Possible Solution

Additional context / Screenshots
Add any other context about the problem here. If applicable, add screenshots to help explain.

@msagolj
Copy link
Contributor

msagolj commented Jul 23, 2020

@kiransg89 this a very vague description of your issue :) Please add more context: exact AEM version, Core Comp Version, what errors do you observe, reproducible steps ?

The AMP extension has been tested with example project on 6.4.8.1 and 6.5.5, see also working example page at:
https://www.aemcomponents.dev/content/core-components-examples/library/page-authoring/text.amp.html

@kiransg89
Copy link
Author

kiransg89 commented Jul 23, 2020

Hi @msagolj ,
I am currently working on AEM 6.4.8 and I am using core components version 2.11.0 (Recent release).
I am seeing following issue in AMP for image page:
image
Is their someway can we call global clienlibs by calling

<style amp-custom data-sly-use.clientlibs="${'com.adobe.cq.wcm.core.components.models.ClientLibraries' @ resourceTypes = page.componentsResourceTypes, filter='.*\\.amp', _### **categories='globals'**_}"> ${clientlibs.cssInline @ context="unsafe"} </style>

If I do the above then component level clientlibs are not getting picked up

@vladbailescu vladbailescu added the bug Unexpected problem or unintended behavior that impairs normal functioning of the product. label Jul 24, 2020
@vladbailescu vladbailescu added this to the 2.12.0 milestone Jul 24, 2020
@vladbailescu
Copy link
Member

Indeed, as per https://amp.dev/documentation/guides-and-tutorials/develop/style_and_layout/:

Define CSS within the <style amp-custom> tag inside the head of the document. There is only one <style amp-custom> tag allowed on each AMP page.

@kiransg89
Copy link
Author

Hi @vladbailescu ,

I did some work around,
like created the clientlib(amp) under page component and embedded all the global clientlibs:
image

@vladbailescu
Copy link
Member

@kiransg89 , that's nice but we will still need to fix our library to be compliant to the spec. Thanks for discovering the issue.

@gabrielwalt gabrielwalt changed the title AMP is not working [AMP] Multiple style amp-custom elements per page Aug 31, 2020
@msagolj msagolj modified the milestones: 2.12.0, 2.13.0 Oct 26, 2020
@msagolj msagolj modified the milestones: 2.13.0, 2.14 Dec 1, 2020
vladbailescu added a commit that referenced this issue Jan 13, 2021
* Switched from loading the base CSS via a new <style amp-custom> tag to embedding it in the page's AMP CSS
vladbailescu added a commit that referenced this issue Jan 15, 2021
* Switched from loading the base CSS via a new <style amp-custom> tag to embedding it in the page's AMP CSS

Fixes #1116
@vladbailescu vladbailescu modified the milestones: 2.14.0, 2.13.4 Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected problem or unintended behavior that impairs normal functioning of the product.
Projects
None yet
Development

No branches or pull requests

3 participants