-
Notifications
You must be signed in to change notification settings - Fork 383
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
Remove 'sizes' attribute for <amp-iframe> and <amp-video>; add appropriate layout #937
Merged
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
87e8501
Issue #864: Remove 'sizes' attribute for <amp-iframe> and <amp-video>.
936f302
Issue #864: Remove extra line in set_layout().
6848541
Issue #864: Remove the 'sizes' attribute from <amp-img>.
9348ed3
Merge branch 'develop' into fix/864-remove-sizes
11405b3
Issue #864: Remove layout="responsive" from <amp-iframe>.
f95c0be
Issue #864: Add comment after addition of 'style.'
6bd4326
Issue #864: Remove <amp-img> sizes attribute if present.
613f16f
Issue #864: Add layout="responsive" to <amp-iframe>.
878e023
Issue #864: Convert 'data-amp-layout' to 'layout.'
037f6ef
Issue #864: Allow 'data-amp-layout' in wp_kses() for 'post.'
a233b59
Issue #864: Change logic for adding 'data-amp-layout'.
011e478
Issue #864: Move add_layout() to AMP_Theme_Support.
c5c46ac
Merge in develop, resolve conflicts.
e344418
Rename to whitelist_layout_in_wp_kses_allowed_html().
adce308
864 - Use intrinsic as default layout for images
mehigh 81e20c5
Merge in 0.7, resolve conflicts.
bc5c407
Update unit tests for layout.
ece0f86
Align equals signs, use () after class.
02db229
Set default layout of <amp-iframe> and <amp-video> to intrinsic.
f983029
Remove my duplicate layout assignment in AMP_Video_Sanitizer.
f5a3c73
Conditionally set layout to 'responsive' for <amp-video>.
c347f2b
Merge in 0.7 branch, resolve conflict.
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
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
Oops, something went wrong.
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.
@kienstra We need to add
layout
attribute here so that it's possible to control the image sizing at the theme's level (see my comment in the AMPConf theme).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 sounds good, @delawski. If it's alright, I'll work on this in several hours.
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.
@delawski @kienstra Adding a
layout
to an HTMLimg
element makes me nervous. I think it would be better actually to introduce andata-amp-layout
attribute which the sanitizer could convert into alayout
attribute when converting animg
into anamp-img
. This would allow for regular plain HTML in post content to be decorated with thedata-amp-layout
attribute and for the content to be still served as valid HTML. This will also allow for suchimg[data-amp-layout]
to be styled with CSS in non-AMP responses. This would play nicely into adding AMP support for Gutenberg blocks as described in #902 (comment)We'll need to allow
data-amp-layout
attribute to Kses for it to be saved withunfiltered_html
.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.
Thanks, @westonruter. The 2 commits below convert
'data-amp-layout'
to'layout'
in the image sanitizer, and allow'data-amp-layout'
inwp_kses()
, when in a'post'
context.