feat: [FC-0070] render iframe with xblocks #1375

PKulkoRaccoonGang · 2024-10-09T21:41:08Z

Settings

EDX_PLATFORM_REPOSITORY: https://github.com/raccoongang/edx-platform.git
EDX_PLATFORM_VERSION: Peter_Kulko/listen-to-xblock-interaction-events

TUTOR_GROVE_NEW_MFES:
  authoring:
    port: 18000
    repository: https://github.com/raccoongang/frontend-app-course-authoring.git
    version: Peter_Kulko/render-iframe-with-xblocks

TUTOR_GROVE_WAFFLE_FLAGS:
  - name: contentstore.new_studio_mfe.use_new_unit_page
    everyone: true

TUTOR_GROVE_MFE_LMS_COMMON_SETTINGS:
  MFE_CONFIG:
    ENABLE_UNIT_PAGE: true

Description

This PR refactors the CourseUnit component by removing the DraggableList and CourseXBlock components and replacing them with a simpler XBlockContainerIframe. Additionally, it introduces new constants for iframe handling.

Testing instructions

сreate a new course.
open the Course unit page.
add some XBlocks to the unit.
the XBlocks will be displayed within an iframe.

Other information

The primary goal of this PR is to add an iframe for displaying XBlock content. Future changes will focus on enhancing the user experience and improving the interaction between actions passed from the React application to the Backbone framework.

openedx-webhooks · 2024-10-09T21:41:12Z

Thanks for the pull request, @PKulkoRaccoonGang!

What's next?

Please work through the following steps to get your changes ready for engineering review:

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To 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:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

🔘 Let us know that your PR is ready for review:

Who will review my changes?

This repository is currently maintained by @openedx/2u-tnl. Tag them in a comment and let them know that your changes are ready for review.

Where 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:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

codecov · 2024-10-09T21:52:38Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.95%. Comparing base (e118eb5) to head (eb5847e).
Report is 5 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1375      +/-   ##
==========================================
- Coverage   93.27%   92.95%   -0.33%     
==========================================
  Files        1052     1048       -4     
  Lines       20503    20379     -124     
  Branches     4312     4364      +52     
==========================================
- Hits        19125    18944     -181     
- Misses       1318     1371      +53     
- Partials       60       64       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/generic/hooks/index.jsx

src/course-unit/xblock-container-iframe/index.jsx

open-craft-grove · 2024-10-11T10:25:44Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-10-13T12:46:24Z

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-10-15T03:50:34Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-10-15T21:35:26Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

src/course-unit/xblock-container-iframe/index.tsx

src/course-unit/xblock-container-iframe/messages.ts

open-craft-grove · 2024-10-18T08:35:45Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-10-18T11:35:47Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-10-31T21:49:18Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-10-31T22:18:15Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-10-31T22:47:47Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-11-01T08:18:28Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

arbrandes · 2024-11-01T15:46:18Z

@PKulkoRaccoonGang, before I dive in, is this the only backend PR this depends on?

openedx/edx-platform#35694

PKulkoRaccoonGang · 2024-11-04T06:50:07Z

@arbrandes the key task of this PR is to display the iframe on the course unit page. Previously, a PR with a template and some styles for displaying xblocks was already merged into the edx-platform.

The platform branch that is currently specified for sandbox deployment is needed exclusively for some styles that I fixed while working on the following tasks.

refactor: added some tests

PKulkoRaccoonGang · 2024-11-07T18:26:02Z

[inform]: Changes related to editing functionality will be added in the following PRs.
#1444

open-craft-grove · 2024-11-07T19:02:19Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

arbrandes

I know there are upcoming editing changes, but noted that if I click edit and the block is down the page, somewhat, then the page won't scroll up to show the editor.

Other than that, the code looks good and it works as well as it sets out to. Approved!

arbrandes · 2024-11-08T12:37:08Z

src/course-unit/xblock-container-iframe/hooks.tsx

+ *
+ * This hook depends on the unit id just to make sure it re-evaluates whenever the ID changes.  If
+ * we change whether or not the Unit component is re-mounted when the unit ID changes, this may
+ * become important, as this hook will otherwise only evaluate the useLayoutEffect once.


Nice write-up. Thank you for the explanation!

(Also, this kind of thing is precisely why I continue to distrust iframes. But that's an entirely different discussion.)

arbrandes · 2024-11-08T13:12:35Z

src/generic/hooks/index.tsx

@@ -0,0 +1,2 @@
+// eslint-disable-next-line import/prefer-default-export


Once this merges we should be able to bump frontend-build and get rid of lines like this.

arbrandes · 2024-11-08T13:52:48Z

I'm merging based on the assumption that there will be some improvements later on. Off the top of my head:

The scroll-to omission I mention in a comment above,
The IFRAME_BOTTOM_OFFSET constant, which should probably be calculated instead of hard-coded

rpenido · 2024-11-11T18:21:31Z

Hi @PKulkoRaccoonGang!
After this change, the Unit Outiline page get stuck with an iFrame in the loading state.
Do you know if I need any additional configuration to make it work?

Thanks!

pomegranited · 2024-11-12T01:20:22Z

@rpenido @PKulkoRaccoonGang the new unit page is working for me so long as I run your branch from draft PR openedx/edx-platform#35777

Should that have been marked as a dependency of this PR, and merged before this one? CC @arbrandes

navinkarkera · 2024-11-12T10:10:32Z

@pomegranited @rpenido It seems to work for me even without openedx/edx-platform#35777 but the iframe height is stuck at 220px.

PKulkoRaccoonGang · 2024-11-12T14:00:57Z

@rpenido @pomegranited @navinkarkera Thanks for letting me know! I’m currently trying to reproduce this issue on the master platform and authoring, but so far without success. Could you provide more context on the issue you found? You could also try running npm run build in a CMS container to see if that helps.

[update]: The infinite loading issue may be related to the TypeError: null is not an object (evaluating 'r.appendChild') error that can be received if you add tags to a unit/xblock. Since tagging functionality is planned for future PRs, this change is not currently in the edx-platform.

Made a separate PR to fix this issue.
openedx/edx-platform#35827

navinkarkera · 2024-11-12T14:50:47Z

@PKulkoRaccoonGang It works fine for me in Brave (chromium based browser) but not in firefox. The height is not adjusted and I can see below error in console:

Uncaught DOMException: Permission denied to access property "offsetHeight" on cross-origin object
    <anonymous> http://studio.local.openedx.io:8001/container_embed/block-v1:ITcracy+DemoX+CourseX+type@vertical+block@59dc4eea07fa4c20aad677d433ada9f1:1336
    <anonymous> http://studio.local.openedx.io:8001/container_embed/block-v1:ITcracy+DemoX+CourseX+type@vertical+block@59dc4eea07fa4c20aad677d433ada9f1:1395

navinkarkera · 2024-11-12T14:53:01Z

Also, if I edit any component using the new mfe editors and go back it loads the whole studio again inside the iframe.

rpenido · 2024-11-12T22:23:31Z

@PKulkoRaccoonGang It works fine for me in Brave (chromium based browser) but not in firefox. The height is not adjusted and I can see below error in console:

Uncaught DOMException: Permission denied to access property "offsetHeight" on cross-origin object
    <anonymous> http://studio.local.openedx.io:8001/container_embed/block-v1:ITcracy+DemoX+CourseX+type@vertical+block@59dc4eea07fa4c20aad677d433ada9f1:1336
    <anonymous> http://studio.local.openedx.io:8001/container_embed/block-v1:ITcracy+DemoX+CourseX+type@vertical+block@59dc4eea07fa4c20aad677d433ada9f1:1395

Hi @navinkarkera! The height adjustment is working for me in Firefox.
Could you check using the taxonomy-sandbox branch for the tutor-mfe? We added some URLs to the CORS_WHITELIST that could be missing in master.

PKulkoRaccoonGang requested a review from a team as a code owner October 9, 2024 21:41

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 9, 2024

PKulkoRaccoonGang marked this pull request as draft October 9, 2024 21:41

bradenmacdonald reviewed Oct 10, 2024

View reviewed changes

src/generic/hooks/index.jsx Outdated Show resolved Hide resolved

src/course-unit/xblock-container-iframe/index.jsx Outdated Show resolved Hide resolved

PKulkoRaccoonGang added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Oct 11, 2024

PKulkoRaccoonGang added create-sandbox open-craft-grove should create a sandbox environment from this PR and removed create-sandbox open-craft-grove should create a sandbox environment from this PR labels Oct 13, 2024

PKulkoRaccoonGang self-assigned this Oct 13, 2024

PKulkoRaccoonGang marked this pull request as ready for review October 13, 2024 12:14

PKulkoRaccoonGang requested a review from arbrandes October 13, 2024 12:14

PKulkoRaccoonGang added create-sandbox open-craft-grove should create a sandbox environment from this PR and removed create-sandbox open-craft-grove should create a sandbox environment from this PR labels Oct 13, 2024

PKulkoRaccoonGang changed the title ~~feat: render iframe with xblocks~~ feat: [FC-0070] render iframe with xblocks Oct 13, 2024

PKulkoRaccoonGang requested review from ihor-romaniuk and GlugovGrGlib October 13, 2024 12:25

PKulkoRaccoonGang added create-sandbox open-craft-grove should create a sandbox environment from this PR and removed create-sandbox open-craft-grove should create a sandbox environment from this PR labels Oct 15, 2024

ihor-romaniuk reviewed Oct 16, 2024

View reviewed changes

src/course-unit/xblock-container-iframe/index.tsx Show resolved Hide resolved

src/course-unit/xblock-container-iframe/messages.ts Show resolved Hide resolved

PKulkoRaccoonGang added create-sandbox open-craft-grove should create a sandbox environment from this PR and removed create-sandbox open-craft-grove should create a sandbox environment from this PR labels Oct 18, 2024

mphilbrick211 added the FC Relates to an Axim Funded Contribution project label Oct 30, 2024

PKulkoRaccoonGang force-pushed the Peter_Kulko/render-iframe-with-xblocks branch from 18bea34 to d4abc74 Compare October 31, 2024 21:20

PKulkoRaccoonGang force-pushed the Peter_Kulko/render-iframe-with-xblocks branch from d4abc74 to aa96f1a Compare October 31, 2024 21:55

PKulkoRaccoonGang force-pushed the Peter_Kulko/render-iframe-with-xblocks branch from aa96f1a to ec7cdce Compare November 1, 2024 07:38

This was referenced Nov 6, 2024

feat: [FC-0070] rendering library content in unit page #1475

Open

feat: [FC-0070] rendering library content in unit page openedx/edx-platform#35785

Open

feat: render iframe with xblocks

eb5847e

refactor: added some tests

PKulkoRaccoonGang force-pushed the Peter_Kulko/render-iframe-with-xblocks branch from ec7cdce to eb5847e Compare November 7, 2024 18:23

arbrandes approved these changes Nov 8, 2024

View reviewed changes

arbrandes merged commit e59f284 into openedx:master Nov 8, 2024
6 of 7 checks passed

PKulkoRaccoonGang mentioned this pull request Nov 12, 2024

fix: fixed issue with iframe loading in Authoring mfe openedx/edx-platform#35827

Closed

navinkarkera mentioned this pull request Nov 12, 2024

Update workflow for library-sourced content (Authoring MFE - Unit Page) #1464

Open

rpenido mentioned this pull request Nov 12, 2024

fix: error while loading iframe on mfe Unit Outline [FC-0062] openedx/edx-platform#35828

Merged

bradenmacdonald mentioned this pull request Nov 13, 2024

Indicate library-sourced content (Authoring MFE - Unit Page) #1463

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [FC-0070] render iframe with xblocks #1375

feat: [FC-0070] render iframe with xblocks #1375

PKulkoRaccoonGang commented Oct 9, 2024 •

edited

Loading

openedx-webhooks commented Oct 9, 2024 •

edited

Loading

codecov bot commented Oct 9, 2024 •

edited

Loading

open-craft-grove commented Oct 11, 2024

open-craft-grove commented Oct 13, 2024

open-craft-grove commented Oct 15, 2024

open-craft-grove commented Oct 15, 2024

open-craft-grove commented Oct 18, 2024

open-craft-grove commented Oct 18, 2024

open-craft-grove commented Oct 31, 2024

open-craft-grove commented Oct 31, 2024

open-craft-grove commented Oct 31, 2024

open-craft-grove commented Nov 1, 2024

arbrandes commented Nov 1, 2024

PKulkoRaccoonGang commented Nov 4, 2024 •

edited

Loading

PKulkoRaccoonGang commented Nov 7, 2024

open-craft-grove commented Nov 7, 2024

arbrandes left a comment

arbrandes Nov 8, 2024

arbrandes Nov 8, 2024

arbrandes Nov 8, 2024

arbrandes commented Nov 8, 2024 •

edited

Loading

rpenido commented Nov 11, 2024 •

edited

Loading

pomegranited commented Nov 12, 2024

navinkarkera commented Nov 12, 2024 •

edited

Loading

PKulkoRaccoonGang commented Nov 12, 2024 •

edited

Loading

navinkarkera commented Nov 12, 2024

navinkarkera commented Nov 12, 2024

rpenido commented Nov 12, 2024

		@@ -0,0 +1,2 @@
		// eslint-disable-next-line import/prefer-default-export

feat: [FC-0070] render iframe with xblocks #1375

feat: [FC-0070] render iframe with xblocks #1375

Conversation

PKulkoRaccoonGang commented Oct 9, 2024 • edited Loading

Description

Testing instructions

Other information

openedx-webhooks commented Oct 9, 2024 • edited Loading

What's next?

🔘 Get product approval

🔘 Provide context

🔘 Get a green build

🔘 Let us know that your PR is ready for review:

Who will review my changes?

Where can I find more information?

When can I expect my changes to be merged?

codecov bot commented Oct 9, 2024 • edited Loading

Codecov Report

open-craft-grove commented Oct 11, 2024

open-craft-grove commented Oct 13, 2024

open-craft-grove commented Oct 15, 2024

open-craft-grove commented Oct 15, 2024

open-craft-grove commented Oct 18, 2024

open-craft-grove commented Oct 18, 2024

open-craft-grove commented Oct 31, 2024

open-craft-grove commented Oct 31, 2024

open-craft-grove commented Oct 31, 2024

open-craft-grove commented Nov 1, 2024

arbrandes commented Nov 1, 2024

PKulkoRaccoonGang commented Nov 4, 2024 • edited Loading

PKulkoRaccoonGang commented Nov 7, 2024

open-craft-grove commented Nov 7, 2024

arbrandes left a comment

Choose a reason for hiding this comment

arbrandes Nov 8, 2024

Choose a reason for hiding this comment

arbrandes Nov 8, 2024

Choose a reason for hiding this comment

arbrandes Nov 8, 2024

Choose a reason for hiding this comment

arbrandes commented Nov 8, 2024 • edited Loading

rpenido commented Nov 11, 2024 • edited Loading

pomegranited commented Nov 12, 2024

navinkarkera commented Nov 12, 2024 • edited Loading

PKulkoRaccoonGang commented Nov 12, 2024 • edited Loading

navinkarkera commented Nov 12, 2024

navinkarkera commented Nov 12, 2024

rpenido commented Nov 12, 2024

PKulkoRaccoonGang commented Oct 9, 2024 •

edited

Loading

openedx-webhooks commented Oct 9, 2024 •

edited

Loading

codecov bot commented Oct 9, 2024 •

edited

Loading

PKulkoRaccoonGang commented Nov 4, 2024 •

edited

Loading

arbrandes commented Nov 8, 2024 •

edited

Loading

rpenido commented Nov 11, 2024 •

edited

Loading

navinkarkera commented Nov 12, 2024 •

edited

Loading

PKulkoRaccoonGang commented Nov 12, 2024 •

edited

Loading