search

overhangio / openedx-scorm-xblock

SCORM XBlock for Open edX
GNU Affero General Public License v3.0
37 stars 48 forks source link

fix: enable browser fullscreen mode for the scorm x-block #23

Closed bydawen closed 1 year ago

bydawen commented 1 year ago

Fix to prevent incorrect rendering fullscreen mode when scorm x-block in MFE application.

ihor-romaniuk commented 1 year ago

@regisb Could you take a look at this PR, please?

regisb commented 1 year ago

Yes, this is on my todo-list.

bydawen commented 1 year ago

Thanks again for your PR @bydawen, and sorry about the review delay.

This PR was made to make it possible to display scorm XBlocks in fullscreen when the learning MFE is enabled. I verified that it works as expected in Nutmeg.

The "native browser fullscreen" implementation makes the previous "fullscreen document" implementation kinda obsolete. However, we would still need to keep it to preserve the "fullscreen by default" feature. The problem is that this "fullscreen by default" feature will not work in the learning MFE -- because "fullscreen document" doesn't either. Since Nutmeg, we expect the learning MFE to be used by all platforms. The previous learning courseware is considered legacy. This means that "fullscreen by default" just does not work for most people. As a consequence I propose the following:

  1. Deprecate the "fullscreen by default" feature.
  2. Get rid of the "fullscreen document" implementation.

@bydawen I understand that the proposed changes may be outside of the scope of your PR. If you'd like to implement them in this PR, that's great! If not, just tell me and I'll do the changes myself.

Hello @regisb i'm sorry but may i ask you to do it? For now i'm not be able to do your proposal. I hope for your understanding.

regisb commented 1 year ago

Hello @regisb i'm sorry but may i ask you to do it?

Sure thing, no trouble at all.