Skip to content

Report iframed courseware content dimensions to parent window #22972

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.

Sign up for GitHub

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

Jump to bottom
Merged
abutterworth merged 2 commits into from
Jan 29, 2020
Merged

Report iframed courseware content dimensions to parent window #22972

abutterworth merged 2 commits into from
Jan 29, 2020

Conversation

@abutterworth
Copy link
Contributor

@abutterworth abutterworth commented Jan 29, 2020 •
edited
Loading

davestgermain
davestgermain reviewed Jan 29, 2020
View reviewed changes
lms/templates/courseware/courseware-chromeless.html
</nav>
% endif

<script type="text/javascript">
Copy link
Contributor

@davestgermain davestgermain Jan 29, 2020

Choose a reason for hiding this comment

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

Why not consolidate this with the earlier iframe check? Appending the <base> tag can happen here, right?

Copy link
Contributor Author

@abutterworth abutterworth Jan 29, 2020

Choose a reason for hiding this comment

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

It can, I just thought repeating myself to keep the functionality isolated would be good. Also it's worth noting that the base tag script needs to be in the head. Not a strong reason though. Let me know if you prefer combining them.

Copy link
Contributor

@davidjoy davidjoy Jan 29, 2020

Choose a reason for hiding this comment

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

I put it at the bottom originally so that most of the page would be loaded by the time it ran. It may not be important, but keeping it there may cut down on events as things resize and fill in during load.

@edx-status-bot
Copy link

edx-status-bot commented Jan 29, 2020

Your PR has finished running tests. There were no failures.

@abutterworth abutterworth merged commit 59bb664 into master Jan 29, 2020
@abutterworth abutterworth deleted the abutterworth/resize-vertical-block-iframe branch January 29, 2020 17:44
@edx-pipeline-bot
Copy link
Contributor

edx-pipeline-bot commented Jan 29, 2020

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

edx-pipeline-bot commented Jan 29, 2020

EdX Release Notice: This PR has been deployed to the production environment.

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

Reviewers

@ormsbee ormsbee Awaiting requested review from ormsbee

@davidjoy davidjoy Awaiting requested review from davidjoy

1 more reviewer

@davestgermain davestgermain davestgermain approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@abutterworth @edx-status-bot @edx-pipeline-bot @davidjoy @davestgermain