Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -251,15 +251,16 @@ Wrap
''''

Unlike Modify, the Wrap operation adds a React component around the widget, and a single widget can receive more than
one wrap operation. Each wrapper function takes in a ``component`` and ``id`` prop.
one wrap operation. Each wrapper function takes in a ``component``, ``id`` and ``pluginProps`` prop.

.. code-block::

const wrapWidget = ({ component, idx }) => (
const wrapWidget = ({ component, idx, pluginProps }) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

This was wrong before the PR, but do you mind fixing it? idx is not referenced at all by createElement.

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure if I got it..
The idx is being used as key here:

(component, wrapper, idx) => React.createElement(wrapper, { component, key: idx }),

<div className="bg-warning" data-testid={`wrapper${idx + 1}`} key={idx}>
<p>This is a wrapper component that is placed around the widget.</p>
{component}
<p>With this wrapper, you can add anything before or after the widget.</p>
<p>You can use the pluginProps to pass in any additional props to the wrapper: {pluginProps.prop1}</p>
</div>
);

Expand All @@ -272,6 +273,9 @@ one wrap operation. Each wrapper function takes in a ``component`` and ``id`` pr
{
op: PLUGIN_OPERATIONS.Wrap,
widgetId: 'default_contents',
pluginProps: {
prop1: 'prop1',
},
wrapper: wrapWidget,
}

Expand Down
1 change: 1 addition & 0 deletions src/plugins/PluginSlot.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ function BasePluginSlot({
wrapComponent(
() => container,
pluginConfig.wrappers,
pluginProps,
),
);
} else {
Expand Down
39 changes: 36 additions & 3 deletions src/plugins/PluginSlot.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import React from 'react';
import '@testing-library/jest-dom';
import classNames from 'classnames';
import { render, fireEvent } from '@testing-library/react';
import { render, fireEvent, within } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { logError } from '@edx/frontend-platform/logging';
import { IntlProvider } from '@edx/frontend-platform/i18n';
Expand Down Expand Up @@ -78,14 +78,15 @@ function DefaultContents({ className, onClick, ...rest }) {
);
}

function PluginSlotWrapper({ slotOptions, children }) {
function PluginSlotWrapper({ slotOptions, children, pluginProps }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

In this file, please create a new set of tests for when there are no pluginProps specified.

Copy link
Author

Choose a reason for hiding this comment

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

Done: 15bb3db

return (
<IntlProvider locale="en">
<PluginSlot
id="test-slot"
data-testid="test-slot-id"
as="div"
slotOptions={slotOptions}
pluginProps={pluginProps}
>
{children}
</PluginSlot>
Expand Down Expand Up @@ -182,9 +183,39 @@ describe('PluginSlot', () => {
{
op: PLUGIN_OPERATIONS.Wrap,
widgetId: 'default_contents',
wrapper: ({ component }) => (
wrapper: ({ component, pluginProps }) => (
<div data-testid="custom-wrapper">
{component}
<div data-testid="custom-wrapper-props">
{pluginProps?.prop1 && `This is a wrapper with ${pluginProps?.prop1}.`}
</div>
</div>
),
},
],
keepDefault: true,
});

const { getByTestId } = render(<TestPluginSlot pluginProps={{ prop1: 'prop1' }} />);
const customWrapper = getByTestId('custom-wrapper');
const defaultContent = getByTestId('default_contents');
expect(customWrapper).toContainElement(defaultContent);
const pluginProps = within(customWrapper).getByTestId('custom-wrapper-props');
expect(pluginProps).toHaveTextContent('This is a wrapper with prop1.');
});

it('should wrap a Plugin when using the "wrap" operation without passing props', () => {
usePluginSlot.mockReturnValueOnce({
plugins: [
{
op: PLUGIN_OPERATIONS.Wrap,
widgetId: 'default_contents',
wrapper: ({ component, pluginProps }) => (
<div data-testid="custom-wrapper">
{component}
<div data-testid="custom-wrapper-no-props">
{`This is a wrapper without props: ${JSON.stringify(pluginProps)}`}
</div>
</div>
),
},
Expand All @@ -196,6 +227,8 @@ describe('PluginSlot', () => {
const customWrapper = getByTestId('custom-wrapper');
const defaultContent = getByTestId('default_contents');
expect(customWrapper).toContainElement(defaultContent);
const pluginProps = within(customWrapper).getByTestId('custom-wrapper-no-props');
expect(pluginProps).toHaveTextContent('This is a wrapper without props: {}');
});

it('should not render a widget if the Hide operation is applied to it', () => {
Expand Down
5 changes: 3 additions & 2 deletions src/plugins/data/utils.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,14 @@ export const organizePlugins = (defaultContents, plugins) => {
*
* @param {Function} renderComponent - Function that returns JSX (i.e. React Component)
* @param {Array} wrappers - Array of components that each use a "component" prop to render the wrapped contents
* @params {object} pluginProps - Props defined in the PluginSlot
* @returns {React.ReactElement} - The plugin component wrapped by any number of wrappers provided.
*/
export const wrapComponent = (renderComponent, wrappers) => wrappers.reduce(
export const wrapComponent = (renderComponent, wrappers, pluginProps) => wrappers.reduce(
// Disabled lint because currently we don't have a unique identifier for this
// The "component" and "wrapper" are both functions
// eslint-disable-next-line react/no-array-index-key
(component, wrapper, idx) => React.createElement(wrapper, { component, key: idx }),
(component, wrapper, idx) => React.createElement(wrapper, { component, key: idx, pluginProps }),
renderComponent(),
);

Expand Down
17 changes: 15 additions & 2 deletions src/plugins/data/utils.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ const mockIsAdminWrapper = ({ widget }) => {
return isAdmin ? widget : null;
};

const makeMockElementWrapper = (testId = 0) => function MockElementWrapper({ component }) {
const makeMockElementWrapper = (testId = 0) => function MockElementWrapper({ component, pluginProps }) {
return (
<div data-testid={`wrapper${testId}`}>
This is a wrapper.
{pluginProps?.prop1 && `This is a wrapper with ${pluginProps?.prop1}.`}
{component}
</div>
);
Expand Down Expand Up @@ -181,6 +181,18 @@ describe('organizePlugins', () => {
describe('wrapComponent', () => {
describe('when provided with a single wrapper in an array', () => {
it('should wrap the provided component', () => {
const wrappedComponent = wrapComponent(mockRenderWidget, [makeMockElementWrapper()], { prop1: 'prop1' });
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please add a test where a component gets wrapped specifically without any pluginProps?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the review @arbrandes!

Done: 15bb3db


const { getByTestId } = render(wrappedComponent);

const wrapper = getByTestId('wrapper0');
const widget = getByTestId('widget');

expect(wrapper).toContainElement(widget);
expect(wrapper).toHaveTextContent('This is a wrapper with prop1.');
});

it('should wrap the provided component without passing props', () => {
const wrappedComponent = wrapComponent(mockRenderWidget, [makeMockElementWrapper()]);

const { getByTestId } = render(wrappedComponent);
Expand All @@ -189,6 +201,7 @@ describe('wrapComponent', () => {
const widget = getByTestId('widget');

expect(wrapper).toContainElement(widget);
expect(wrapper).not.toHaveTextContent('This is a wrapper with prop1.');
});
});
describe('when provided with multiple wrappers in an array', () => {
Expand Down