edx-platform
edx-platform copied to clipboard
openedx
fix: save scroll position on exit from video xblock fullscreen mode
This merge request contains a fix for toggling video xblock full-screen mode and saving the previous window top offset position on exit from the full-screen state.
A related bug was found here https://bugs.chromium.org/p/chromium/issues/detail?id=142427 but it still reproduces.
Realised solution: Save the scroll position before the turn on the fullscreen mode and scroll to the previous position on turn off the fullscreen mode.
Dependent PR to MFE Learning: This MR https://github.com/openedx/frontend-app-learning/pull/983 must be merged with this MR.
Thanks for the pull request, @ihor-romaniuk! Please note that it may take us up to several weeks or months to complete a review and merge your PR.
Feel free to add as much of the following information to the ticket as you can:
- supporting documentation
- Open edX discussion forum threads
- timeline information ("this must be merged by XX date", and why that is)
- partner information ("this is a course on edx.org")
- any other information that can help Product understand the context for the PR
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.
Please let us know once your PR is ready for our review and all tests are green.
📣 💥 Heads-up: You must either rebase onto master or merge master into your branch to keep passing required checks.
We added a new required check, "Tests Successful," that this PR does not yet run. Rebasing will get it started.
If you have any questions, please reach out to the Architecture team (either #architecture on Open edX Slack or #external-architecture on edX internal Slack).
Hi @ProductRyan - just checking to see if you've had a chance to take a look at this?
4/5/23 - Product review complete.
@openedx/content-aurora @mattcarter - is this something you can take a look at for review / merge? This PR is related to the other "scroll position" PRs listed here: #31053.
Hi @ihor-romaniuk! While this is in review, would you mind re-running the tests on this PR? We've had a couple new ones pop up (shellcheck). Thanks!
@openedx/content-aurora @mattcarter - Hi there! Just seeing if you are able to review/merge this for us? Thanks!
@openedx/content-aurora @mattcarter - Hi there! Just seeing if you are able to review/merge this for us? Thanks!
Hi @mattcarter @openedx/content-aurora! Friendly ping on this :)
LGTM 👍
THanks, @leangseu-edx! Do you know who could get this merged for us?
@ihor-romaniuk 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.
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.
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.