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

Add default values for AMP Timeago block #1586

Merged
merged 1 commit into from Nov 4, 2018

Conversation

Projects
None yet
2 participants
@kienstra
Collaborator

kienstra commented Nov 2, 2018

Thanks to @csossi for reporting this in #1580.

Creating an AMP Timeago block with the default values causes a validation error in the Chrome extension because it doesn't have the required height value:

height-value-required

So this sets a default value of 20 for the height. This is the value in this example.

It also sets the default layout to fixed-height. This is the only layout available that doesn't require a width (there's no default width in the editor):

new-default-values

Fixes #1580

Add a default height for the AMP Timeago block
As Claudio reported,
there's an error when using the AMP Timeago block
with the default values
The layout is inially responsive,
which requires both a width an height.
And another option of fixed also requires a width and height.
So set fixed-height as the default layout,
as set a default height of 20.
This is the value in the documentation example.
So at least this block with the default values
won't have a validation error

@see https://www.ampproject.org/ko/docs/reference/components/amp-timeago
@kienstra

This comment has been minimized.

Collaborator

kienstra commented Nov 2, 2018

Request For Code Review

Hi @westonruter,
Could you please review this?

Thanks, and have a great weekend.

@kienstra kienstra requested a review from westonruter Nov 2, 2018

@westonruter westonruter added this to the v1.0 milestone Nov 4, 2018

@westonruter westonruter merged commit af51929 into 1.0 Nov 4, 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 fix/1580-amp-timeago branch Nov 4, 2018

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