Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: plugin slot implementation #465

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
19 changes: 5 additions & 14 deletions src/containers/CoursesPanel/CourseList/index.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React from 'react';
import PropTypes from 'prop-types';

import { Pagination } from '@openedx/paragon';
import {
Expand All @@ -8,11 +7,13 @@ import {
import CourseCard from 'containers/CourseCard';

import { useIsCollapsed } from './hooks';
import { useCourseListData } from '../hooks';

export const CourseList = ({
filterOptions, setPageNumber, numPages, showFilters, visibleList,
}) => {
export const CourseList = () => {
const isCollapsed = useIsCollapsed();
const {
filterOptions, setPageNumber, numPages, showFilters, visibleList,
} = useCourseListData();
return (
<>
{showFilters && (
Expand All @@ -38,14 +39,4 @@ export const CourseList = ({
);
};

CourseList.propTypes = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no issue with removing propTypes, but this change is not obviously related to the plugin slot refactor. Would you mind adding it to a "changelog" in the PR comment?

Copy link
Member Author

@MaxFrank13 MaxFrank13 Oct 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I went back on this change. Originally I thought it made sense to separate the hook call into it's own component, but looking back at this, I think it's better to pass the data in as pluginProps. This way any custom components (like the example) don't need to call for the data themselves.

And yet, if a consumer wanted to do that (say they had a private API they wanted to get their course data from), they still can.

The only thing that changed here now is that courseListData is passed as one object instead of each value within courseListData being passed as individual props.

showFilters: PropTypes.bool.isRequired,
// eslint-disable-next-line react/forbid-prop-types
visibleList: PropTypes.arrayOf(PropTypes.object).isRequired,
// eslint-disable-next-line react/forbid-prop-types
filterOptions: PropTypes.object.isRequired,
numPages: PropTypes.number.isRequired,
setPageNumber: PropTypes.func.isRequired,
};

export default CourseList;
14 changes: 11 additions & 3 deletions src/containers/CoursesPanel/CourseList/index.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,16 @@ import { shallow } from '@edx/react-unit-test-utils';

import { useIsCollapsed } from './hooks';
import CourseList from '.';
import { useCourseListData } from '../hooks';

jest.mock('./hooks', () => ({
useIsCollapsed: jest.fn(),
}));

jest.mock('../hooks', () => ({
useCourseListData: jest.fn(),
}));

jest.mock('containers/CourseCard', () => 'CourseCard');
jest.mock('containers/CourseFilterControls', () => ({
ActiveCourseFilters: 'ActiveCourseFilters',
Expand All @@ -22,9 +27,12 @@ describe('CourseList', () => {
};
useIsCollapsed.mockReturnValue(false);

const createWrapper = (courseListData = defaultCourseListData) => (
shallow(<CourseList {...courseListData} />)
);
const createWrapper = (courseListData = defaultCourseListData) => {
useCourseListData.mockReturnValue(courseListData);
return (
shallow(<CourseList />)
);
};

describe('no courses or filters', () => {
test('snapshot', () => {
Expand Down
18 changes: 2 additions & 16 deletions src/containers/CoursesPanel/__snapshots__/index.test.jsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,7 @@ exports[`CoursesPanel no courses snapshot 1`] = `
<CourseFilterControls />
</div>
</div>
<PluginSlot
id="no_courses_view"
>
<NoCoursesView />
</PluginSlot>
<NoCoursesViewSlot />
</div>
`;

Expand All @@ -44,16 +40,6 @@ exports[`CoursesPanel with courses snapshot 1`] = `
<CourseFilterControls />
</div>
</div>
<PluginSlot
id="course_list"
>
<CourseList
filterOptions={{}}
numPages={1}
setPageNumber={[MockFunction setPageNumber]}
showFilters={false}
visibleList={[]}
/>
</PluginSlot>
<CourseListSlot />
</div>
`;
18 changes: 4 additions & 14 deletions src/containers/CoursesPanel/index.jsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
import React from 'react';

import { PluginSlot } from '@openedx/frontend-plugin-framework';
import { useIntl } from '@edx/frontend-platform/i18n';

import { reduxHooks } from 'hooks';
import {
CourseFilterControls,
} from 'containers/CourseFilterControls';
import NoCoursesView from './NoCoursesView';

import CourseList from './CourseList';
import CourseListSlot from 'plugin-slots/CourseListSlot';
import NoCoursesViewSlot from 'plugin-slots/NoCoursesViewSlot';

import { useCourseListData } from './hooks';

Expand All @@ -35,17 +33,9 @@ export const CoursesPanel = () => {
</div>
</div>
{hasCourses ? (
<PluginSlot
id="course_list"
>
<CourseList {...courseListData} />
</PluginSlot>
<CourseListSlot />
) : (
<PluginSlot
id="no_courses_view"
>
<NoCoursesView />
</PluginSlot>
<NoCoursesViewSlot />
MaxFrank13 marked this conversation as resolved.
Show resolved Hide resolved
)}
</div>
);
Expand Down
4 changes: 2 additions & 2 deletions src/containers/Dashboard/DashboardLayout.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import PropTypes from 'prop-types';

import { Container, Col, Row } from '@openedx/paragon';

import WidgetSidebar from '../WidgetContainers/WidgetSidebar';
import WidgetSidebarSlot from 'plugin-slots/WidgetSidebarSlot';

import hooks from './hooks';

Expand Down Expand Up @@ -42,7 +42,7 @@ export const DashboardLayout = ({ children }) => {
</Col>
<Col {...columnConfig.sidebar} className="sidebar-column">
{!isCollapsed && (<h2 className="course-list-title">&nbsp;</h2>)}
<WidgetSidebar />
<WidgetSidebarSlot />
</Col>
</Row>
</Container>
Expand Down

This file was deleted.

23 changes: 0 additions & 23 deletions src/containers/WidgetContainers/WidgetSidebar/index.jsx

This file was deleted.

18 changes: 0 additions & 18 deletions src/containers/WidgetContainers/WidgetSidebar/index.test.jsx

This file was deleted.

43 changes: 43 additions & 0 deletions src/plugin-slots/CourseListSlot/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# Course List Slot

### Slot ID: `course_list_slot`

## Description

This slot is used for replacing or adding content around the `CourseList` component. The `CourseListSlot` is only rendered if the learner has enrolled in at least one course.

## Example

The space will show the `CourseList` component by default. This can be disabled in the configuration with the `keepDefault` boolean.

![Screenshot of the CourseListSlot](./images/course_list_slot.png)

Setting the MFE's `env.config.jsx` to the following will replace the default experience with a `CustomCourseList` component.

![Screenshot of a custom course list](./images/custom_course_list.png)

```js
import { DIRECT_PLUGIN, PLUGIN_OPERATIONS } from '@openedx/frontend-plugin-framework';
import { CustomCourseList } from '<package-that-exports-your-component>'
MaxFrank13 marked this conversation as resolved.
Show resolved Hide resolved

const config = {
pluginSlots: {
course_list_slot: {
keepDefault: false,
plugins: [
{
op: ops.Insert,
widget: {
id: 'custom_course_list',
type: DIRECT_PLUGIN,
priority: 60,
RenderWidget: CustomCourseList,
},
},
],
},
},
}

export default config;
```
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
11 changes: 11 additions & 0 deletions src/plugin-slots/CourseListSlot/index.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import React from 'react';

import { PluginSlot } from '@openedx/frontend-plugin-framework';
import CourseList from 'containers/CoursesPanel/CourseList';

export const CourseListSlot = () => (
<PluginSlot id="course_list_slot">
<CourseList />
</PluginSlot>
);
export default CourseListSlot;
43 changes: 43 additions & 0 deletions src/plugin-slots/NoCoursesViewSlot/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# No Courses View Slot

### Slot ID: `no_courses_view_slot`

## Description

This slot is used for replacing or adding content around the `NoCoursesView` component. The `NoCoursesViewSlot` only renders if the learner has not yet enrolled in any courses.

## Example

The space will show the `NoCoursesView` by default. This can be disabled in the configuration with the `keepDefault` boolean.

![Screenshot of the no courses view](./images/no_courses_view_slot.png)

Setting the MFE's `env.config.jsx` to the following will replace the default experience with a `CustomNoCoursesCTA` component.

![Screenshot of a custom no courses view](./images/custom_no_courses_view.png)

```js
import { DIRECT_PLUGIN, PLUGIN_OPERATIONS } from '@openedx/frontend-plugin-framework';
import { CustomSidebarPanel } from 'package-that-exports-your-component';
MaxFrank13 marked this conversation as resolved.
Show resolved Hide resolved

const config = {
pluginSlots: {
no_courses_view_slot: {
keepDefault: false,
plugins: [
{
op: ops.Insert,
widget: {
id: 'custom_no_courses_CTA',
type: DIRECT_PLUGIN,
priority: 60,
RenderWidget: CustomSidebarPanel,
},
},
],
},
},
}

export default config;
```
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
12 changes: 12 additions & 0 deletions src/plugin-slots/NoCoursesViewSlot/index.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import React from 'react';

import { PluginSlot } from '@openedx/frontend-plugin-framework';
import NoCoursesView from 'containers/CoursesPanel/NoCoursesView';

export const NoCoursesViewSlot = () => (
<PluginSlot id="no_courses_view_slot">
<NoCoursesView />
</PluginSlot>
);

export default NoCoursesViewSlot;
3 changes: 3 additions & 0 deletions src/plugin-slots/README.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# `frontend-app-learner-dashboard` Plugin Slots

* [`footer_slot`](./FooterSlot/)
* [`widget_sidebar_slot`](./WidgetSidebarSlot/)
* [`course_list_slot`](./CourseListSlot/)
* [`no_courses_view_slot`](./NoCoursesViewSlot/)
43 changes: 43 additions & 0 deletions src/plugin-slots/WidgetSidebarSlot/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# Widget Sidebar Slot

### Slot ID: `widget_sidebar_slot`

## Description

This slot is used for adding content to the right-hand sidebar.

## Example

The space will show the `LookingForChallengeWidget` by default. This can be disabled in the configuration with the `keepDefault` boolean.

![Screenshot of the widget sidebar](./images/looking_for_challenge_widget.png)

Setting the MFE's `env.config.jsx` to the following will replace the default experience with a `CustomSidebarPanel` component.

![Screenshot of a custom call-to-action in the sidebar](./images/custom_CTA_sidebar.png)
MaxFrank13 marked this conversation as resolved.
Show resolved Hide resolved

```js
import { DIRECT_PLUGIN, PLUGIN_OPERATIONS } from '@openedx/frontend-plugin-framework';
import { CustomSidebarPanel } from 'package-that-exports-your-component';
MaxFrank13 marked this conversation as resolved.
Show resolved Hide resolved

const config = {
pluginSlots: {
widget_sidebar_slot: {
keepDefault: false,
plugins: [
{
op: ops.Insert,
widget: {
id: 'custom_sidebar_panel',
type: DIRECT_PLUGIN,
priority: 60,
RenderWidget: CustomSidebarPanel,
},
},
],
},
},
}

export default config;
```
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
13 changes: 13 additions & 0 deletions src/plugin-slots/WidgetSidebarSlot/index.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import React from 'react';

import { PluginSlot } from '@openedx/frontend-plugin-framework';
import LookingForChallengeWidget from 'widgets/LookingForChallengeWidget';

// eslint-disable-next-line arrow-body-style
export const WidgetSidebar = () => (
MaxFrank13 marked this conversation as resolved.
Show resolved Hide resolved
<PluginSlot id="widget_sidebar_plugin_slot">

Check warning on line 8 in src/plugin-slots/WidgetSidebarSlot/index.jsx

View check run for this annotation

Codecov / codecov/patch

src/plugin-slots/WidgetSidebarSlot/index.jsx#L8

Added line #L8 was not covered by tests
<LookingForChallengeWidget />
</PluginSlot>
);

export default WidgetSidebar;
MaxFrank13 marked this conversation as resolved.
Show resolved Hide resolved
Loading