feat: [FC-86] Added course overview for Course About page#47
Conversation
|
Thanks for the pull request, @PKulkoRaccoonGang! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo 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:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere 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:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #47 +/- ##
==========================================
+ Coverage 98.57% 98.77% +0.19%
==========================================
Files 89 96 +7
Lines 845 981 +136
Branches 129 160 +31
==========================================
+ Hits 833 969 +136
Misses 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
47acddc to
37d0560
Compare
src/course-about/messages.ts
Outdated
| }, | ||
| noCourseOverview: { | ||
| id: 'catalog.course-about.no-course-overview', | ||
| defaultMessage: 'No course overview added', |
There was a problem hiding this comment.
I don't think we should have this message (and especially not show it to learners).
The legacy page doesn't have this
so we should match that.
I messed around locally a bit to see what a "don't show the card when we don't have anything to put in it" version of this could look like.
diff --git a/src/course-about/course-overview/index.tsx b/src/course-about/course-overview/index.tsx
index 1eaf333..48ed156 100644
--- a/src/course-about/course-overview/index.tsx
+++ b/src/course-about/course-overview/index.tsx
@@ -4,7 +4,6 @@ import {
import { getConfig } from '@edx/frontend-platform';
import { useIntl } from '@edx/frontend-platform/i18n';
import { getAuthenticatedUser } from '@edx/frontend-platform/auth';
-import classNames from 'classnames';
import messages from '../messages';
import type { CourseOverviewProps } from './types';
@@ -19,6 +18,24 @@ export const CourseOverview = ({ overviewData, courseId }: CourseOverviewProps)
const processedOverviewData = processOverviewContent(overviewData, getConfig().LMS_BASE_URL);
const hasOverviewContent = processedOverviewData.trim().length > 0;
+ if (!hasOverviewContent) {
+ if (!isGlobalStaff) {
+ return undefined;
+ }
+
+ return (
+ <Button
+ as="a"
+ size="sm"
+ block={isExtraSmall}
+ variant="outline-primary"
+ href={`${getConfig().STUDIO_BASE_URL}/settings/details/${courseId}`}
+ >
+ {intl.formatMessage(messages.viewAboutPageInStudio)}
+ </Button>
+ );
+ }
+
return (
<Container className="px-0">
<Card>
@@ -40,14 +57,10 @@ export const CourseOverview = ({ overviewData, courseId }: CourseOverviewProps)
/>
)}
<Card.Section>
- {hasOverviewContent ? (
+ {
/* eslint-disable-next-line react/no-danger */
<div dangerouslySetInnerHTML={{ __html: processedOverviewData }} />
- ) : (
- <div className={classNames('text-center', isGlobalStaff ? 'mb-5.5' : 'my-5.5')}>
- <p className="m-0">{intl.formatMessage(messages.noCourseOverview)}</p>
- </div>
- )}
+ }
</Card.Section>
</Card>
</Container>
There was a problem hiding this comment.
This change and your suggestions regarding the card have been added to the PR. Thanks!
There was a problem hiding this comment.
Using an ActionRow looks like a great solution! Thanks!
b885c5f to
418065c
Compare
418065c to
b02e37a
Compare
brian-smith-tcril
left a comment
There was a problem hiding this comment.
Overall this looks great! Couple small comments/suggestions but nothing major.
Thanks for putting this together!
src/course-about/messages.ts
Outdated
| }, | ||
| noCourseOverview: { | ||
| id: 'catalog.course-about.no-course-overview', | ||
| defaultMessage: 'No course overview added', |
There was a problem hiding this comment.
Using an ActionRow looks like a great solution! Thanks!
98018bc to
d8c977d
Compare
brian-smith-tcril
left a comment
There was a problem hiding this comment.
Thanks for putting all of this together! I just pulled down the latest version and tested locally and it looks wonderful!
LGTM!

Description
Added course overview for Course About page.
Useful information
Initial setup
tutor-mfeplugin from the draft PR that adds support for Catalog MFE.http://apps.local.openedx.io:1998/catalog/courses/<course_id>/aboutHow Has This Been Tested?
http://apps.local.openedx.io:1998/catalog/courses/<course_id>/about).