From 208a3a4da628a7f08d84dab82a4a0f7061e72b59 Mon Sep 17 00:00:00 2001 From: Adam Stankiewicz Date: Fri, 23 Aug 2024 14:19:30 -0400 Subject: [PATCH] feat: support merging props with PluginOperations.Modify --- example-plugin-app/src/DefaultIframe.jsx | 4 +- example-plugin-app/src/PluginIframe.jsx | 2 +- example/env.config.jsx | 39 +- example/package-lock.json | 6 + example/package.json | 1 + example/src/ExamplePage.jsx | 83 ++++- .../src/components/DefaultDirectWidget.jsx | 2 +- example/src/components/ModularComponent.jsx | 4 +- example/src/components/PluginDirect.jsx | 2 +- .../src/pluginSlots/PluginSlotWithInsert.jsx | 8 +- .../PluginSlotWithModifyDefaultContents.jsx | 107 ++++++ .../PluginSlotWithModifyWrapHide.jsx | 8 +- .../PluginSlotWithModularPlugins.jsx | 6 +- .../pluginSlots/PluginSlotWithoutDefault.jsx | 6 +- package-lock.json | 14 + package.json | 6 +- src/plugins/Plugin.jsx | 2 +- src/plugins/PluginContainer.jsx | 18 +- src/plugins/PluginContainerDirect.jsx | 26 +- src/plugins/PluginContainerIframe.jsx | 5 +- src/plugins/PluginSlot.jsx | 68 +++- src/plugins/PluginSlot.test.jsx | 352 ++++++++++++++++-- src/plugins/data/hooks.js | 5 +- src/plugins/data/shapes.js | 4 + src/plugins/data/utils.jsx | 90 ++++- src/plugins/data/utils.test.jsx | 30 +- 26 files changed, 763 insertions(+), 135 deletions(-) create mode 100644 example/src/pluginSlots/PluginSlotWithModifyDefaultContents.jsx diff --git a/example-plugin-app/src/DefaultIframe.jsx b/example-plugin-app/src/DefaultIframe.jsx index 2f338acd..60b3d306 100644 --- a/example-plugin-app/src/DefaultIframe.jsx +++ b/example-plugin-app/src/DefaultIframe.jsx @@ -6,7 +6,7 @@ import { Plugin } from '@openedx/frontend-plugin-framework'; function DefaultComponent() { return (
-

Default iFrame Widget

+

Default iFrame Widget

This is a component that lives in the example-plugins-app and is provided in this host MFE via iFrame.

@@ -17,7 +17,7 @@ function DefaultComponent() { function ErrorFallback(error) { return (
-

+

Oops! An error occurred. Please refresh the screen to try again.


diff --git a/example-plugin-app/src/PluginIframe.jsx b/example-plugin-app/src/PluginIframe.jsx index 6be2d2c4..bee81b3f 100644 --- a/example-plugin-app/src/PluginIframe.jsx +++ b/example-plugin-app/src/PluginIframe.jsx @@ -5,7 +5,7 @@ export default function PluginIframe() { return (
-

Inserted iFrame Plugin

+

Inserted iFrame Plugin

This is a component that lives in the example-plugins-app and is provided in this host MFE via iFrame plugin.

diff --git a/example/env.config.jsx b/example/env.config.jsx index 934353b2..6bf59335 100644 --- a/example/env.config.jsx +++ b/example/env.config.jsx @@ -4,7 +4,6 @@ import { IFRAME_PLUGIN, PLUGIN_OPERATIONS, } from '@openedx/frontend-plugin-framework'; -import DefaultDirectWidget from './src/components/DefaultDirectWidget'; import PluginDirect from './src/components/PluginDirect'; import ModularComponent from './src/components/ModularComponent'; @@ -18,12 +17,28 @@ const modifyWidget = (widget) => { return modifiedWidget; }; -const wrapWidget = ({ component, idx }) => ( -
-

This is a wrapper component that is placed around the default content.

+const modifyWidgetDefaultContents = (widget) => { + const newContent = { + 'data-custom-attr': 'customValue', + 'data-another-custom-attr': '', + className: 'font-weight-bold', + style: { color: 'blue' }, + onClick: (e) => { console.log('Username clicked!', 'custom', e); }, + }; + widget.content = newContent; + return widget; +}; + +const wrapWidget = ({ component }) => ( +
+
+

This is a wrapper component that is placed around the default content.

+
{component} -

With this wrapper, you can add anything before or after a component.

-

Note in the JS config that an iFrame plugin was Inserted, but a Hide operation was also used to hide it!

+
+

With this wrapper, you can add anything before or after a component.

+

Note in the JS config that an iFrame plugin was Inserted, but a Hide operation was also used to hide it!

+
); @@ -179,7 +194,17 @@ const config = { }, }, ], - } + }, + slot_with_username_pii: { + keepDefault: true, + plugins: [ + { + op: PLUGIN_OPERATIONS.Modify, + widgetId: 'default_contents', + fn: modifyWidgetDefaultContents, + }, + ], + }, }, }; diff --git a/example/package-lock.json b/example/package-lock.json index 6d6c2473..eaab5ab2 100644 --- a/example/package-lock.json +++ b/example/package-lock.json @@ -9,6 +9,7 @@ "version": "1.0.0", "license": "AAGPL-3.0", "dependencies": { + "classnames": "^2.5.1", "core-js": "^3.29.1", "prop-types": "^15.8.1", "react": "^17.0.0", @@ -6150,6 +6151,11 @@ "integrity": "sha512-a3KdPAANPbNE4ZUv9h6LckSl9zLsYOP4MBmhIPkRaeyybt+r4UghLvq+xw/YwUcC1gqylCkL4rdVs3Lwupjm4Q==", "dev": true }, + "node_modules/classnames": { + "version": "2.5.1", + "resolved": "https://registry.npmjs.org/classnames/-/classnames-2.5.1.tgz", + "integrity": "sha512-saHYOzhIQs6wy2sVxTM6bUDsQO4F50V9RQ22qBpEdCW+I+/Wmke2HOl6lS6dTpdxVhb88/I6+Hs+438c3lfUow==" + }, "node_modules/clean-css": { "version": "5.3.3", "resolved": "https://registry.npmjs.org/clean-css/-/clean-css-5.3.3.tgz", diff --git a/example/package.json b/example/package.json index 6c811680..f13b8620 100644 --- a/example/package.json +++ b/example/package.json @@ -12,6 +12,7 @@ "author": "edX", "license": "AAGPL-3.0", "dependencies": { + "classnames": "^2.5.1", "core-js": "^3.29.1", "prop-types": "^15.8.1", "react": "^17.0.0", diff --git a/example/src/ExamplePage.jsx b/example/src/ExamplePage.jsx index 56be359d..5d89a3b1 100644 --- a/example/src/ExamplePage.jsx +++ b/example/src/ExamplePage.jsx @@ -1,31 +1,74 @@ -import React from 'react'; +import { + Container, Row, Col, Stack, +} from '@openedx/paragon'; +import PluginSlotWithModifyDefaultContents from './pluginSlots/PluginSlotWithModifyDefaultContents'; import PluginSlotWithInsert from './pluginSlots/PluginSlotWithInsert'; import PluginSlotWithModifyWrapHide from './pluginSlots/PluginSlotWithModifyWrapHide'; import PluginSlotWithModularPlugins from './pluginSlots/PluginSlotWithModularPlugins'; import PluginSlotWithoutDefault from './pluginSlots/PluginSlotWithoutDefault'; +const pluginExamples = [ + { + id: 'plugin-operation-insert', + label: 'Plugin Operation: Insert', + Component: PluginSlotWithInsert, + }, + { + id: 'plugin-operation-modify-wrap-hide', + label: 'Plugin Operation: Modify, Wrap, and Hide', + Component: PluginSlotWithModifyWrapHide, + }, + { + id: 'plugin-operation-modify-default-content', + label: 'Plugin Operation: Modify Default Content', + Component: PluginSlotWithModifyDefaultContents, + }, + { + id: 'direct-plugins-modular-components', + label: 'Direct Plugins Using Modular Components', + Component: PluginSlotWithModularPlugins, + }, + { + id: 'no-default-content', + label: 'Default Content Set to False', + Component: PluginSlotWithoutDefault, + }, +]; + export default function ExamplePage() { return ( -
-

Plugins Page

- -

- This page is here to help test the plugins module. A plugin configuration can be added in - index.jsx and this page will display that plugin. -

-

- To do this, a plugin MFE must be running on some other port. - To make it a more realistic test, you may also want to edit your - /etc/hosts file (or your system's equivalent) to provide an alternate domain for - 127.0.0.1 at which you can load the plugin. -

-
- - - - -
+
+ + + +

Plugins Page

+

+ This page is here to help test the plugins module. A plugin configuration can be added in + index.jsx and this page will display that plugin. +

+

+ To do this, a plugin MFE must be running on some other port. + To make it a more realistic test, you may also want to edit your + /etc/hosts file (or your system's equivalent) to provide an alternate domain for + 127.0.0.1 at which you can load the plugin. +

+

Examples

+ +
    + {pluginExamples.map(({ id, label }) => ( +
  • + {label} +
    • + ))} +
+ + {pluginExamples.map(({ Component, id, label }) => )} + + + + +
); } diff --git a/example/src/components/DefaultDirectWidget.jsx b/example/src/components/DefaultDirectWidget.jsx index 80b760d4..8945ba1e 100644 --- a/example/src/components/DefaultDirectWidget.jsx +++ b/example/src/components/DefaultDirectWidget.jsx @@ -3,7 +3,7 @@ import React from 'react'; export default function DefaultDirectWidget() { return (
-

Default Direct Widget

+

Default Direct Widget

This widget is a default component that lives in the example app and is directly inserted via JS configuration. Note that this default widget appears after the Inserted Direct Plugin. This is because this component's diff --git a/example/src/components/ModularComponent.jsx b/example/src/components/ModularComponent.jsx index a40543e3..d341368b 100644 --- a/example/src/components/ModularComponent.jsx +++ b/example/src/components/ModularComponent.jsx @@ -4,11 +4,11 @@ import PropTypes from 'prop-types'; export default function ModularComponent({ content }) { return (

-

{ content.title }

+

{content.title}

This is a modular component that lives in the example app.

-

+

{content.uniqueText}

diff --git a/example/src/components/PluginDirect.jsx b/example/src/components/PluginDirect.jsx index 0f484530..96b1282e 100644 --- a/example/src/components/PluginDirect.jsx +++ b/example/src/components/PluginDirect.jsx @@ -3,7 +3,7 @@ import React from 'react'; export default function PluginDirect() { return (
-

Inserted Direct Plugin

+

Inserted Direct Plugin

This plugin is a component that lives in the example app and is directly inserted via JS configuration. What makes this unique is that it isn't part of the default content defined for this slot, but is instead diff --git a/example/src/pluginSlots/PluginSlotWithInsert.jsx b/example/src/pluginSlots/PluginSlotWithInsert.jsx index d09c9ea2..adee3f98 100644 --- a/example/src/pluginSlots/PluginSlotWithInsert.jsx +++ b/example/src/pluginSlots/PluginSlotWithInsert.jsx @@ -2,15 +2,15 @@ import React from 'react'; import { PluginSlot } from '@openedx/frontend-plugin-framework'; -function PluginSlotWithInsert() { +function PluginSlotWithInsert({ id, label }) { return ( -

-

Plugin Operation: Insert

+
+

{label}

-

Default Content

+

Default Content

This widget represents a component that is wrapped by the Plugin Slot. diff --git a/example/src/pluginSlots/PluginSlotWithModifyDefaultContents.jsx b/example/src/pluginSlots/PluginSlotWithModifyDefaultContents.jsx new file mode 100644 index 00000000..45c4c836 --- /dev/null +++ b/example/src/pluginSlots/PluginSlotWithModifyDefaultContents.jsx @@ -0,0 +1,107 @@ +import React from 'react'; + +import { PluginSlot } from '@openedx/frontend-plugin-framework'; +import classNames from 'classnames'; + + +// Example component used as the default childen within a PluginSlot +const Username = ({ className, onClick, ...rest }) => { + const authenticatedUser = { username: 'testuser' }; + const { username } = authenticatedUser; + return ( + { + console.log('Username clicked!', 'default', e); + onClick?.(e); + }} + {...rest} + > + {username} + + ); +}; + +const UsernameWithPluginContent = ({ className, onClick, content = {} }) => { + const { + className: classNameFromPluginContent, + onClick: onClickFromPluginContent, + ...contentRest + } = content; + const updatedProps = { + className: classNames(className, classNameFromPluginContent), + onClick: (e) => { + onClick?.(e); + onClickFromPluginContent?.(e); + }, + }; + return ; +}; + +function PluginSlotWithModifyDefaultContents({ id, label }) { + return ( +

+

{label}

+

+ The following PluginSlot examples demonstrate the PLUGIN_OPERATIONS.Modify operation, when + the widgetId is default_contents. Any configured, custom plugin content is + merged with any existing props passed to the component(s) represented by default_contents. +

+
    +
  • Custom className overrides are concatenated with the className prop passed to the default_contents component(s), if any.
    • +
  • Custom style overrides are shallow merged with the style prop passed to the default_contents component(s), if any.
    • +
  • Custom event handlers (e.g., onClick) are executed in sequence, after any event handlers passed to the default_contents component(s), if any.
    • +
+ {/* + { + console.log('Username clicked!', 'prop', e); + }} + /> + */} + + { + console.log('Username clicked!', 'prop', e); + }} + /> + + + {/* { console.log('Username clicked!', 'pluginProps', e); }, + }} + slotOptions={{ + mergeProps: true, + }} + > + { + console.log('Username clicked!', 'prop', e); + }} + /> + */} +
+ ); +} + +export default PluginSlotWithModifyDefaultContents; diff --git a/example/src/pluginSlots/PluginSlotWithModifyWrapHide.jsx b/example/src/pluginSlots/PluginSlotWithModifyWrapHide.jsx index c4772fd3..7fd24ac7 100644 --- a/example/src/pluginSlots/PluginSlotWithModifyWrapHide.jsx +++ b/example/src/pluginSlots/PluginSlotWithModifyWrapHide.jsx @@ -4,19 +4,19 @@ import { PluginSlot } from '@openedx/frontend-plugin-framework'; import ModularComponent from '../components/ModularComponent'; -function PluginSlotWithModifyWrapHide() { +function PluginSlotWithModifyWrapHide({ id, label }) { const content = { title: 'Default Content', uniqueText: "Because this modular component is default content, this text is passed in as a prop within PluginSlot." } return ( -
-

Plugin Operation: Modify, Wrap, and Hide

+
+

{label}

- +
); diff --git a/example/src/pluginSlots/PluginSlotWithModularPlugins.jsx b/example/src/pluginSlots/PluginSlotWithModularPlugins.jsx index 5a069294..57c9c7b2 100644 --- a/example/src/pluginSlots/PluginSlotWithModularPlugins.jsx +++ b/example/src/pluginSlots/PluginSlotWithModularPlugins.jsx @@ -3,15 +3,15 @@ import React from 'react'; import { PluginSlot } from '@openedx/frontend-plugin-framework'; import ModularComponent from '../components/ModularComponent'; -function PluginSlotWithModularPlugins() { +function PluginSlotWithModularPlugins({ id, label }) { const content = { title: 'Default Content', uniqueText: 'Default content are set with a priority of 50, which is why it appears second in this slot.', } return ( -
-

Direct Plugins Using Modular Components

+
+

{label}

diff --git a/example/src/pluginSlots/PluginSlotWithoutDefault.jsx b/example/src/pluginSlots/PluginSlotWithoutDefault.jsx index ff494283..e91535f6 100644 --- a/example/src/pluginSlots/PluginSlotWithoutDefault.jsx +++ b/example/src/pluginSlots/PluginSlotWithoutDefault.jsx @@ -2,10 +2,10 @@ import React from 'react'; import { PluginSlot } from '@openedx/frontend-plugin-framework'; -function PluginSlotWithoutDefault() { +function PluginSlotWithoutDefault({ id, label }) { return ( -
-

Default Content Set to False

+
+

{label}

diff --git a/package-lock.json b/package-lock.json index e66892e2..ed9d02ec 100644 --- a/package-lock.json +++ b/package-lock.json @@ -25,6 +25,7 @@ "@testing-library/dom": "^8.20.1", "@testing-library/jest-dom": "^6.1.4", "@testing-library/react": "^12.1.5", + "@testing-library/user-event": "^14.5.2", "glob": "7.2.3", "husky": "7.0.4", "jest": "29.7.0", @@ -4781,6 +4782,19 @@ "react-dom": "<18.0.0" } }, + "node_modules/@testing-library/user-event": { + "version": "14.5.2", + "resolved": "https://registry.npmjs.org/@testing-library/user-event/-/user-event-14.5.2.tgz", + "integrity": "sha512-YAh82Wh4TIrxYLmfGcixwD18oIjyC1pFQC2Y01F2lzV2HTMiYrI0nze0FD0ocB//CKS/7jIUgae+adPqxK5yCQ==", + "dev": true, + "engines": { + "node": ">=12", + "npm": ">=6" + }, + "peerDependencies": { + "@testing-library/dom": ">=7.21.4" + } + }, "node_modules/@tootallnate/once": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/@tootallnate/once/-/once-2.0.0.tgz", diff --git a/package.json b/package.json index 32834866..4ae1a527 100644 --- a/package.json +++ b/package.json @@ -16,9 +16,10 @@ "lint": "fedx-scripts eslint --ext .js --ext .jsx .", "lint:fix": "fedx-scripts eslint --fix --ext .js --ext .jsx .", "snapshot": "fedx-scripts jest --updateSnapshot", - "start": "./node_modules/.bin/fedx-scripts babel src --watch --out-dir dist --source-maps --ignore **/*.test.jsx,**/*.test.js,**/setupTest.js --copy-files", + "start": "fedx-scripts babel src --watch --out-dir dist --source-maps --ignore **/*.test.jsx,**/*.test.js,**/setupTest.js", "start:example": "npm --prefix example start & npm --prefix example-plugin-app start", - "test": "fedx-scripts jest --coverage --passWithNoTests" + "test": "fedx-scripts jest --coverage --passWithNoTests", + "test:watch": "npm run test -- --watch" }, "husky": { "hooks": { @@ -51,6 +52,7 @@ "@testing-library/dom": "^8.20.1", "@testing-library/jest-dom": "^6.1.4", "@testing-library/react": "^12.1.5", + "@testing-library/user-event": "^14.5.2", "glob": "7.2.3", "husky": "7.0.4", "jest": "29.7.0", diff --git a/src/plugins/Plugin.jsx b/src/plugins/Plugin.jsx index 461468b5..8d463daa 100644 --- a/src/plugins/Plugin.jsx +++ b/src/plugins/Plugin.jsx @@ -92,7 +92,7 @@ Plugin.propTypes = { }; Plugin.defaultProps = { - className: null, + className: undefined, ErrorFallbackComponent: null, style: {}, ready: true, diff --git a/src/plugins/PluginContainer.jsx b/src/plugins/PluginContainer.jsx index 8bd158a7..ece4e95e 100644 --- a/src/plugins/PluginContainer.jsx +++ b/src/plugins/PluginContainer.jsx @@ -3,7 +3,6 @@ import React from 'react'; import PropTypes from 'prop-types'; -// eslint-disable-next-line import/no-extraneous-dependencies import PluginContainerIframe from './PluginContainerIframe'; import PluginContainerDirect from './PluginContainerDirect'; @@ -11,10 +10,10 @@ import { IFRAME_PLUGIN, DIRECT_PLUGIN, } from './data/constants'; -import { pluginConfigShape } from './data/shapes'; +import { pluginConfigShape, slotOptionsShape } from './data/shapes'; -function PluginContainer({ config, ...props }) { - if (config === null) { +function PluginContainer({ config, slotOptions, ...props }) { + if (!config) { return null; } @@ -23,21 +22,19 @@ function PluginContainer({ config, ...props }) { switch (config.type) { case IFRAME_PLUGIN: renderer = ( - + ); break; case DIRECT_PLUGIN: renderer = ( - + ); break; default: break; } - return ( - renderer - ); + return renderer; } export default PluginContainer; @@ -45,8 +42,11 @@ export default PluginContainer; PluginContainer.propTypes = { /** Configuration for the Plugin in this container — i.e pluginSlot[id].example_plugin */ config: PropTypes.shape(pluginConfigShape), + /** Options passed to the PluginSlot */ + slotOptions: PropTypes.shape(slotOptionsShape), }; PluginContainer.defaultProps = { config: null, + slotOptions: {}, }; diff --git a/src/plugins/PluginContainerDirect.jsx b/src/plugins/PluginContainerDirect.jsx index 0bb35710..23127407 100644 --- a/src/plugins/PluginContainerDirect.jsx +++ b/src/plugins/PluginContainerDirect.jsx @@ -1,30 +1,40 @@ import React, { Suspense } from 'react'; import PropTypes from 'prop-types'; -import { directPluginConfigShape } from './data/shapes'; +import { directPluginConfigShape, slotOptionsShape } from './data/shapes'; +import { mergeRenderWidgetPropsWithPluginContent } from './data/utils'; -function PluginContainerDirect({ config, loadingFallback, ...props }) { - const { - RenderWidget, id, content, - } = config; +function PluginContainerDirect({ + config, slotOptions, loadingFallback, ...props +}) { + const { RenderWidget, id } = config; + + // When applicable, merge base RenderWidget props with custom plugin content, if any. + const propsForRenderWidget = mergeRenderWidgetPropsWithPluginContent({ + pluginSlotOptions: slotOptions, + pluginConfig: config, + renderWidgetProps: props, + }); return ( - + ); } PluginContainerDirect.propTypes = { /** Configuration for the Plugin in this container (i.e. pluginSlot[id].example_plugin) */ - config: PropTypes.shape(directPluginConfigShape), + config: PropTypes.shape(directPluginConfigShape).isRequired, /** Custom fallback component used when component is not ready (i.e. "loading") */ loadingFallback: PropTypes.node, + /** Options passed to the PluginSlot */ + slotOptions: PropTypes.shape(slotOptionsShape), }; PluginContainerDirect.defaultProps = { - config: null, loadingFallback: null, + slotOptions: {}, }; export default PluginContainerDirect; diff --git a/src/plugins/PluginContainerIframe.jsx b/src/plugins/PluginContainerIframe.jsx index 40eafb45..22aa0795 100644 --- a/src/plugins/PluginContainerIframe.jsx +++ b/src/plugins/PluginContainerIframe.jsx @@ -84,7 +84,7 @@ export default PluginContainerIframe; PluginContainerIframe.propTypes = { /** Configuration for the Plugin in this container (i.e. pluginSlot[id].example_plugin) */ - config: PropTypes.shape(iframePluginConfigShape), + config: PropTypes.shape(iframePluginConfigShape).isRequired, /** Custom fallback component used when component is not ready (i.e. "loading") */ loadingFallback: PropTypes.node, /** Classes to apply to the iframe */ @@ -92,7 +92,6 @@ PluginContainerIframe.propTypes = { }; PluginContainerIframe.defaultProps = { - config: null, loadingFallback: null, - className: null, + className: undefined, }; diff --git a/src/plugins/PluginSlot.jsx b/src/plugins/PluginSlot.jsx index f03193cd..9d5d72b7 100644 --- a/src/plugins/PluginSlot.jsx +++ b/src/plugins/PluginSlot.jsx @@ -5,14 +5,36 @@ import classNames from 'classnames'; import { Spinner } from '@openedx/paragon'; import PropTypes from 'prop-types'; import { useIntl } from '@edx/frontend-platform/i18n'; +import { logError } from '@edx/frontend-platform/logging'; import messages from './Plugin.messages'; import { usePluginSlot } from './data/hooks'; import PluginContainer from './PluginContainer'; -import { organizePlugins, wrapComponent } from './data/utils'; +import { mergeRenderWidgetPropsWithPluginContent, organizePlugins, wrapComponent } from './data/utils'; +import { slotOptionsShape } from './data/shapes'; + +/** + * Custom error class for specific error handling. + * + * @class PluginSlotMultipleChildrenError + * @extends {Error} + */ +export class PluginSlotInvalidChildrenElementError extends Error { + /** + * Creates an instance of PluginSlotInvalidChildrenElementError. + * + * @param {string} pluginSlotId The PluginSlot identifier to use within the log message. + * @param {string} widgetId The widget identifier to use within the log message. + */ + constructor(pluginSlotId, widgetId) { + const message = `[PluginSlot id="${pluginSlotId}" widgetId="${widgetId}"] The children of PluginSlot must only contain a valid React element.`; + super(message); + this.name = 'PluginSlotInvalidChildrenElementError'; + } +} const PluginSlot = forwardRef(({ - as, children, id, pluginProps, ...props + as, children, id, pluginProps, slotOptions, ...props }, ref) => { /** the plugins below are obtained by the id passed into PluginSlot by the Host MFE. See example/src/PluginsPage.jsx for an example of how PluginSlot is populated, and example/src/index.jsx for a dummy JS config that holds all plugins @@ -22,15 +44,21 @@ const PluginSlot = forwardRef(({ const { formatMessage } = useIntl(); const defaultContents = React.useMemo(() => { - if (keepDefault) { - return ([{ - id: 'default_contents', - priority: 50, - RenderWidget: children, - }]); + if (!keepDefault) { + return []; + } + const widgetId = 'default_contents'; + if (!React.isValidElement(children)) { + const PluginSlotError = new PluginSlotInvalidChildrenElementError(id, widgetId); + logError(PluginSlotError); + throw PluginSlotError; } - return []; - }, [children, keepDefault]); + return ([{ + id: widgetId, + priority: 50, + RenderWidget: children, + }]); + }, [id, children, keepDefault]); const finalPlugins = React.useMemo(() => organizePlugins(defaultContents, plugins), [defaultContents, plugins]); @@ -54,15 +82,25 @@ const PluginSlot = forwardRef(({ // If hidden, don't push to finalChildren if (!pluginConfig.hidden) { let container; - // If default content, render children + // If default content, render children (merging any custom defined props from + // pluginConfig.content with existing props, if any, on `RenderWidget`). if (pluginConfig.id === 'default_contents') { - container = pluginConfig.RenderWidget; + const updatedPropsForRenderWidget = mergeRenderWidgetPropsWithPluginContent({ + pluginSlotOptions: slotOptions, + pluginConfig, + pluginProps, + renderWidgetProps: pluginConfig.RenderWidget.props, + }); + container = React.isValidElement(pluginConfig.RenderWidget) + ? React.cloneElement(pluginConfig.RenderWidget, { ...updatedPropsForRenderWidget, key: pluginConfig.id }) + : pluginConfig.RenderWidget; } else { container = ( ); @@ -91,6 +129,7 @@ const PluginSlot = forwardRef(({ finalChildren, ); }); +PluginSlot.displayName = 'PluginSlot'; export default PluginSlot; @@ -102,11 +141,14 @@ PluginSlot.propTypes = { /** ID of the PluginSlot configuration */ id: PropTypes.string.isRequired, /** Props that are passed down to each Plugin in the Slot */ - pluginProps: PropTypes.object, // eslint-disable-line + pluginProps: PropTypes.shape(), + /** Options passed to the PluginSlot */ + slotOptions: PropTypes.shape(slotOptionsShape), }; PluginSlot.defaultProps = { as: React.Fragment, children: null, pluginProps: {}, + slotOptions: {}, }; diff --git a/src/plugins/PluginSlot.test.jsx b/src/plugins/PluginSlot.test.jsx index 97d04904..16251d14 100644 --- a/src/plugins/PluginSlot.test.jsx +++ b/src/plugins/PluginSlot.test.jsx @@ -2,10 +2,12 @@ import React from 'react'; import '@testing-library/jest-dom'; +import classNames from 'classnames'; import { render, fireEvent } 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'; -import PluginSlot from './PluginSlot'; +import PluginSlot, { PluginSlotInvalidChildrenElementError } from './PluginSlot'; import { usePluginSlot } from './data/hooks'; import { PLUGIN_OPERATIONS } from './data/constants'; @@ -47,21 +49,90 @@ global.ResizeObserver = jest.fn(function mockResizeObserver() { this.disconnect = jest.fn(); }); +// Mock callback functions +const pluginContentOnClick = jest.fn(); +const defaultContentsOnClick = jest.fn(); +const mockOnClick = jest.fn(); + // TODO: APER-3119 — Write unit tests for plugin scenarios not already tested for https://2u-internal.atlassian.net/browse/APER-3119 const content = { text: 'This is a widget.' }; -const TestPluginSlot = ( - - { + defaultContentsOnClick(e); + onClick?.(e); + }; + + return ( +
-
- {content.text} -
- - -); + {content.text} +
+ ); +} + +function PluginSlotWrapper({ slotOptions, children }) { + return ( + + + {children} + + + ); +} + +function TestPluginSlot({ + hasDefaultContentsOnClick = false, + hasChildrenElement = true, + hasMultipleChildren = false, + ...rest +}) { + const defaultContentsProps = { + className: 'other-classname', + style: { background: 'gray' }, + }; + if (hasDefaultContentsOnClick) { + defaultContentsProps.onClick = mockOnClick; + } + if (hasChildrenElement && hasMultipleChildren) { + return ( + + + + + ); + } + return ( + + {hasChildrenElement + ? + : content.text} + + ); +} + +const defaultSlotOptions = { + mergeProps: true, +}; + +function TestPluginSlotWithSlotOptions({ slotOptions: slotOptionsOverride, ...rest }) { + const slotOptions = { + ...defaultSlotOptions, + ...slotOptionsOverride, + }; + return ; +} describe('PluginSlot', () => { beforeEach(() => { @@ -73,7 +144,7 @@ describe('PluginSlot', () => { }); it('should render multiple types of Plugin in a single slot config', () => { - const { container, getByTestId } = render(TestPluginSlot); + const { container, getByTestId } = render(); const iframeElement = container.querySelector('iframe'); const defaultContent = getByTestId('default_contents'); const pluginSlot = getByTestId('test-slot-id'); @@ -83,7 +154,7 @@ describe('PluginSlot', () => { }); it('should order each Plugin by priority', () => { - const { container, getByTestId } = render(TestPluginSlot); + const { container, getByTestId } = render(); const iframeElement = container.querySelector('iframe'); const defaultContent = getByTestId('default_contents'); const pluginSlot = getByTestId('test-slot-id'); @@ -106,8 +177,8 @@ describe('PluginSlot', () => { { op: PLUGIN_OPERATIONS.Wrap, widgetId: 'default_contents', - wrapper: ({ component, idx }) => ( -
+ wrapper: ({ component }) => ( +
{component}
), @@ -116,11 +187,10 @@ describe('PluginSlot', () => { keepDefault: true, }); - const { getByTestId } = render(TestPluginSlot); - const wrapper1 = getByTestId('wrapper1'); + const { getByTestId } = render(); + const customWrapper = getByTestId('custom-wrapper'); const defaultContent = getByTestId('default_contents'); - - expect(wrapper1).toContainElement(defaultContent); + expect(customWrapper).toContainElement(defaultContent); }); it('should not render a widget if the Hide operation is applied to it', () => { @@ -134,7 +204,7 @@ describe('PluginSlot', () => { ], keepDefault: true, }); - const { container } = render(TestPluginSlot); + const { container } = render(); const iframeElement = container.querySelector('iframe'); expect(iframeElement).toBeNull(); @@ -153,8 +223,244 @@ describe('PluginSlot', () => { ], keepDefault: true, }); - render(TestPluginSlot); + render(); expect(logError).toHaveBeenCalledWith('the insert operation config is invalid for widget id: invalid_config'); }); + + it('should throw an error for invalid children element', () => { + try { + render(); + throw new Error('PluginSlot was rendered with invalid children element. Expected to throw PluginSlotInvalidChildrenElement'); + } catch (error) { + const isExpectedErrorClass = error instanceof PluginSlotInvalidChildrenElementError; + if (!isExpectedErrorClass) { + // re-raise unexpected error classes + throw error; + } + expect(error.message).toEqual( + '[PluginSlot id="test-slot" widgetId="default_contents"] The children of PluginSlot must only contain a valid React element.', + ); + } + }); + + it('should handle keepDefault=false', () => { + usePluginSlot.mockReturnValueOnce({ + plugins: [ + { + op: PLUGIN_OPERATIONS.Insert, + widget: { + id: 'inserted_direct_plugin', + type: 'DIRECT_PLUGIN', + priority: 1, + RenderWidget: () =>
Inserted Direct Plugin
, + }, + }, + ], + keepDefault: false, + }); + const { container, queryByTestId, getByTestId } = render(); + const defaultContent = queryByTestId('default_contents'); + + expect(container).not.toContainElement(defaultContent); + const insertedPlugin = getByTestId('inserted_direct_plugin'); + expect(container).toContainElement(insertedPlugin); + }); + + it.each([ + { + MockPluginSlot: , + hasPluginSlotChildElement: true, + hasMockOnClick: false, + pluginContent: { 'data-custom-attr': 'abc' }, + includedAttributes: ['data-custom-attr'], + }, + { + MockPluginSlot: , + hasPluginSlotChildElement: true, + hasMockOnClick: false, + pluginContent: { 'data-custom-attr': '' }, + includedAttributes: ['data-custom-attr'], + }, + { + MockPluginSlot: , + hasPluginSlotChildElement: true, + hasMockOnClick: false, + pluginContent: { 'data-custom-attr': undefined }, + includedAttributes: ['data-custom-attr'], + }, + { + MockPluginSlot: , + hasPluginSlotChildElement: true, + hasMockOnClick: false, + pluginContent: { 'data-custom-attr': null }, + includedAttributes: ['data-custom-attr'], + }, + { + MockPluginSlot: , + hasPluginSlotChildElement: true, + hasMockOnClick: false, + pluginContent: { className: 'test123' }, + includedAttributes: ['className'], + }, + { + MockPluginSlot: , + hasPluginSlotChildElement: true, + hasMockOnClick: false, + pluginContent: { style: 'invalid' }, + includedAttributes: [], + }, + { + MockPluginSlot: , + hasPluginSlotChildElement: true, + hasMockOnClick: false, + pluginContent: { style: { color: 'blue' } }, + includedAttributes: ['style'], + }, + { + MockPluginSlot: , + hasPluginSlotChildElement: true, + hasMockOnClick: true, + pluginContent: { onClick: pluginContentOnClick }, + includedAttributes: ['onClick'], + }, + { + MockPluginSlot: , + hasPluginSlotChildElement: true, + hasMockOnClick: false, + pluginContent: { onClick: pluginContentOnClick }, + includedAttributes: ['onClick'], + }, + { + MockPluginSlot: , + hasPluginSlotChildElement: true, + hasMockOnClick: false, + pluginContent: {}, + includedAttributes: [], + }, + { + MockPluginSlot: , + hasPluginSlotChildElement: true, + hasMockOnClick: false, + pluginContent: undefined, + includedAttributes: [], + }, + { + MockPluginSlot: , + hasPluginSlotChildElement: true, + hasMockOnClick: false, + pluginContent: null, + includedAttributes: [], + }, + // { + // MockPluginSlot: , + // hasPluginSlotChildElement: false, + // hasMockOnClick: false, + // pluginContent: { 'data-custom-attr': 'abc' }, + // includedAttributes: [], + // }, + // { + // MockPluginSlot: , + // hasPluginSlotChildElement: false, + // hasMockOnClick: false, + // pluginContent: {}, + // includedAttributes: [], + // }, + // { + // MockPluginSlot: , + // hasPluginSlotChildElement: false, + // hasMockOnClick: false, + // pluginContent: null, + // includedAttributes: [], + // }, + // { + // MockPluginSlot: , + // hasPluginSlotChildElement: false, + // hasMockOnClick: false, + // pluginContent: undefined, + // includedAttributes: [], + // }, + ])('should handle the Modify operation for the default_contents widget with slotOptions.mergeProps=true (%s)', async ({ + MockPluginSlot, + hasPluginSlotChildElement, + hasMockOnClick, + pluginContent, + includedAttributes, + }) => { + usePluginSlot.mockReturnValueOnce({ + plugins: [ + { + op: PLUGIN_OPERATIONS.Modify, + widgetId: 'default_contents', + fn: (widget) => ({ + ...widget, + content: pluginContent, + }), + }, + ], + keepDefault: true, + }); + const { queryByTestId, getByText } = render(MockPluginSlot); + + expect(getByText(content.text)).toBeInTheDocument(); + const defaultContents = queryByTestId('default_contents'); + + if (hasPluginSlotChildElement) { + expect(defaultContents).toBeInTheDocument(); + expect(defaultContents).toHaveClass('default-classname'); + expect(defaultContents).toHaveClass('other-classname'); + expect(defaultContents).toHaveStyle({ background: 'gray' }); + + await userEvent.click(defaultContents); + const expectedEventObject = expect.objectContaining({ type: 'click', target: expect.any(Element) }); + expect(defaultContentsOnClick).toHaveBeenCalledTimes(1); + expect(defaultContentsOnClick).toHaveBeenCalledWith(expectedEventObject); + if (hasMockOnClick) { + expect(mockOnClick).toHaveBeenCalledTimes(1); + expect(mockOnClick).toHaveBeenCalledWith(expectedEventObject); + } + + if (!pluginContent) { return; } + + Object.entries(pluginContent).forEach(([key, value]) => { + const expectedValue = !value ? '' : value; + if (!includedAttributes.includes(key)) { + if (key === 'style') { + Object.entries(value).forEach(([styleProperty, styleValue]) => { + const expectedStyle = {}; + expectedStyle[styleProperty] = styleValue; + expect(defaultContents).not.toHaveStyle(expectedStyle); + }); + return; + } + expect(defaultContents).not.toHaveAttribute(key); + return; + } + + if (key === 'className') { + expect(defaultContents).toHaveClass(expectedValue); + return; + } + + if (key === 'style') { + Object.entries(value).forEach(([styleProperty, styleValue]) => { + const expectedStyle = {}; + expectedStyle[styleProperty] = styleValue; + expect(defaultContents).toHaveStyle(expectedStyle); + }); + return; + } + + if (key === 'onClick') { + expect(pluginContentOnClick).toHaveBeenCalledTimes(1); + expect(pluginContentOnClick).toHaveBeenCalledWith(expectedEventObject); + return; + } + + expect(defaultContents).toHaveAttribute(key, expectedValue); + }); + } else { + expect(defaultContents).not.toBeInTheDocument(); + } + }); }); diff --git a/src/plugins/data/hooks.js b/src/plugins/data/hooks.js index dbafb097..df115874 100644 --- a/src/plugins/data/hooks.js +++ b/src/plugins/data/hooks.js @@ -15,8 +15,9 @@ import { PLUGIN_MOUNTED, PLUGIN_READY, PLUGIN_UNMOUNTED } from './constants'; * @returns {Object} - JS configuration for the PluginSlot */ export function usePluginSlot(id) { - if (getConfigSlots() && getConfigSlots()[id] !== undefined) { - return getConfigSlots()[id]; + const configSlots = getConfigSlots()?.[id]; + if (configSlots) { + return configSlots; } return { keepDefault: true, plugins: [] }; } diff --git a/src/plugins/data/shapes.js b/src/plugins/data/shapes.js index e94d206c..f0b1461a 100644 --- a/src/plugins/data/shapes.js +++ b/src/plugins/data/shapes.js @@ -27,3 +27,7 @@ export const directPluginConfigShape = { /** Content that is passed to the RenderWidget function */ content: PropTypes.object, }; + +export const slotOptionsShape = { + mergeProps: PropTypes.bool, +}; diff --git a/src/plugins/data/utils.jsx b/src/plugins/data/utils.jsx index e63ae8e9..ea923ec8 100644 --- a/src/plugins/data/utils.jsx +++ b/src/plugins/data/utils.jsx @@ -7,10 +7,10 @@ import { PLUGIN_OPERATIONS, requiredPluginTypes } from './constants'; * Called by validatePlugin to compare plugin config to the required data and data types * @returns {Boolean} - returns true if all types are correct and present according to the plugin operation */ -const validateRequirements = (requiredTypes, widgetConfig) => (Object.keys(requiredTypes).every( +const validateRequirements = (requiredTypes, widgetConfig) => Object.keys(requiredTypes).every( // eslint-disable-next-line valid-typeof (field) => (widgetConfig[field] && (typeof widgetConfig[field] === requiredTypes[field])), -)); +); /** * Called by organizePlugins to validate plugin configurations @@ -18,8 +18,9 @@ const validateRequirements = (requiredTypes, widgetConfig) => (Object.keys(requi */ export const validatePlugin = (pluginConfig) => { let requiredTypes = {}; - // eslint-disable-next-line prefer-const - let { op, ...config } = pluginConfig; + const { op } = pluginConfig; + let config = pluginConfig; + if (!op) { logError('There is a config with an invalid PLUGIN_OPERATION. Check to make sure it is configured correctly.'); } if (op === PLUGIN_OPERATIONS.Insert) { @@ -37,6 +38,7 @@ export const validatePlugin = (pluginConfig) => { if (!validateRequirements(requiredTypes, config)) { logError(`the ${op} operation config is invalid for widget id: ${config.widgetId || config.id || 'MISSING ID'}`); } + return true; }; @@ -49,7 +51,6 @@ export const validatePlugin = (pluginConfig) => { */ export const organizePlugins = (defaultContents, plugins) => { const newContents = [...defaultContents]; - plugins.forEach(change => { validatePlugin(change); if (change.op === PLUGIN_OPERATIONS.Insert) { @@ -87,19 +88,84 @@ export const wrapComponent = (renderComponent, wrappers) => 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, idx }), + (component, wrapper, idx) => React.createElement(wrapper, { component, key: idx }), renderComponent(), ); /** * Called by usePluginSlot to retrieve the most up-to-date Config Document* - * @returns {Object} - The pluginSlots object in Config Document + * @returns {object|undefined} - The pluginSlots object in Config Document */ export const getConfigSlots = () => getConfig()?.pluginSlots; -export default { - getConfigSlots, - organizePlugins, - validatePlugin, - wrapComponent, +/** + * @param {object} [originalProps] + * @param {object} [newProps] + * @returns {object} Original props merged with new props. + */ +const mergeProps = (originalProps = {}, newProps = {}) => { + const updatedProps = { ...originalProps }; + Object.entries(newProps).forEach(([attributeName, attributeValue]) => { + let transformedAttributeValue = !attributeValue ? '' : attributeValue; + if (attributeName === 'className') { + // Append the `className` to the existing `className` prop value (if any) + transformedAttributeValue = [updatedProps.className, attributeValue].join(' ').trim(); + } else if (attributeName === 'style') { + // Only update `style` prop if attributeValue is an object + if (typeof attributeValue !== 'object') { + return; + } + // Merge the `style` object with the existing `style` prop object (if any) + transformedAttributeValue = { ...updatedProps.style, ...attributeValue }; + } else if (typeof attributeValue === 'function') { + // Merge the function with the existing prop's function + const oldFn = updatedProps[attributeName]; + transformedAttributeValue = oldFn ? (...args) => { + oldFn(...args); + attributeValue(...args); + } : attributeValue; + } + updatedProps[attributeName] = transformedAttributeValue; + }); + return updatedProps; +}; + +/** + * Merges the plugin content with the RenderWidget props, if any. Handles special cases + * like merging `className`, `style`, and functions. + * @param {object} [pluginSlotOptions] + * @param {object} pluginConfig + * @param {object} pluginProps + * @param {object} renderWidgetProps + * @returns {object} - Updated RenderWidget props merged with custom pluginConfig.content. + */ +export const mergeRenderWidgetPropsWithPluginContent = ({ + pluginSlotOptions, + pluginConfig, + pluginProps, + renderWidgetProps, +}) => { + // Always merge RenderWidget props and pluginProps and provide a `key`. + const renderWidgetPropsWithPluginProps = mergeProps(renderWidgetProps, pluginProps); + let updatedRenderWidgetProps = { key: pluginConfig.id, ...renderWidgetPropsWithPluginProps }; + + // No custom plugin content; return updated props; + if (!pluginConfig.content) { + return updatedRenderWidgetProps; + } + + // Handle custom plugin content + const { mergeProps: shouldMergeProps } = pluginSlotOptions; + + if (shouldMergeProps) { + updatedRenderWidgetProps = mergeProps(updatedRenderWidgetProps, pluginConfig.content); + } else { + updatedRenderWidgetProps = { + ...updatedRenderWidgetProps, + // pass custom props contained with `content` prop + content: pluginConfig.content, + }; + } + + return updatedRenderWidgetProps; }; diff --git a/src/plugins/data/utils.test.jsx b/src/plugins/data/utils.test.jsx index 82e3cb43..b93c4163 100644 --- a/src/plugins/data/utils.test.jsx +++ b/src/plugins/data/utils.test.jsx @@ -22,12 +22,14 @@ const mockIsAdminWrapper = ({ widget }) => { return isAdmin ? widget : null; }; -const mockElementWrapper = ({ component, idx }) => ( -
- This is a wrapper. - {component} -
-); +const makeMockElementWrapper = (testId = 0) => function MockElementWrapper({ component }) { + return ( +
+ This is a wrapper. + {component} +
+ ); +}; const mockRenderWidget = () => (
@@ -179,11 +181,11 @@ describe('organizePlugins', () => { describe('wrapComponent', () => { describe('when provided with a single wrapper in an array', () => { it('should wrap the provided component', () => { - const wrappedComponent = wrapComponent(mockRenderWidget, [mockElementWrapper]); + const wrappedComponent = wrapComponent(mockRenderWidget, [makeMockElementWrapper()]); const { getByTestId } = render(wrappedComponent); - const wrapper = getByTestId('wrapper1'); + const wrapper = getByTestId('wrapper0'); const widget = getByTestId('widget'); expect(wrapper).toContainElement(widget); @@ -193,14 +195,14 @@ describe('wrapComponent', () => { it('should wrap starting with the first wrapper in the array', () => { const wrappedComponent = wrapComponent( mockRenderWidget, - [mockElementWrapper, mockElementWrapper, mockElementWrapper], + [makeMockElementWrapper(), makeMockElementWrapper(1), makeMockElementWrapper(2)], ); const { getByTestId } = render(wrappedComponent); - const innermostWrapper = getByTestId('wrapper1'); - const middleWrapper = getByTestId('wrapper2'); - const outermostWrapper = getByTestId('wrapper3'); + const innermostWrapper = getByTestId('wrapper0'); + const middleWrapper = getByTestId('wrapper1'); + const outermostWrapper = getByTestId('wrapper2'); const widget = getByTestId('widget'); expect(innermostWrapper).toContainElement(widget); @@ -401,7 +403,7 @@ describe('validatePlugin', () => { const validWrapConfig = { op: PLUGIN_OPERATIONS.Wrap, widgetId: 'random_plugin', - wrapper: mockElementWrapper, + wrapper: makeMockElementWrapper(), }; expect(validatePlugin(validWrapConfig)).toBe(true); }); @@ -412,7 +414,7 @@ describe('validatePlugin', () => { }; const invalidWrapConfig2 = { op: PLUGIN_OPERATIONS.Wrap, - wrapper: mockElementWrapper, + wrapper: makeMockElementWrapper(), }; try {