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

Eliminate amp-wp-enforced-sizes style from theme support stylesheet #1153

Merged
merged 1 commit into from May 17, 2018

Conversation

3 participants
@westonruter
Copy link
Member

westonruter commented May 17, 2018

This style rule causes problems on AMP Native sites (where amp theme support is present). An image element may have margins and width associated with it which the amp-wp-enforced-sizes style rule clobbers unexpectedly.

For example, in non-AMP given:

image

With CSS properties:

.logo {
    display: block;
    margin: 2rem 0 0;
    width: 300px;
    max-width: 100%;
    margin-bottom: 2rem;
}

This gets rendered in AMP as:

image

Since the following override:

.amp-wp-enforced-sizes {
    max-width: 100%;
    margin: 0 auto;
}

I don't think this is relevant in an amp theme support context. I believe it was only relevant to AMP legacy post templates and can be removed now.

@westonruter westonruter added this to the v1.0 milestone May 17, 2018

@westonruter westonruter requested a review from kienstra May 17, 2018

@kienstra
Copy link
Collaborator

kienstra left a comment

Approved

Hi @westonruter,
Thanks, good idea to remove that style rule when theme support is present. I think you're right that it doesn't make sense outside of legacy templating.

@westonruter westonruter merged commit ab4b7f5 into develop May 17, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@westonruter westonruter deleted the update/amp-default-stylesheet branch May 17, 2018

@westonruter westonruter added this to Ready for QA in v1.0 May 31, 2018

@kienstra kienstra moved this from Ready for QA to Ready for Merging in v1.0 Jun 6, 2018

@kienstra kienstra moved this from Ready for Merging to In Production in v1.0 Dec 11, 2018

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