Skip to content

Conversation

saraburns1
Copy link
Contributor

Description

Fixes openedx/openedx-aspects#316 by scrolling window to the xblock location instead of scrolling within the iFrame

Supporting information

Link to other information about the change, such as Jira issues, GitHub issues, or Discourse discussions.
Be sure to check they are publicly readable, or if not, repeat the information here.

Testing instructions

Test with openedx/frontend-app-authoring#2363
Need to enable Aspects in-context metrics (https://github.com/openedx/frontend-plugin-aspects)

Deadline

"None" if there's no rush, or provide a specific date or event (and reason) if there is one.

Other information

Include anything else that will help reviewers and consumers understand the change.

  • Does this change depend on other changes elsewhere?
  • Any special concerns or limitations? For example: deprecations, migrations, security, or accessibility.
  • If your database migration can't be rolled back easily.

@saraburns1 saraburns1 changed the title fix: scroll entire window instead of iframe fix: send scroll offset message to window Aug 8, 2025
@saraburns1 saraburns1 requested review from arbrandes and farhaanbukhsh and removed request for arbrandes and farhaanbukhsh August 8, 2025 16:01
@brian-smith-tcril
Copy link
Contributor

I think before we land this we should:

  • Confirm frontend-app-authoring is the only place this is used. This is moving the responsibility of actually scrolling the content into view over to the MFE, so we should make sure any MFEs that are currently relying on this functionality are updated to handle the new message being sent.
  • Land the frontend-app-authoring PR - if this PR lands before that does then we'll lose the scrolling functionality until that one lands.

@saraburns1
Copy link
Contributor Author

@brian-smith-tcril - the only other place scrollToXblock is used is in frontend-plugin-aspects which is installed within frontend-app-authoring for the in-context metrics in studio so I think we're good

@brian-smith-tcril
Copy link
Contributor

@brian-smith-tcril - the only other place scrollToXblock is used is in frontend-plugin-aspects which is installed within frontend-app-authoring for the in-context metrics in studio so I think we're good

Sounds good! This is technically a breaking change so we should probably make sure the commit message communicates that.

@saraburns1 saraburns1 force-pushed the sburns_unit_iframe_cutoff branch from b4289b4 to 0b04200 Compare August 15, 2025 14:42
@brian-smith-tcril brian-smith-tcril changed the title fix: send scroll offset message to window feat!: Make MFE scroll content instead of iFrame when scrollToXblock is called Aug 15, 2025
@brian-smith-tcril
Copy link
Contributor

@saraburns1 I created the fast-track DEPR ticket, please feel free to edit it if there's anything you'd like to add/update!

@farhaanbukhsh
Copy link
Member

farhaanbukhsh commented Aug 19, 2025

@brian-smith-tcril @saraburns1 scrollToXBlocks was introduced during the landing of in-context metrics, so I assume it's not being used in many places. However, it is always better to be safe than sorry. Thanks a lot for the DEPR ticket.

Also please let me know if I can merge this since I tested this and it is working 😄

Copy link
Member

@farhaanbukhsh farhaanbukhsh left a comment

Choose a reason for hiding this comment

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

👍

  • ✅ I tested this with incontext-metrics and it's working fine 👍🏾
  • ✅ I read through the code
  • ❌ I checked for accessibility issues
  • ❌ Includes documentation

@brian-smith-tcril
Copy link
Contributor

@farhaanbukhsh the DEPR ticket is a "fast-track" DEPR, and is marked as "Transition Already Unblocked."

Feel free to merge this!

@farhaanbukhsh farhaanbukhsh merged commit 405282c into master Aug 19, 2025
49 checks passed
@farhaanbukhsh farhaanbukhsh deleted the sburns_unit_iframe_cutoff branch August 19, 2025 20:02
@farhaanbukhsh
Copy link
Member

Thanks @brian-smith-tcril and @saraburns1 for the work. 🚀

@edx-pipeline-bot
Copy link
Contributor

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

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

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.

In-context metrics: First component in a unit cut off when there is a warning on the Studio unit page
4 participants