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

✨ Bento amp-gist #37333

Merged
merged 12 commits into from
Feb 14, 2022
Merged

✨ Bento amp-gist #37333

merged 12 commits into from
Feb 14, 2022

Conversation

deepaklalwani97
Copy link
Contributor

This PR adds a bento component for `amp-gist.

  • Add amp-gist
  • Add types
  • Add type annotations
  • Add test case
  • Update Storybook
  • prettify new amp-gist

@deepaklalwani97 deepaklalwani97 marked this pull request as ready for review January 13, 2022 08:55
@amp-owners-bot amp-owners-bot bot requested a review from nainar January 13, 2022 08:55
@amp-owners-bot
Copy link

amp-owners-bot bot commented Jan 13, 2022

Hey @ampproject/wg-caching! These files were changed:

extensions/amp-gist/1.0/test/validator-amp-gist.html
extensions/amp-gist/1.0/test/validator-amp-gist.out

Copy link
Contributor

@MichaelRybak MichaelRybak left a comment

Choose a reason for hiding this comment

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

Approving changes in validator-. files. Please wait for approvals from other reviewers.

@dethstrobe
Copy link
Contributor

There doesn't appear to be a README.md?

Comment on lines +56 to +64
<ProxyIframeEmbed
title={title}
options={options}
ref={iframeRef}
type={TYPE}
messageHandler={messageHandler}
style={height ? {...style, height} : style}
{...rest}
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

How does this iframe get a src url to load the resource?

I see this must work, since you have it tested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As this component is a third-party embed component it generates the src URL from the 3p/github.js based on the gist ID passed to this component as prop

@deepaklalwani97
Copy link
Contributor Author

@dethstrobe I have updated the README.md for the component.

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

Successfully merging this pull request may close these issues.

None yet

4 participants