-
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
Support disable-inline-width
attribute when applying layout sizes
#27083
Conversation
65e5758
to
ba1ebb0
Compare
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.
Code LGTM. I think this still needs validator changes to make it valid?
Hey @ampproject/wg-caching, these files were changed:
|
…mpproject#27083) * Do not apply sizes for `disable-inline-width` * Add documentation for `disable-inline-width` * Add amp-img test * Update documentation * Move attribute check to ternary * Add attribute to validator
* cl/301228504 Revision bump for ampproject#27083 * cl/301306664 Revision bump for ampproject#27180 * Update validator-amp-list.out * Update validator-amp-list.out * Update validator-amp-list.out
Is this related to #17053? does the presence of |
@westonruter Yes, this is related as it directly resolves the original issue filed:
|
@caroqliu I think I may have identified something that was missed with support for Lines 59 to 65 in 97099ce
Should not that last selector be changed to exclude elements that have the - [width][height][sizes]:not(.i-amphtml-layout-responsive)
+ [width][height][sizes]:not([disable-inline-width]):not(.i-amphtml-layout-responsive) Otherwise, a regression is introduced by adopting |
This allows specifying the
sizes
on elements without getting the sizing behavior of sizes in AMP (which adds a width based on which media query matches). Addresses #21736See discussion doc for more