feat: add component preview to library authoring [FC-0062] #1242

rpenido · 2024-08-30T02:13:23Z

Description

This PR adds an XBlock preview to the library authoring sidebar.

More information

Part of:

Component Sidebar: Preview tab #1189

Depends on:

feat: new URL to render a v2 xblock in an iframe [FC-0062] edx-platform#35473

Testing Instructions

Checkout this branch and feat: new URL to render a v2 xblock in an iframe [FC-0062] edx-platform#35473
Open the library authoring home for a library with some components
Click on the component and check the preview on the sidebar. You can't interact with the XBlock here.
Clicking on expand should open a modal dialog with the component preview. In this preview, you can interact with the XBlock

Private ref: FAL-3801

openedx-webhooks · 2024-08-30T02:13:27Z

Thanks for the pull request, @rpenido!

What's next?

Please work through the following steps to get your changes ready for engineering review:

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

🔘 Let us know that your PR is ready for review:

Who will review my changes?

This repository is currently maintained by @openedx/2u-tnl. Tag them in a comment and let them know that your changes are ready for review.

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

codecov · 2024-08-30T20:59:11Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.33%. Comparing base (fd48fef) to head (d2edd55).
Report is 8 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1242      +/-   ##
==========================================
+ Coverage   92.31%   92.33%   +0.02%     
==========================================
  Files        1013     1020       +7     
  Lines       18733    18838     +105     
  Branches     3947     3956       +9     
==========================================
+ Hits        17293    17395     +102     
- Misses       1374     1377       +3     
  Partials       66       66

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/library-authoring/LibraryBlock/wrap.js

ChrisChV

@rpenido Looks good. Great work! Some small nits 👍

One question. The Expand button overlaps the text on some components. is that expected?

I tested this: I followed the testing instructions and I tested with multiple component types.
I read through the code and considered the security, stability and performance implications of the changes.
I tested that the UI can be used with a keyboard only (tab order, keyboard controls).
Includes tests for bugfixes and/or features added.
Includes documentation
Includes fixtures that create objects required for manual testing.

ChrisChV · 2024-09-13T02:00:43Z

src/library-authoring/LibraryBlock/LibraryBlock.tsx

+    // const frame = iframeRef.current.contentWindow;
+    // const sendReply = async (data) => {
+    //   frame?.postMessage({ ...data, replyKey }, '*');
+    // };


I think we can delete this commented code?

Fixed here: 0ed8e02

ChrisChV · 2024-09-13T02:08:09Z

src/library-authoring/data/apiHooks.ts

@@ -53,11 +53,10 @@ export const libraryAuthoringQueryKeys = {
    'libraryBlockTypes',
  ],
  xblockFields: (contentLibraryId: string, usageKey: string) => [
-    ...libraryAuthoringQueryKeys.all,


@rpenido Why is this being deleted? If the query libraryAuthoringQueryKeys.all is invalidated, then xblockFields is not invalidated.

Sorry. Not intended! This was fixed after I merged the last changes.

…-tab

rpenido · 2024-09-13T17:25:31Z

Thanks for your review @ChrisChV!
I addressed the issues you found.

One question. The Expand button overlaps the text on some components. is that expected?

Yes! I think this was the intention of the design.
We can't change it if it is an issue in the UI/UX review.

bradenmacdonald · 2024-09-13T18:15:47Z

@rpenido At a very specific browser window height, there is some weird issue at the bottom of the modal:

Most other heights it appears fine but if you resize it, you'll see this issue. Also it seems weird that there are two close buttons on the modal. Can we just remove the "bottom bar" of the modal (get rid of "Close", but keep the "X"), or was that specified by the UX team?

rpenido · 2024-09-13T18:23:21Z

weird issue at the bottom of the modal

I think this is an issue with the Paragon modal introduced after changing the overflow behavior.
Tomorrow, I'll check other Paragon components to remove the Close button and see if it reduces the chance of this happening again.

bradenmacdonald

Looks good! Just some minor comments. Ping me on Mattermost if/when you want me to hit merge.

src/library-authoring/LibraryBlock/LibraryBlock.tsx

bradenmacdonald · 2024-09-13T18:34:58Z

src/library-authoring/component-info/ComponentPreview.tsx

+        >
+          {intl.formatMessage(messages.previewExpandButtonTitle)}
+        </Button>
+        <LibraryBlock usageKey={usageKey} />


In the future, it would be awesome to use something like react-reverse-portal to "re-parent" the <LibraryBlock> so that it moves from the sidebar to the modal instead of being copied into the modal. Then for example, you could start playing a video in the sidebar and "Expand" it and it would still be playing in the modal. Please mention that to the UX team as a possible future improvement after this merges (and post MVP).

Sure! I will mention that!

Co-authored-by: Braden MacDonald <[email protected]>

rpenido · 2024-09-14T13:56:24Z

Thank you for your review @bradenmacdonald.
I removed the footer close button.
I tried other modal components, but the issue in Paragon affects all of them. 😞

I think this is ready to merge!

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Aug 30, 2024

rpenido force-pushed the rpenido/fal-3801-component-sidebar-preview-tab branch 4 times, most recently from 9c73221 to 46f4dcb Compare August 30, 2024 19:10

rpenido added 3 commits August 30, 2024 17:06

feat: add component preview to library authoring

2a03de9

fix: change view shape

a9023ba

fix: types and cleanup

70cd2ef

rpenido force-pushed the rpenido/fal-3801-component-sidebar-preview-tab branch from 46f4dcb to 70cd2ef Compare August 30, 2024 20:33

fix: eslint

1c509b6

rpenido force-pushed the rpenido/fal-3801-component-sidebar-preview-tab branch from 6f1d198 to 1c509b6 Compare August 30, 2024 20:49

rpenido added 5 commits September 3, 2024 16:38

feat: add expand button and modal

ab56de4

fix: minor style adjust

68de1a1

fix: adjust style and remove hardcoded url

31c6ccb

fix: add istanbul ignore in the code that depends on the frame

ffa566f

fix: eslint

30f60cf

rpenido commented Sep 7, 2024

View reviewed changes

src/library-authoring/LibraryBlock/wrap.js Outdated Show resolved Hide resolved

bradenmacdonald and others added 2 commits September 11, 2024 14:11

feat: simpler xblock embedding

990815c

fix: design issues and removing overlay from modal overview

395e6f6

rpenido force-pushed the rpenido/fal-3801-component-sidebar-preview-tab branch from 0c8c047 to 4c4905d Compare September 11, 2024 18:53

rpenido mentioned this pull request Sep 11, 2024

feat: new URL to render a v2 xblock in an iframe [FC-0062] openedx/edx-platform#35473

Merged

rpenido changed the title ~~feat: add component preview to library authoring~~ feat: add component preview to library authoring [FC-0062] Sep 11, 2024

rpenido force-pushed the rpenido/fal-3801-component-sidebar-preview-tab branch from 4c4905d to a9847d2 Compare September 11, 2024 19:14

refactor: code cleanup

0e9b907

rpenido force-pushed the rpenido/fal-3801-component-sidebar-preview-tab branch from a9847d2 to 0e9b907 Compare September 11, 2024 19:56

rpenido marked this pull request as ready for review September 12, 2024 21:09

rpenido requested a review from a team as a code owner September 12, 2024 21:09

ChrisChV approved these changes Sep 13, 2024

View reviewed changes

rpenido added 2 commits September 13, 2024 13:20

Merge branch 'master' into rpenido/fal-3801-component-sidebar-preview…

a70323d

…-tab

fix: remove unused code and add localization

0ed8e02

rpenido requested a review from bradenmacdonald September 13, 2024 17:25

bradenmacdonald approved these changes Sep 13, 2024

View reviewed changes

bradenmacdonald mentioned this pull request Sep 13, 2024

Support static assets in the Learning Core runtime openedx/edx-platform#34737

Closed

rpenido and others added 2 commits September 14, 2024 10:46

docs: add comments from review

a5bd650

Co-authored-by: Braden MacDonald <[email protected]>

fix: remove footer close button

d2edd55

bradenmacdonald merged commit 121ced4 into openedx:master Sep 14, 2024
7 checks passed

bradenmacdonald deleted the rpenido/fal-3801-component-sidebar-preview-tab branch September 14, 2024 17:03

mphilbrick211 added the FC Relates to an Axim Funded Contribution project label Sep 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add component preview to library authoring [FC-0062] #1242

feat: add component preview to library authoring [FC-0062] #1242

rpenido commented Aug 30, 2024 •

edited

Loading

openedx-webhooks commented Aug 30, 2024 •

edited

Loading

codecov bot commented Aug 30, 2024 •

edited

Loading

ChrisChV left a comment

ChrisChV Sep 13, 2024

rpenido Sep 13, 2024

ChrisChV Sep 13, 2024

rpenido Sep 13, 2024

rpenido commented Sep 13, 2024

bradenmacdonald commented Sep 13, 2024

rpenido commented Sep 13, 2024

bradenmacdonald left a comment

bradenmacdonald Sep 13, 2024

rpenido Sep 14, 2024

rpenido commented Sep 14, 2024

feat: add component preview to library authoring [FC-0062] #1242

feat: add component preview to library authoring [FC-0062] #1242

Conversation

rpenido commented Aug 30, 2024 • edited Loading

Description

More information

Testing Instructions

openedx-webhooks commented Aug 30, 2024 • edited Loading

What's next?

🔘 Get product approval

🔘 Provide context

🔘 Get a green build

🔘 Let us know that your PR is ready for review:

Who will review my changes?

Where can I find more information?

When can I expect my changes to be merged?

codecov bot commented Aug 30, 2024 • edited Loading

Codecov Report

ChrisChV left a comment

Choose a reason for hiding this comment

ChrisChV Sep 13, 2024

Choose a reason for hiding this comment

rpenido Sep 13, 2024

Choose a reason for hiding this comment

ChrisChV Sep 13, 2024

Choose a reason for hiding this comment

rpenido Sep 13, 2024

Choose a reason for hiding this comment

rpenido commented Sep 13, 2024

bradenmacdonald commented Sep 13, 2024

rpenido commented Sep 13, 2024

bradenmacdonald left a comment

Choose a reason for hiding this comment

bradenmacdonald Sep 13, 2024

Choose a reason for hiding this comment

rpenido Sep 14, 2024

Choose a reason for hiding this comment

rpenido commented Sep 14, 2024

rpenido commented Aug 30, 2024 •

edited

Loading

openedx-webhooks commented Aug 30, 2024 •

edited

Loading

codecov bot commented Aug 30, 2024 •

edited

Loading