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

Fix for AMP height & width invalid attributes #695

Closed
wants to merge 1 commit into from
Closed

Fix for AMP height & width invalid attributes #695

wants to merge 1 commit into from

Conversation

onlineth
Copy link

Sometimes, there are dimensions for height and not the width and vice-versa. This leaves one attribute with no value which results as an invalid layout property. With this, now it will use the dimension for the given attribute and for the other dimension that wasn't given, it will use the default dimension.

Fixes invalid layout properties specifically with an empty value for height or width attributes.
@onlineth
Copy link
Author

onlineth commented May 15, 2017

Agreeing with @luigitec - #693 (comment), the unit tests do need to be updated.

if ( $new_attributes['height'] === '' ) {
$new_attributes['height'] = self::FALLBACK_HEIGHT;
}
if ( $new_attributes['width'] === '' ) {
Copy link
Member

Choose a reason for hiding this comment

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

Instead of strict equality with an empty string, perhaps empty() just in case the value is null or false.

@westonruter
Copy link
Member

See also #575.

@westonruter
Copy link
Member

I believe this was fixed with #979. Please let me know if otherwise.

@onlineth onlineth deleted the patch-1 branch March 21, 2018 15:25
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