[Product Pull Request] Exiting full-screen video does not return to correct scroll position

[arbrandes]

For Contributing Author:

This is the Primary Product Ticket for the following community contribution: Fix the bug that when exiting a full-screen video, the browser does not return to the correct scroll position.

Video demo:

Screen.2023-03-03.at.12.24.10.mov

Checklist prior to undergoing Product Review:

The following information is required in order for Product Managers to be able to review your pull request:

• Explanation of the problem being solved
• Description of how users will be impacted, and which users will be impacted
• Screenshots or video showing the functionality or fix, before and after
• Reproduction steps and/or testing steps

Only if necessary:

• If necessary, links to corresponding configuration changes
• If necessary, links to corresponding enablement changes, particularly waffle/toggle status details

Description

When exiting full-screen video, the browser does not return to the correct scroll position.

TODO: screenshots, reproduction instructions

PRs

master

• fix: save scroll position on exit from video xblock fullscreen mode frontend-app-learning#983
• fix: save scroll position on exit from video xblock fullscreen mode edx-platform#31061

palm

• fix: save scroll position on exit from video xblock fullscreen mode frontend-app-learning#1118
• Fix/scroll position on exit from video fullscreen mode palm edx-platform#32306

nutmeg

• fix: save scroll position on exit from video xblock fullscreen mode frontend-app-learning#981
• fix: save scroll position on exit from video xblock fullscreen mode edx-platform#31053

maple

• Fix/scroll position on exit from video fullscreen mode maple frontend-app-learning#982
• fix: save scroll position on exit from video xblock fullscreen mode edx-platform#31055

olive

• fix: save scroll position on exit from video xblock fullscreen mode frontend-app-learning#1077
• fix: save scroll position on exit from video xblock fullscreen mode edx-platform#31871

For Product Manager doing the review:

What criteria should be analyzed from Product to approve a PR?

• The problem being solved by the feature or fix is clear.
• There is clarity on how the change or fix will impact the end user.
• It is clear that the change will not negatively impact users or other areas of the platform.
• The change is implemented comprehensively.
• Any changes to UI use the current, standard Paragon Design System: https://paragon-openedx.netlify.app/

[ProductRyan]

@arbrandes - Let's use this PR as the main communication space for all of the PRs related to this issue.

I see that you have screenshots and reproduction steps in your TODO list. I'll be happy to give this a Product Review once those are added. I'm specifically interested to see the issue that you are attempting to fix and how your fix changes the user experience, perhaps through a video?

[arbrandes]

@ProductRyan, actually, I'm just acting as a product manager of sorts: I created this issue so we can track all of the related PRs. I was not aware of the issue at all until I saw that bunch of PRs.

So the right thing to do here is to assign @ihor-romaniuk: Ihor, is it ok if I do that?

[jmakowski1123]

@arbrandes - We're trying to shift all open PRs that require product review to the Open edX Roadmap. The idea is to create an umbrella ticket (like you've brilliantly done here) for each PR that serves as the source of truth for all product info. If no objections from you, I'm going to use this ticket as that umbrella ticket, and change the repo to the platform_roadmap, so there's consistency with all other product-level tickets we're creating, and I can use labels uniformly. cc @mphilbrick211 @itsjeyd

[github-actions]

Thanks for your submission, @openedx/open-edx-project-managers will review shortly.

[ihor-romaniuk]

Hi @ProductRyan @arbrandes @brian-smith-tcril.

@ProductRyan, I attached a short video about fixes as you asked.
Sorry for such a short video but GitHub has a limitation about file size.

Screen.2023-03-03.at.12.24.10.mov

Comments were fixed for PR openedx/edx-platform#31053 and added to related PRs for other releases.

Also, I have created new PRs for the olive release:

• fix: save scroll position on exit from video xblock fullscreen mode edx-platform#31871
• fix: save scroll position on exit from video xblock fullscreen mode frontend-app-learning#1077

[arbrandes]

I think we're now waiting for @ProductRyan to take a look at the video, right? Thanks, @ihor-romaniuk!

[itsjeyd]

Hey @ProductRyan, do you have any updates on when you'll be able to get back to this review?

CC @mphilbrick211

[ProductRyan]

@ihor-romaniuk this looks like a nice fix, thanks for contributing and bearing with us as we continue to develop our product review process. @itsjeyd this is approved from a product POV.

[itsjeyd]

Noted, thanks @ProductRyan!

@ihor-romaniuk I marked all PRs belonging to this ticket as ready for (engineering) review.

CC @mphilbrick211

[ihor-romaniuk]

Hi @itsjeyd. All PRs were updated and them are ready for review. Please take a look.

[itsjeyd]

Great, thanks @ihor-romaniuk. For frontend-app-learning, I posted on the main PR (#983) to get the changes lined up for engineering review.

For edx-platform, CC @mphilbrick211 (just in case you hadn't seen the latest update yet).

[ihor-romaniuk]

Hello everyone.
Because of a new Open edX Palm release, I've prepared additional PRs for that.
@arbrandes please add them to the PRs list:
openedx/edx-platform#32306
openedx/frontend-app-learning#1118

[leangseu-edx]

I left a comment on Maple pr. The rest are tested and approved.

[mphilbrick211]

Hi @leangseu-edx - just confirming that Product review is complete? Can we send to Engineering?

[leangseu-edx]

@mphilbrick211 This was approved by product before it was sent to me (engineering). Am I missing something?

[mphilbrick211]

@mphilbrick211 This was approved by product before it was sent to me (engineering). Am I missing something?

That was my typo, @leangseu-edx - apologies for that! I still see this one is open - can it also be merged? openedx/edx-platform#31055

[ihor-romaniuk]

@arbrandes I think we can close this roadmap because all MRs were merged and some of them were closed due to outdated edx version and no need for these changes.

Thank you all for your work!