-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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-iframe] changes to publish to npm #36190
Merged
Merged
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
823414f
WIP
dmanek 93ad29b
Merge branch 'main' into amp-iframe-docs
dmanek 29f19f4
Merge branch 'main' into amp-iframe-docs
dmanek 4b324a7
1. Rename IFrame to BentoIframe
dmanek 21aa71a
updates to amp-iframe.md
dmanek 5db93a2
add CSS
dmanek ee4c0a3
minor fix
dmanek 8aae272
adding text on `frameborder` deprecation
dmanek 9669d1f
address code review comment
dmanek c4869ad
Merge branch 'main' into amp-iframe-docs
dmanek File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
/* | ||
* Pre-upgrade: | ||
* - display:block element | ||
* - size-defined element | ||
*/ | ||
amp-iframe { | ||
display: block; | ||
overflow: hidden; | ||
position: relative; | ||
} | ||
|
||
/* Pre-upgrade: size-defining element - hide text. */ | ||
amp-iframe:not(.i-amphtml-built) { | ||
color: transparent !important; | ||
} | ||
|
||
/* Pre-upgrade: size-defining element - hide children. */ | ||
amp-iframe:not(.i-amphtml-built) | ||
> :not([placeholder]):not([slot='i-amphtml-svc']) { | ||
display: none; | ||
content-visibility: hidden; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,15 +12,8 @@ bento: true | |
|
||
## Usage | ||
|
||
[tip type="warning"] | ||
|
||
This component is work in progress. | ||
|
||
[/tip] | ||
|
||
Displays an AMP valid iframe. `amp-iframe` has several important differences from vanilla iframes that are | ||
designed to make it more secure and avoid AMP files that are dominated by a | ||
single iframe: | ||
designed to make it more secure and avoid AMP files that are dominated by a single iframe: | ||
|
||
- An `amp-iframe` may not appear close to the top of the document (except for | ||
iframes that use `placeholder` as described | ||
|
@@ -41,7 +34,6 @@ single iframe: | |
height="100" | ||
sandbox="allow-scripts allow-same-origin" | ||
layout="responsive" | ||
frameborder="0" | ||
src="https://www.google.com/maps/embed/v1/place?key=AIzaSyAyAS599A2GGPKTmtNr9CptD61LE4gN6oQ&q=iceland" | ||
> | ||
</amp-iframe> | ||
|
@@ -50,17 +42,19 @@ single iframe: | |
<amp-iframe width="200" height="100" | ||
sandbox="allow-scripts allow-same-origin" | ||
layout="responsive" | ||
frameborder="0" | ||
src="https://www.google.com/maps/embed/v1/place?key=AIzaSyAyAS599A2GGPKTmtNr9CptD61LE4gN6oQ&q=iceland"> | ||
</amp-iframe> | ||
|
||
### Migrating from 0.1 | ||
|
||
Unlike `0.1`, the experimental `1.0` version of `amp-iframe` has deprecated the `frameborder` attribute. Use the CSS `border` property to control `<amp-iframe>` borders. | ||
|
||
### Use existing AMP components instead of `amp-iframe` | ||
|
||
The `amp-iframe` component should be considered a fallback if the required user | ||
experience is not possible by other means in AMP, that is, there's not already | ||
an existing AMP component | ||
for the use case. This is because there are many benefits to using an AMP | ||
component tailored for a specific use-case such as: | ||
an existing AMP component for the use case. This is because there are many | ||
benefits to using an AMP component tailored for a specific use-case such as: | ||
|
||
- Better resource management and performance | ||
- Custom components can provide built-in placeholder images in some cases. | ||
|
@@ -92,6 +86,10 @@ The reasons for this policy are that: | |
- `amp-iframe` has no fully iframe controlled resize mechanism. | ||
- Viewability information may not be available to `amp-iframe`. | ||
|
||
### Standalone use outside valid AMP documents | ||
|
||
Bento AMP allows you to use AMP components in non-AMP pages without needing to commit to fully valid AMP. You can take these components and place them in implementations with frameworks and CMSs that don't support AMP. Read more in our guide [Use AMP components in non-AMP pages](https://amp.dev/documentation/guides-and-tutorials/start/bento_guide/). To find the standalone version of `amp-iframe`, see [`bento-iframe`](./1.0/README.md). | ||
|
||
### Iframe with placeholder <a name="iframe-with-placeholder"></a> | ||
|
||
It is possible to have an `amp-iframe` appear at the top of a document when the | ||
|
@@ -243,7 +241,8 @@ window.addEventListener('message', function (event) { | |
|
||
The intersection message would be sent by the parent to the iframe in the format of IntersectionObserver entry wheneve there is intersectionRatio change across thresholds [0, 0.05, 0.1, ... 0.9, 0.95, 1]. | ||
|
||
## Iframe & Consent Data | ||
<!-- TODO: uncomment when Bento amp-consent (v1.0) is ready --> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's create an issue or something to refer to here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. Created #36211 & linked in todo. |
||
<!-- ## Iframe & Consent Data | ||
|
||
Iframes can send a `send-consent-data` message to receive consent data if a CMP is present on their parents page. | ||
|
||
|
@@ -288,6 +287,7 @@ window.addEventListener('message', function (event) { | |
console.log(event.data.consentString); | ||
}); | ||
``` | ||
--> | ||
|
||
## Attributes | ||
|
||
|
@@ -353,5 +353,4 @@ direct user purpose such as being invisible or small. | |
|
||
## Validation | ||
|
||
See [`amp-iframe` rules](validator-amp-iframe.protoascii) | ||
in the AMP validator specification. | ||
See [`amp-iframe` rules](validator-amp-iframe.protoascii) in the AMP validator specification. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Until README.md exists, this will have a dead link issue in CI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right. README.me is added in #36210 and will be merged soon.