Skip to content

Added wrapper around iframe to make it responsive.#243

Merged
Gaurav0 merged 1 commit intoember-cli:masterfrom
brianally:embed
Oct 27, 2015
Merged

Added wrapper around iframe to make it responsive.#243
Gaurav0 merged 1 commit intoember-cli:masterfrom
brianally:embed

Conversation

@brianally
Copy link
Copy Markdown
Contributor

Fixes #242. The embed markup includes a wrapper div and some minimal inline styles on both it and the iframe to allow it to use the full width of the containing element. I chose 16:9 ratio. I tried 4:3 but found it to be too tall. This could be configurable, although it would mean an extra step for the user in creating the embed code.

@brianally
Copy link
Copy Markdown
Contributor Author

I just noticed i needlessly changed a couple of lines. I saw later that the rest of the file uses single quotes so changed my doubles, changing a couple of originals as well.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Many people who are embedding this in a page will want to specify the width and the height. How would they do this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

By setting the width of the containing element, ie. what they're embedding it into. With this change, the iframe takes up the full width of the "column" it's in and the height adjusts proportionally. See here for more info on how this works.

As it stands now, users cannot set the dimensions of the iframe unless they edit the markup before pasting into their own document.

@Gaurav0
Copy link
Copy Markdown
Contributor

Gaurav0 commented Oct 27, 2015

👍

Gaurav0 added a commit that referenced this pull request Oct 27, 2015
Added wrapper around iframe to make it responsive.
@Gaurav0 Gaurav0 merged commit de00d0d into ember-cli:master Oct 27, 2015
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