fix: make iframe wrapper take all vieport width

[0x29a]

Description

Resolves openedx/xblock-drag-and-drop-v2#312. Probably it was introduced in #27.

Consider how the block interface behaves in the legacy courseware (Lilac):

2023-04-02.16-46-58.mp4

On the video you can see that when the viewport reaches 480px, the block resets content width and makes it scrollable. This was implemented this way intentionally in this PR: openedx/xblock-drag-and-drop-v2#135.

Here is how it behaves in this MFE:

2023-04-02.16-49-45.mp4

Something weird happens when the viewport width reaches 574px. The iframe shrinks, making XBlock very hard to use.

With changes from this PR:

2023-04-02.16-54-47.mp4

It basically removes / compensates padding from the iframe's parent containers and sets 100% width for the iframe container itself. You can see that the block now behaves as in the legacy courseware.

Testing instructions

1. Switch $(DEVSTACK_ROOT)/frontend-app-learning to this branch.
2. Verify that xblock-drag-and-drop-v2's content behaves exactly as on the third video in different browsers.

Misc notes

You may notice that this icon is shifted, when it's supposed to stay next to the navigation:

It's not related to this PR's changes, and probably has been introduced here: #414

Also see this comment with a diagram where this icon should be.

private-ref

[openedx-webhooks]

Thanks for the pull request, @0x29a! 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.

[codecov]

Codecov Report

Patch coverage has no change and project coverage change: +0.04 🎉

Comparison is base (2d132f1) 87.67% compared to head (303d173) 87.71%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1094      +/-   ##
==========================================
+ Coverage   87.67%   87.71%   +0.04%     
==========================================
  Files         264      264              
  Lines        4453     4453              
  Branches     1110     1110              
==========================================
+ Hits         3904     3906       +2     
+ Misses        535      533       -2     
  Partials       14       14              

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[Agrendalath]

👍

• I tested this: checked that the DnDv2, Video, Discussion, and CAPA (without images) XBlocks are scaled correctly for mobile resolutions.
• I read through the code
• I checked for accessibility issues
• Includes documentation
• I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository: n/a

[Agrendalath]

@brian-smith-tcril, would you like to review this as the CC of this repo?

[Agrendalath]

@0x29a, could you please change the first sentence of this PR's description to Resolves openedx/xblock-drag-and-drop-v2#312.? I am curious if this will work for a different repository.

[0x29a]

@0x29a, could you please change the first sentence of this PR's description to Resolves openedx/xblock-drag-and-drop-v2#312.? I am curious if this will work for a different repository.

Done, @Agrendalath. Looks like it doesn't work.

Edit: wait, it should close the issue when this merged, right?

[brian-smith-tcril]

This looks like a solid solution code-wise to me!

Has anyone looked at this from a design/product perspective yet?

[Agrendalath]

@brian-smith-tcril,

Has anyone looked at this from a design/product perspective yet?

No. Who would be the best person to ping here?

[brian-smith-tcril]

@brian-smith-tcril,

Has anyone looked at this from a design/product perspective yet?

No. Who would be the best person to ping here?

I'm not sure. I asked in the #wg-frontend channel on slack, I'm hoping someone there might have an idea.

[itsjeyd]

@Agrendalath @brian-smith-tcril Also looping in @jmakowski1123 and @ProductRyan for help with scheduling this PR for product review.

CC @0x29a

[Agrendalath]

@0x29a,

Done, @Agrendalath. Looks like it doesn't work.

It seems to be working. The PR is linked in the "Development" section of the issue.

Edit: wait, it should close the issue when this merged, right?

Correct.

[itsjeyd]

Hi @jmakowski1123, do you think this will need a round of product review?

[itsjeyd]

Hey, @Daniel-hershel, would you be available to review this PR from product perspective?

[Daniel-hershel]

Hi @itsjeyd this PR, and other PRs for Studio should actually go to Brad Brown for review who is the PM for studio. Thanks!

[jmakowski1123]

Apologies @itsjeyd, I completely missed this ticket when it came through triage. My mistake. Here's the Feature Ticket for tracking product review: openedx/platform-roadmap#268

[itsjeyd]

No problem @jmakowski1123, and thanks for the feature ticket.

@Daniel-hershel Noted re: assigning Studio-related PRs to Brad Brown (@cablaa77), I've done that now for this one.

[itsjeyd]

Hey @cablaa77, this PR is ready for product review.

CC @brian-smith-tcril @0x29a @Agrendalath

[itsjeyd]

Hi @cablaa77, just checking in to see if you had any updates about when product review for this PR might start?

[Daniel-hershel]

@itsjeyd I see that this is still on the Aurora board, is there a way to get it moved on to the TNL board? Thanks

[itsjeyd]

@Daniel-hershel By Aurora board do you mean this tab of the Contributions board? It is possible to move the PR card from that tab to the one for TNL, yep :)

I've done that now; let me know if you had something else in mind?

And for future reference, is it just this PR that is owned by TNL or has ownership for the whole repo changed from Aurora to TNL?

CC @mphilbrick211

[itsjeyd]

For reference, product review is in progress now. @ali-hugo is handling it, see openedx/platform-roadmap#268.

[Agrendalath]

@0x29a, could you please rebase this to rerun the CI? It looks like the tests action is stuck.

@brian-smith-tcril, we have the product approval for this. Can you merge it once the CI passes?

[0x29a]

Done, @Agrendalath.

[itsjeyd]

@Agrendalath Just FYI, I'm re-adding the product review label here. In the context of OSPR management, @mphilbrick211 and I use board statuses to track where PRs are at in the review process. There is a dedicated status for PRs that are currently undergoing product review. This PR is no longer in that column, indicating that product review is complete.

The product review label is a permanent marker that indicates that a PR includes changes that affect product (= end user experience). So it should not be removed once it's added to a PR by @jmakowski1123.

I hope that helps, let me know if you need more info on the points above :)

[itsjeyd]

FTR, we're currently still waiting on an official answer about whether @ali-hugo's product approval is binding.

If it's not, another round of product review will be needed.

So we're not in a position to merge this PR just yet.

[Agrendalath]

@itsjeyd,

FTR, we're currently still waiting on an official answer about whether @ali-hugo's product approval openedx/platform-roadmap#268 (comment).

I see; thanks for explaining. I thought it was a binding decision.

I hope that helps, let me know if you need more info on the points above :)

It's very odd that this PR is still blocked on the product review for over 3 months. It resolves the issue reported by 2U (openedx/xblock-drag-and-drop-v2#312); in March, they asked if we could look into it as a part of the maintainership program (but it turned out to be an issue with the whole MFE, as we found out during the implementation). I'm just curious why a ticket that improves the UX (by making the MFE responsive; it does not change anything else) needs the product review. I believe having transparency here would be helpful in understanding the whole process. I saw your comments in this thread (including the most recent one); thank you for looking into improving this situation!

Also, would you mind taking a quick look at openedx/edx-platform#32570 as the OSPR manager? This one has been stuck for three weeks without a decision if it requires a product review.

[ali-hugo]

FTR, we're currently still waiting on an official answer about whether @ali-hugo's product approval is binding.

@itsjeyd @Agrendalath I asked about this in the Core Product Working Group meeting yesterday. Currently, product reviews are only binding if performed by the Product Owner. However, it seems that everyone is in agreement that this process is not working and needs to be improved. There are apparently conversations going on about how to improve the process - I'm just not sure where or by whom! (@itsjeyd Are you involved in these discussions?)

In the meantime, I guess we just have to keep bugging the Product Owner. Do either of you know how to see who that is?

[itsjeyd]

@Agrendalath

I'm just curious why a ticket that improves the UX (by making the MFE responsive; it does not change anything else) needs the product review.

The original process for identifying PRs needing product review was that @jmakowski1123 would go over incoming OSPRs (i.e., PRs in the Needs Triage column on the Contributions board) and:

1. Add the product review label to any relevant PRs.
2. Create a platform-roadmap ticket (like [Product Pull Request] fix: make iframe wrapper take all vieport width platform-roadmap#268) for each new feature being introduced via one or more OSPRs.

As far as I can tell, the process ended up being a bit different for this PR -- it didn't have a roadmap ticket initially, and the conversation about the need for product review started when @brian-smith-tcril posted this comment.

It's very odd that this PR is still blocked on the product review for over 3 months. It resolves the issue reported by 2U (openedx/xblock-drag-and-drop-v2#312); in March, they asked if we could look into it as a part of the maintainership program (...).

Do you remember who you were talking to at the time? Maybe they could help us expedite the process here :)

--

@ali-hugo

I asked about this in the Core Product Working Group meeting yesterday. Currently, product reviews are only binding if performed by the Product Owner.

Thanks, that's good to know. CC @mphilbrick211

However, it seems that everyone is in agreement that this process is not working and needs to be improved. There are apparently conversations going on about how to improve the process - I'm just not sure where or by whom! (@itsjeyd Are you involved in these discussions?)

I'm not aware of or currently involved in any conversations about improving the product review process, specifically.

The main conversation that I've been contributing to is the one on Pull Request Review Delays. It has been mostly focusing on engineering review until now (although it some of the ideas/comments from that thread could probably be generalized and apply to the product review process as well).

In the meantime, I guess we just have to keep bugging the Product Owner. Do either of you know how to see who that is?

My understanding from this comment is that @Daniel-hershel is handling product reviews for Aurora repos. I'm not completely sure if that's the same as holding the Product Owner role, though.

Either way he did mention above that:

... this PR, and other PRs for Studio should actually go to Brad Brown (@cablaa77) for review who is the PM for studio.

[itsjeyd]

@Agrendalath

Also, would you mind taking a quick look at openedx/edx-platform#32570 as the OSPR manager? This one has been stuck for three weeks without a decision if it requires a product review.

@mphilbrick211 is handling OSPR management for edx-platform PRs, so I'm tagging her here for awareness and follow-up :)

[Agrendalath]

@itsjeyd,

Do you remember who you were talking to at the time? Maybe they could help us expedite the process here :)

Yeah, I asked for help with this immediately after seeing the comment you linked 😕

[cablaa77]

I am approving this PR. Apologies for the amount of time it has taken to respond.

[Agrendalath]

Thanks, @cablaa77!

@brian-smith-tcril, we have everything to move this forward now.

[itsjeyd]

@cablaa77 Thanks a lot!