Skip to content

🐛 [Story attachments] Fix attachment demos#37331

Closed
mszylkowski wants to merge 8 commits intoampproject:mainfrom
mszylkowski:fixAttachmentDemos
Closed

🐛 [Story attachments] Fix attachment demos#37331
mszylkowski wants to merge 8 commits intoampproject:mainfrom
mszylkowski:fixAttachmentDemos

Conversation

@mszylkowski
Copy link
Copy Markdown
Contributor

When deploying a demo, we need to manually import the page attachment into the doc because the installExtensionForDoc function generates a url for the extension that doesn't exist on firebase. However, page-attachment assumed that the extension was installed after amp-story, so it fetched the services synchronously.

If we fetch the services asynchronously, we can build the page attachment only after amp-story is built.

@amp-owners-bot
Copy link
Copy Markdown

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

extensions/amp-story-page-attachment/0.1/amp-story-draggable-drawer.js
extensions/amp-story-page-attachment/0.1/amp-story-page-attachment.js

@processprocess
Copy link
Copy Markdown
Contributor

Nice. I tested this on firebase, the attachment is opening but it's not styled and is throwing errors.
Demo

Screen Shot 2022-01-11 at 10 05 49 AM

Screen Shot 2022-01-11 at 10 08 48 AM

It would also be good to add the import to the other example files using amp-story-page-attachment.
Should it also be added in the /visual-tests documents?

@mszylkowski
Copy link
Copy Markdown
Contributor Author

Closing in favor of #37335

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants