Content Libraries: Advanced Component Info & OLX Editor [FC-0062] #1346

bradenmacdonald · 2024-09-28T21:05:39Z

Description

This PR adds a new "Advanced details" section to the "Details" tab of library components in the Authoring MFE. This replaces the previous "developer info" panel that was only visible in development builds. This one is visible to everyone, but hidden behind a collapsible widget.

Collapsed (default)	Expanded

It includes the OLX editor, with a warning that you need to be very careful using it:

If you make a mistake in the OLX and it's invalid, you'll get an error message like this:

I also added a list of assets, though it depends on this WIP:

TODO before merging

fix: the preview in the Preview tab does not reflect changes unless you click "Expand"
add tests?
rebase on Improve LibraryContext, convert tests to testUtils [FC-0062] #1345 and add support for read-only mode ?

Private ref: FAL-3880

openedx-webhooks · 2024-09-28T21:05:43Z

Thanks for the pull request, @bradenmacdonald!

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-09-28T21:15:07Z

Codecov Report

Attention: Patch coverage is 93.02326% with 6 lines in your changes missing coverage. Please review.

Project coverage is 92.94%. Comparing base (b957f3b) to head (413259e).

Files with missing lines	Patch %	Lines
src/generic/CodeEditor.tsx	90.90%	2 Missing ⚠️
...authoring/component-info/ComponentAdvancedInfo.tsx	95.12%	2 Missing ⚠️
...rary-authoring/component-info/ComponentPreview.tsx	60.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1346      +/-   ##
==========================================
- Coverage   92.97%   92.94%   -0.04%     
==========================================
  Files        1036     1037       +1     
  Lines       19427    19509      +82     
  Branches     4131     4084      -47     
==========================================
+ Hits        18063    18133      +70     
- Misses       1299     1313      +14     
+ Partials       65       63       -2

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

rpenido

Hi @bradenmacdonald, this is looking good!

I found a small bug and added one possible solution. Let me know what you think!

rpenido · 2024-10-07T19:06:57Z

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

+                  */}
+                </Alert>
+              )}
+              <CodeEditor readOnly={!isEditingOLX} editorRef={editorRef}>{olx}</CodeEditor>


Small bug: If you click on another component, the CodeEditor is not updating.

We can fix that making the CodeEditor rerender when we change the usageKey.

Suggested change

<CodeEditor readOnly={!isEditingOLX} editorRef={editorRef}>{olx}</CodeEditor>

<CodeEditor key={usageKey} readOnly={!isEditingOLX} editorRef={editorRef}>{olx}</CodeEditor>

rpenido · 2024-10-07T19:07:57Z

src/generic/CodeEditor.tsx

@@ -0,0 +1,55 @@
+/* eslint-disable import/prefer-default-export */


I think this can go as we are also exporting the type here.

Suggested change

/* eslint-disable import/prefer-default-export */

rpenido · 2024-10-07T19:12:50Z

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

@@ -0,0 +1,122 @@
+/* eslint-disable no-nested-ternary */
+/* eslint-disable import/prefer-default-export */


It's not directly related here, but is there a reason why we don't remove this lint rule upstream?
Personally, I also prefer named exports.

Good point. I guess I've just been lazy, but it's time to do that. openedx/eslint-config#164

rpenido · 2024-10-07T19:18:04Z

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

+      title={intl.formatMessage(messages.advancedDetailsTitle)}
+    >
+      <dl>
+        <dt><FormattedMessage {...messages.advancedDetailsUsageKey} /></dt>


Not sure if this matters for the MVP, but we are using a different style across the sidebar

Suggested change

<dt><FormattedMessage {...messages.advancedDetailsUsageKey} /></dt>

<h3 className="h5"><FormattedMessage {...messages.advancedDetailsUsageKey} /></h3>

bradenmacdonald · 2024-10-07T20:53:27Z

@rpenido Thanks! I addressed your comments.

ChrisChV

Looks good. Waiting for the review of @rpenido to merge

rpenido

LGTM 👍
Thank you for your work, @bradenmacdonald!

I tested this using the instructions from the PR
I read through the code
I checked for accessibility issues
Includes documentation

CC @ChrisChV

bradenmacdonald requested a review from a team as a code owner September 28, 2024 21:05

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

bradenmacdonald changed the title ~~Content Libraries: Advanced Component Info & OLX Editor~~ Content Libraries: Advanced Component Info & OLX Editor [FC-0062] Sep 28, 2024

bradenmacdonald added the FC Relates to an Axim Funded Contribution project label Sep 28, 2024

bradenmacdonald force-pushed the braden/advanced-component-info branch from bc07fcb to 581189a Compare September 28, 2024 21:18

bradenmacdonald mentioned this pull request Sep 29, 2024

XBlock asset support for new libraries experience (WIP) openedx/edx-platform#35557

Closed

bradenmacdonald force-pushed the braden/advanced-component-info branch from d729d02 to 15a681e Compare October 1, 2024 23:57

bradenmacdonald added 5 commits October 4, 2024 14:29

feat: Advanced Component info (not just on developer builds)

48928d5

feat: OLX editing

f9f379a

feat: List XBlock assets (static files) in the component details tab too

ea18449

feat: refresh component preview after updating OLX

d486df0

fix: i18n

2586d3e

bradenmacdonald force-pushed the braden/advanced-component-info branch from 15a681e to b342efd Compare October 5, 2024 17:22

fix: don't allow editing OLX if library is read-only

86518db

bradenmacdonald force-pushed the braden/advanced-component-info branch 3 times, most recently from 8312613 to f2b5ef7 Compare October 5, 2024 19:06

test: update tests

413259e

bradenmacdonald force-pushed the braden/advanced-component-info branch from f2b5ef7 to 413259e Compare October 5, 2024 19:24

test: increase test coverage

8854f3f

rpenido reviewed Oct 7, 2024

View reviewed changes

fix: addressed comments from code review

0a55d9e

ChrisChV approved these changes Oct 8, 2024

View reviewed changes

rpenido approved these changes Oct 8, 2024

View reviewed changes

bradenmacdonald merged commit 75f937e into openedx:master Oct 8, 2024
6 checks passed

bradenmacdonald deleted the braden/advanced-component-info branch October 8, 2024 16:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Content Libraries: Advanced Component Info & OLX Editor [FC-0062] #1346

Content Libraries: Advanced Component Info & OLX Editor [FC-0062] #1346

bradenmacdonald commented Sep 28, 2024 •

edited

Loading

openedx-webhooks commented Sep 28, 2024

codecov bot commented Sep 28, 2024 •

edited

Loading

rpenido left a comment

rpenido Oct 7, 2024 •

edited

Loading

rpenido Oct 7, 2024

rpenido Oct 7, 2024

bradenmacdonald Oct 7, 2024

rpenido Oct 7, 2024

bradenmacdonald commented Oct 7, 2024

ChrisChV left a comment

rpenido left a comment •

edited

Loading

	<CodeEditor readOnly={!isEditingOLX} editorRef={editorRef}>{olx}</CodeEditor>
	<CodeEditor key={usageKey} readOnly={!isEditingOLX} editorRef={editorRef}>{olx}</CodeEditor>

		@@ -0,0 +1,55 @@
		/* eslint-disable import/prefer-default-export */

		@@ -0,0 +1,122 @@
		/* eslint-disable no-nested-ternary */
		/* eslint-disable import/prefer-default-export */

	<dt><FormattedMessage {...messages.advancedDetailsUsageKey} /></dt>
	<h3 className="h5"><FormattedMessage {...messages.advancedDetailsUsageKey} /></h3>

Content Libraries: Advanced Component Info & OLX Editor [FC-0062] #1346

Content Libraries: Advanced Component Info & OLX Editor [FC-0062] #1346

Conversation

bradenmacdonald commented Sep 28, 2024 • edited Loading

Description

TODO before merging

openedx-webhooks commented Sep 28, 2024

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 Sep 28, 2024 • edited Loading

Codecov Report

rpenido left a comment

Choose a reason for hiding this comment

rpenido Oct 7, 2024 • edited Loading

Choose a reason for hiding this comment

rpenido Oct 7, 2024

Choose a reason for hiding this comment

rpenido Oct 7, 2024

Choose a reason for hiding this comment

bradenmacdonald Oct 7, 2024

Choose a reason for hiding this comment

rpenido Oct 7, 2024

Choose a reason for hiding this comment

bradenmacdonald commented Oct 7, 2024

ChrisChV left a comment

Choose a reason for hiding this comment

rpenido left a comment • edited Loading

Choose a reason for hiding this comment

bradenmacdonald commented Sep 28, 2024 •

edited

Loading

codecov bot commented Sep 28, 2024 •

edited

Loading

rpenido Oct 7, 2024 •

edited

Loading

rpenido left a comment •

edited

Loading