Skip to content

Commit

Permalink
Fix issue where amp-img are not rendered inside amp-list in story pag…
Browse files Browse the repository at this point in the history
…e attachment (#39806)
  • Loading branch information
ychsieh authored Feb 13, 2024
1 parent da152a1 commit f49932e
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 0 deletions.
23 changes: 23 additions & 0 deletions examples/amp-story/attachment.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
<script async custom-element="amp-form" src="https://cdn.ampproject.org/v0/amp-form-0.1.js"></script>
<script async custom-element="amp-story" src="https://cdn.ampproject.org/v0/amp-story-1.0.js"></script>
<script async custom-element="amp-story-page-attachment" src="https://cdn.ampproject.org/v0/amp-story-page-attachment-0.1.js"></script>
<script async custom-element="amp-list" src="https://cdn.ampproject.org/v0/amp-list-0.1.js"></script>
<script async custom-template="amp-mustache" src="https://cdn.ampproject.org/v0/amp-mustache-latest.js"></script>
<link href='https://fonts.googleapis.com/css?family=Georgia|Open+Sans|Roboto' rel='stylesheet' type='text/css'>
<link href="https://fonts.googleapis.com/css?family=Roboto+Condensed" rel="stylesheet">
<title>Attachments</title>
Expand Down Expand Up @@ -232,6 +234,27 @@
</amp-story-page-attachment>
</amp-story-page>

<amp-story-page id="amp-list-of-amp-img">
<amp-story-grid-layer template="fill">
<amp-img layout="fill" src="/examples/visual-tests/amp-story/img/cat1.jpg"></amp-img>
</amp-story-grid-layer>
<amp-story-grid-layer template="vertical">
<p>
<span data-text-background-color="#00A3DB">amp-story-page-attachment: amp-list of amp-img.</span></br></br>
</p>
</amp-story-grid-layer>
<amp-story-page-attachment
title="Lorem ipsum"
layout="nodisplay"
>
<amp-list src="https://afdsi.com/164189302300000061/data.json" width="auto" height="100" layout="fixed-height">
<template type="amp-mustache">
<amp-img src="{{imageUrl}}" width="160" height="160"></amp-img>
</template>
</amp-list>
</amp-story-page-attachment>
</amp-story-page>

<amp-story-page id="outlink-default">
<amp-story-grid-layer template="fill">
<amp-img layout="fill" src="/examples/visual-tests/amp-story/img/cat2.jpg"></amp-img>
Expand Down
10 changes: 10 additions & 0 deletions extensions/amp-list/0.1/amp-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -1147,6 +1147,16 @@ export class AmpList extends AMP.BaseElement {
this.addElementsToContainer_(elements, container);
}

// For amp-list embedded in the amp-story-page-attachment, because the attachment is initially in
// nodisplay mode, the layoutCallback of all amp-img elements are not called at all, even after
// the attachment is opened and amp-list fetches results and inserted built amp-img(from templates)
// into DOM. Force call layoutCallback to render images.
if (this.element.closest('amp-story-page-attachment')) {
this.container_.querySelectorAll('amp-img').forEach((element) => {
element.getImpl().then((impl) => impl.layoutCallback());
});
}

const event = createCustomEvent(
this.win,
AmpEvents_Enum.DOM_UPDATE,
Expand Down

1 comment on commit f49932e

@jaygray0919
Copy link

Choose a reason for hiding this comment

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

Thank you

Please sign in to comment.