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

Make the default layout responsive for the <amp-ooyala-player> block #1585

Merged
merged 1 commit into from Nov 4, 2018

Conversation

Projects
None yet
2 participants
@kienstra
Copy link
Collaborator

kienstra commented Nov 2, 2018

Thanks to @csossi for reporting this in #1581.

Steps To Reproduce

  1. Activate Twenty Seventeen
  2. Create a new post with the block editor
  3. Add an AMP Ooyala Player block
  4. Set these values for it:
    Video embed code (required): Vxc2k0MDE6Y_C7J5podo3UDxlFxGaZrQ
    Player ID (required): 6440813504804d76ba35c8c787a4b33c
    Provider code for the account (required): 5zb2wxOlZcNCe_HVT3a6cawW298X
    Leave the defaults for the rest:

ooyala-player

  1. Publish the post, and go to the front-end
  2. Expected: The <amp-ooyala-player> remains within its container
  3. Actual: it overflows its container

ooyala-overflows-container

  • As Claudio pointed out, the default layout of fixed can cause it to overflow the container.
  • The layout of responsive seems to keep the width to the container size in Twenty Fifteen, Sixteen, and Seventeen.
  • So this changes the default layout from fixed to responsive.
  • Still, if the content area is floated, like in a mobile-first grid, a fixed layout might work better. Responsive layouts mean the element only has the width of its parent. But there's a <select> in the block editor to change this to something like fixed if needed.

Fixes #1581

Make the default layout reponsive for the <amp-ooyala-player> block
As Claudio pointed out,
the default layout of fixed can cause it
to overflow the container.
responsive seems to keep the width to the container size
in Twenty Fifteen, Sixteen, and Seventeen.
Still, if the content area is floated,
like in a mobile-first grid, a fixed layout might work better.
Responsive layouts mean the element only has the width of its parent.
But there's a <select> in the block editor to change this.

@kienstra kienstra changed the title Make the default layout reponsive for the <amp-ooyala-player> block Make the default layout responsive for the <amp-ooyala-player> block Nov 2, 2018

@kienstra

This comment has been minimized.

Copy link
Collaborator Author

kienstra commented Nov 2, 2018

Request For Review

Hi @westonruter,
Hope you're having a great day.

Could you please review this?

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

@kienstra kienstra changed the base branch from develop to 1.0 Nov 2, 2018

@westonruter westonruter merged commit 5ebf1a9 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/1581-ooyala-player branch Nov 4, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.