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

fix: courseware-chromeless.html iframe height #132

Merged

Conversation

kuipumu
Copy link

@kuipumu kuipumu commented Oct 1, 2024

Description

This PR adds a fix to the courseware-chromeless.html template, this template is used to render the XBlock on the learning MFE in an Iframe, this template contains a JavaScript code that sends a postMessage to the parent window that is used by the learning MFE to resize the Iframe to the heigth of its content, on the initial load of the Iframe the height is incorrectly calculated when the maintenance banner is added to the unit content, to fix this an extra 250px is added to the calculated height of the Iframe.

@kuipumu kuipumu requested a review from alexjmpb October 1, 2024 12:45
@kuipumu kuipumu self-assigned this Oct 1, 2024
@kuipumu kuipumu requested review from Serafin-dev and removed request for alexjmpb October 1, 2024 14:09
@kuipumu kuipumu force-pushed the kuipumu/fix-courseware-iframe-heigth branch from 9c73788 to 44d1c52 Compare October 1, 2024 15:17
var lastHeight = window.offsetHeight;
// An extra 250 height is added to make sure the iframe height
// is enough in case a maintance banner is added to the iframe.
var lastHeight = window.offsetHeight + 250;

Choose a reason for hiding this comment

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

Hi @kuipumu . Have you considered document.body.scrollHeight instead of setting an arbitrary value?

Copy link
Author

Choose a reason for hiding this comment

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

@Serafin-dev I tried using scrollHeight but it doesn't seem to work, ¿did you tested locally?

Choose a reason for hiding this comment

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

yes, but same happened to me. If this solves the issue it is ok.

@kuipumu kuipumu merged commit be6e128 into pearson-release/olive.stage Oct 1, 2024
14 of 35 checks passed
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