fix: save scroll position on exit from video xblock fullscreen mode #31061
Conversation
|
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:
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. |
openedx-webhooks
added
the
open-source-contribution
|
📣 💥 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). |
ihor-romaniuk
force-pushed
the
fix/scroll-position-on-exit-from-video-fullscreen-mode-master
branch
from
d58f41f
to
eacb86a
Compare
|
@nedbat Brunch was updated and ready for review. |
|
@ProductRyan @jmakowski1123 this looks ready for Product review. |
arbrandes
added
the
bug
|
Hi @ProductRyan - just checking to see if you've had a chance to take a look at this? |
ihor-romaniuk
force-pushed
the
fix/scroll-position-on-exit-from-video-fullscreen-mode-master
branch
from
fbaaa0f
to
9ac109e
Compare
…lscreen-mode-master
|
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. |
mphilbrick211
removed
the
bug
|
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! |
…lscreen-mode-master
|
@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 :) |
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. |
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 openedx/frontend-app-learning#983 must be merged with this MR.