-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat: post component height to parent window [FC-0083] #36477
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
feat: post component height to parent window [FC-0083] #36477
Conversation
|
Thanks for the pull request, @navinkarkera! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
0fce6f1 to
5659d38
Compare
c1bca0f to
bd45d68
Compare
pomegranited
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
- I tested this using the PR test instructions on Component cards, Component modal, Unit components, and course Unit pages with blocks.
- I read through the code
- I checked for accessibility issues by using my keyboard to navigate
- Includes documentation of the tricky stuff around iframe resizing
-
User-facing strings are extracted for translationN/A
bd45d68 to
46f1a75
Compare
|
@navinkarkera This brokes some advanced editors: Completion LT Consumer The editor is not displayed |
|
@ChrisChV I updated iframe minimum height to 200px here: openedx/frontend-app-authoring@ea853aa and the editors are visible now. vokoscreenNG-2025-04-11_12-27-18.webmThe LTI and pdf previews were not rendering properly even before this PR. |
When I use both ( Note: For the PDF block we are using this requirement: git+https://github.com/open-craft/xblock-pdf.git@v1.1.0#egg=xblock-pdf https://www.loom.com/share/857c28aede314d38b5ae6c12efa8f83d?sid=ae74c79f-8d5d-40ac-acad-f680a28a6b34 |
|
@ChrisChV is the issue fixed with the latest changes in both PRs? I can increase the minimum height tomorrow if required. |
@navinkarkera Only this issue was fixed, but the editor is still small. Is this a simple fix? Just increase the size here: openedx/frontend-app-authoring@ea853aa? If so, I can do it today. |
The LTI editor is being displayed properly no? Am I testing the wrong component? |
From what I see, yes, the LTI has not had a problem, the one that had a problem is the LTI consumer, which is a different block 😄
I have tried changing it to 700px and it works fine |
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
|
@ChrisChV Setting min-height to 700 make unit page previews weird (all components take up 700px), so I created a small PR that sets min height based on preview location (large in modal previews) and blocktypes. Please review: openedx/frontend-app-authoring#1813 whenever possible. |
XBlock rendered in an iframe in frontend-app-authoring needs to resize automatically based on component size. Updates xblock iframe template to post message to parent window on size change.
XBlock rendered in an iframe in frontend-app-authoring needs to resize automatically based on component size. Updates xblock iframe template to post message to parent window on size change.
| var lastHeight = window.parent[0].offsetHeight; | ||
| var lastWidth = window.parent[0].offsetWidth; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@navinkarkera Apparently these lines are causing same-origin errors:
Uncaught SecurityError: Failed to read a named property 'offsetHeight' from 'Window': Blocked a frame with origin "https://studio.stage.edx.org/" from accessing a cross-origin frame.
Can we change the initial value of lastHeight and lastWidth to something that doesn't try to access window.parent?
Also what is the [0] in window.parent[0] even doing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bradenmacdonald Turns out that we don't even need this piece of code.
Created a very small PR: #36643
openedx#36477 (comment) (cherry picked from commit 6508010)
…6652) #36477 (comment) (cherry picked from commit 6508010)
XBlock rendered in an iframe in frontend-app-authoring needs to resize automatically based on component size. Updates xblock iframe template to post message to parent window on size change.


Description
XBlock rendered in an iframe in frontend-app-authoring needs to resize automatically based on component size.
This PR updates xblock iframe template to post message to parent window on size change.
Test instructions
Follow instructions from openedx/frontend-app-authoring#1779 and make sure that the components are properly resized in unit page, preview modals and course unit page.