From aff65a1b71f28b36b6b2fb80653db3bef17d7fd5 Mon Sep 17 00:00:00 2001 From: Adam Stankiewicz Date: Fri, 23 Aug 2024 14:19:30 -0400 Subject: [PATCH 1/6] 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 | 25 +- src/plugins/PluginContainerDirect.jsx | 26 +- src/plugins/PluginContainerIframe.jsx | 5 +- src/plugins/PluginSlot.jsx | 47 ++- 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, 751 insertions(+), 133 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..50762013 --- /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); + }} + /> + {/* // Note: uncomment to see example of incorrectly using multiple `children` elements within `PluginSlot` */} + + { 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..3f7ab403 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,26 @@ 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 +49,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..9819b1e0 100644 --- a/src/plugins/PluginSlot.jsx +++ b/src/plugins/PluginSlot.jsx @@ -9,10 +9,16 @@ import { useIntl } from '@edx/frontend-platform/i18n'; 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'; 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,14 +28,14 @@ const PluginSlot = forwardRef(({ const { formatMessage } = useIntl(); const defaultContents = React.useMemo(() => { - if (keepDefault) { - return ([{ - id: 'default_contents', - priority: 50, - RenderWidget: children, - }]); + if (!keepDefault) { + return []; } - return []; + return ([{ + id: 'default_contents', + priority: 50, + RenderWidget: children, + }]); }, [children, keepDefault]); const finalPlugins = React.useMemo(() => organizePlugins(defaultContents, plugins), [defaultContents, plugins]); @@ -54,15 +60,28 @@ 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 propsForRenderWidget = (pluginConfig.RenderWidget && React.isValidElement(pluginConfig.RenderWidget)) + ? pluginConfig.RenderWidget.props + : {}; + const updatedPropsForRenderWidget = mergeRenderWidgetPropsWithPluginContent({ + pluginSlotOptions: slotOptions, + pluginConfig, + pluginProps, + renderWidgetProps: propsForRenderWidget, + }); + container = React.isValidElement(pluginConfig.RenderWidget) + ? React.cloneElement(pluginConfig.RenderWidget, { ...updatedPropsForRenderWidget, key: pluginConfig.id }) + : pluginConfig.RenderWidget; } else { container = ( ); @@ -91,6 +110,7 @@ const PluginSlot = forwardRef(({ finalChildren, ); }); +PluginSlot.displayName = 'PluginSlot'; export default PluginSlot; @@ -102,11 +122,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..2a487059 100644 --- a/src/plugins/PluginSlot.test.jsx +++ b/src/plugins/PluginSlot.test.jsx @@ -2,7 +2,9 @@ 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'; @@ -47,21 +49,94 @@ 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, + hasChildren = true, + hasChildrenElement = true, + hasMultipleChildren = false, + ...rest +}) { + const defaultContentsProps = { + className: 'other-classname', + style: { background: 'gray' }, + }; + if (hasDefaultContentsOnClick) { + defaultContentsProps.onClick = mockOnClick; + } + if (hasChildren && hasChildrenElement && hasMultipleChildren) { + return ( + + + + + ); + } + if (hasChildren) { + return ( + + {hasChildrenElement + ? + : content.text} + + ); + } + return ; +} + +const defaultSlotOptions = { + mergeProps: true, +}; + +function TestPluginSlotWithSlotOptions({ slotOptions: slotOptionsOverride, ...rest }) { + const slotOptions = { + ...defaultSlotOptions, + ...slotOptionsOverride, + }; + return ; +} describe('PluginSlot', () => { beforeEach(() => { @@ -73,7 +148,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 +158,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 +181,8 @@ describe('PluginSlot', () => { { op: PLUGIN_OPERATIONS.Wrap, widgetId: 'default_contents', - wrapper: ({ component, idx }) => ( -
+ wrapper: ({ component }) => ( +
{component}
), @@ -116,11 +191,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 +208,7 @@ describe('PluginSlot', () => { ], keepDefault: true, }); - const { container } = render(TestPluginSlot); + const { container } = render(); const iframeElement = container.querySelector('iframe'); expect(iframeElement).toBeNull(); @@ -153,8 +227,242 @@ describe('PluginSlot', () => { ], keepDefault: true, }); - render(TestPluginSlot); + render(); expect(logError).toHaveBeenCalledWith('the insert operation config is invalid for widget id: invalid_config'); }); + + it('should handle multiple children', () => { + const { queryAllByTestId } = render(); + const defaultContentsWidgets = queryAllByTestId('default_contents'); + expect(defaultContentsWidgets).toHaveLength(2); + defaultContentsWidgets.forEach((widget) => { + expect(widget).toHaveTextContent(content.text); + }); + }); + + it('should handle empty children', () => { + const { getByTestId } = render(); + expect(getByTestId('test-slot-id')).toBeInTheDocument(); + }); + + 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 { From d115d444447b567c752fa541d962c2df9e6a3ed3 Mon Sep 17 00:00:00 2001 From: Adam Stankiewicz Date: Wed, 4 Sep 2024 12:06:07 -0400 Subject: [PATCH 2/6] fix: provide default content object for widget modification --- example/env.config.jsx | 15 +++++++ example/src/ExamplePage.jsx | 6 +++ ...luginSlotWithModifyDefaultContentsLink.jsx | 42 +++++++++++++++++++ src/plugins/data/utils.jsx | 2 +- 4 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 example/src/pluginSlots/PluginSlotWithModifyDefaultContentsLink.jsx diff --git a/example/env.config.jsx b/example/env.config.jsx index 6bf59335..f0463e13 100644 --- a/example/env.config.jsx +++ b/example/env.config.jsx @@ -29,6 +29,11 @@ const modifyWidgetDefaultContents = (widget) => { return widget; }; +const modifyWidgetDefaultContentsLink = (widget) => { + widget.content.href = 'https://openedx.org'; + return widget; +}; + const wrapWidget = ({ component }) => (
@@ -205,6 +210,16 @@ const config = { }, ], }, + slot_with_hyperlink: { + keepDefault: true, + plugins: [ + { + op: PLUGIN_OPERATIONS.Modify, + widgetId: 'default_contents', + fn: modifyWidgetDefaultContentsLink, + }, + ], + }, }, }; diff --git a/example/src/ExamplePage.jsx b/example/src/ExamplePage.jsx index 5d89a3b1..1bce799c 100644 --- a/example/src/ExamplePage.jsx +++ b/example/src/ExamplePage.jsx @@ -2,6 +2,7 @@ import { Container, Row, Col, Stack, } from '@openedx/paragon'; +import PluginSlotWithModifyDefaultContentsLink from './pluginSlots/PluginSlotWithModifyDefaultContentsLink'; import PluginSlotWithModifyDefaultContents from './pluginSlots/PluginSlotWithModifyDefaultContents'; import PluginSlotWithInsert from './pluginSlots/PluginSlotWithInsert'; import PluginSlotWithModifyWrapHide from './pluginSlots/PluginSlotWithModifyWrapHide'; @@ -24,6 +25,11 @@ const pluginExamples = [ label: 'Plugin Operation: Modify Default Content', Component: PluginSlotWithModifyDefaultContents, }, + { + id: 'plugin-operation-modify-default-content-link', + label: 'Plugin Operation: Modify Default Content (Link)', + Component: PluginSlotWithModifyDefaultContentsLink, + }, { id: 'direct-plugins-modular-components', label: 'Direct Plugins Using Modular Components', diff --git a/example/src/pluginSlots/PluginSlotWithModifyDefaultContentsLink.jsx b/example/src/pluginSlots/PluginSlotWithModifyDefaultContentsLink.jsx new file mode 100644 index 00000000..a944b97d --- /dev/null +++ b/example/src/pluginSlots/PluginSlotWithModifyDefaultContentsLink.jsx @@ -0,0 +1,42 @@ +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 LinkExample = ({ href, content, ...rest }) => { + return Hello world; +}; + +function PluginSlotWithModifyDefaultContentsLink({ 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.
    • +
+ + + +
+ ); +} + +export default PluginSlotWithModifyDefaultContentsLink; diff --git a/src/plugins/data/utils.jsx b/src/plugins/data/utils.jsx index ea923ec8..68e4e3be 100644 --- a/src/plugins/data/utils.jsx +++ b/src/plugins/data/utils.jsx @@ -61,7 +61,7 @@ export const organizePlugins = (defaultContents, plugins) => { } else if (change.op === PLUGIN_OPERATIONS.Modify) { const widgetIdx = newContents.findIndex((w) => w.id === change.widgetId); if (widgetIdx >= 0) { - const widget = { ...newContents[widgetIdx] }; + const widget = { content: {}, ...newContents[widgetIdx] }; newContents[widgetIdx] = change.fn(widget); } } else if (change.op === PLUGIN_OPERATIONS.Wrap) { From be1723154407a8968ff3272c6455bc21bcf2d1d9 Mon Sep 17 00:00:00 2001 From: Adam Stankiewicz Date: Wed, 4 Sep 2024 17:36:01 -0400 Subject: [PATCH 3/6] chore: pr feedback --- README.rst | 6 ++---- example/env.config.jsx | 4 ++-- example/src/ExamplePage.jsx | 10 +++++----- .../PluginSlotWithModifyDefaultContents.jsx | 6 +----- .../PluginSlotWithModifyDefaultContentsLink.jsx | 1 - 5 files changed, 10 insertions(+), 17 deletions(-) diff --git a/README.rst b/README.rst index e3abef1c..3494872b 100644 --- a/README.rst +++ b/README.rst @@ -228,13 +228,11 @@ or its priority. The operation requires the id of the widget that will be modifi .. code-block:: const modifyWidget = (widget) => { - const newContent = { + widget.content = { propExampleA: 'University XYZ Sidebar', propExampleB: SomeOtherIcon, }; - const modifiedWidget = widget; - modifiedWidget.content = newContent; - return modifiedWidget; + return widget; }; /* diff --git a/example/env.config.jsx b/example/env.config.jsx index f0463e13..297cda49 100644 --- a/example/env.config.jsx +++ b/example/env.config.jsx @@ -17,7 +17,7 @@ const modifyWidget = (widget) => { return modifiedWidget; }; -const modifyWidgetDefaultContents = (widget) => { +const modifyWidgetDefaultContentsUsernamePII = (widget) => { const newContent = { 'data-custom-attr': 'customValue', 'data-another-custom-attr': '', @@ -206,7 +206,7 @@ const config = { { op: PLUGIN_OPERATIONS.Modify, widgetId: 'default_contents', - fn: modifyWidgetDefaultContents, + fn: modifyWidgetDefaultContentsUsernamePII, }, ], }, diff --git a/example/src/ExamplePage.jsx b/example/src/ExamplePage.jsx index 1bce799c..52790a4e 100644 --- a/example/src/ExamplePage.jsx +++ b/example/src/ExamplePage.jsx @@ -20,16 +20,16 @@ const pluginExamples = [ label: 'Plugin Operation: Modify, Wrap, and Hide', Component: PluginSlotWithModifyWrapHide, }, - { - id: 'plugin-operation-modify-default-content', - label: 'Plugin Operation: Modify Default Content', - Component: PluginSlotWithModifyDefaultContents, - }, { id: 'plugin-operation-modify-default-content-link', label: 'Plugin Operation: Modify Default Content (Link)', Component: PluginSlotWithModifyDefaultContentsLink, }, + { + 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', diff --git a/example/src/pluginSlots/PluginSlotWithModifyDefaultContents.jsx b/example/src/pluginSlots/PluginSlotWithModifyDefaultContents.jsx index 50762013..dbf9a5b0 100644 --- a/example/src/pluginSlots/PluginSlotWithModifyDefaultContents.jsx +++ b/example/src/pluginSlots/PluginSlotWithModifyDefaultContents.jsx @@ -56,12 +56,9 @@ function PluginSlotWithModifyDefaultContents({ id, label }) { id="slot_with_username_pii" as="div" // Default slotOptions - slotOptions={{ - mergeProps: false, - }} > { console.log('Username clicked!', 'prop', e); }} @@ -80,7 +77,6 @@ function PluginSlotWithModifyDefaultContents({ id, label }) { console.log('Username clicked!', 'prop', e); }} /> - {/* // Note: uncomment to see example of incorrectly using multiple `children` elements within `PluginSlot` */} Date: Wed, 4 Sep 2024 18:03:39 -0400 Subject: [PATCH 4/6] chore: consolidate examples --- example/env.config.jsx | 9 +- example/src/ExamplePage.jsx | 6 - .../PluginSlotWithModifyDefaultContents.jsx | 124 ++++++++++-------- ...luginSlotWithModifyDefaultContentsLink.jsx | 41 ------ 4 files changed, 75 insertions(+), 105 deletions(-) delete mode 100644 example/src/pluginSlots/PluginSlotWithModifyDefaultContentsLink.jsx diff --git a/example/env.config.jsx b/example/env.config.jsx index 297cda49..4a40fc94 100644 --- a/example/env.config.jsx +++ b/example/env.config.jsx @@ -8,24 +8,21 @@ import PluginDirect from './src/components/PluginDirect'; import ModularComponent from './src/components/ModularComponent'; const modifyWidget = (widget) => { - const newContent = { + widget.content = { title: 'Modified Modular Plugin', uniqueText: 'Note that the original text defined in the JS config is replaced by this modified one.', }; - const modifiedWidget = widget; - modifiedWidget.content = newContent; - return modifiedWidget; + return widget; }; const modifyWidgetDefaultContentsUsernamePII = (widget) => { - const newContent = { + widget.content = { '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; }; diff --git a/example/src/ExamplePage.jsx b/example/src/ExamplePage.jsx index 52790a4e..5d89a3b1 100644 --- a/example/src/ExamplePage.jsx +++ b/example/src/ExamplePage.jsx @@ -2,7 +2,6 @@ import { Container, Row, Col, Stack, } from '@openedx/paragon'; -import PluginSlotWithModifyDefaultContentsLink from './pluginSlots/PluginSlotWithModifyDefaultContentsLink'; import PluginSlotWithModifyDefaultContents from './pluginSlots/PluginSlotWithModifyDefaultContents'; import PluginSlotWithInsert from './pluginSlots/PluginSlotWithInsert'; import PluginSlotWithModifyWrapHide from './pluginSlots/PluginSlotWithModifyWrapHide'; @@ -20,11 +19,6 @@ const pluginExamples = [ label: 'Plugin Operation: Modify, Wrap, and Hide', Component: PluginSlotWithModifyWrapHide, }, - { - id: 'plugin-operation-modify-default-content-link', - label: 'Plugin Operation: Modify Default Content (Link)', - Component: PluginSlotWithModifyDefaultContentsLink, - }, { id: 'plugin-operation-modify-default-content', label: 'Plugin Operation: Modify Default Content', diff --git a/example/src/pluginSlots/PluginSlotWithModifyDefaultContents.jsx b/example/src/pluginSlots/PluginSlotWithModifyDefaultContents.jsx index dbf9a5b0..c0aca6d7 100644 --- a/example/src/pluginSlots/PluginSlotWithModifyDefaultContents.jsx +++ b/example/src/pluginSlots/PluginSlotWithModifyDefaultContents.jsx @@ -2,9 +2,14 @@ import React from 'react'; import { PluginSlot } from '@openedx/frontend-plugin-framework'; import classNames from 'classnames'; +import { Stack } from '@openedx/paragon'; -// Example component used as the default childen within a PluginSlot +// Example sub-components used as the default childen within a PluginSlot +const LinkExample = ({ href, content, ...rest }) => { + return Hello world; +}; + const Username = ({ className, onClick, ...rest }) => { const authenticatedUser = { username: 'testuser' }; const { username } = authenticatedUser; @@ -21,7 +26,6 @@ const Username = ({ className, onClick, ...rest }) => { ); }; - const UsernameWithPluginContent = ({ className, onClick, content = {} }) => { const { className: classNameFromPluginContent, @@ -38,64 +42,80 @@ const UsernameWithPluginContent = ({ className, onClick, content = {} }) => { return ; }; +// Example PluginSlot usage. 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 + the widgetId is default_contents. Any configured, custom plugin content may be 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); - }} - /> - + + +
+

Basic

+ + + +
+
+

Advanced

+

When adding custom props to default_contents, there may be special cases regarding how custom props are merged with existing props, including:

+
    +
  • 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); + }} + /> + +
+
); } diff --git a/example/src/pluginSlots/PluginSlotWithModifyDefaultContentsLink.jsx b/example/src/pluginSlots/PluginSlotWithModifyDefaultContentsLink.jsx deleted file mode 100644 index 1c8604e4..00000000 --- a/example/src/pluginSlots/PluginSlotWithModifyDefaultContentsLink.jsx +++ /dev/null @@ -1,41 +0,0 @@ -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 LinkExample = ({ href, content, ...rest }) => { - return Hello world; -}; - -function PluginSlotWithModifyDefaultContentsLink({ 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.
    • -
- - - -
- ); -} - -export default PluginSlotWithModifyDefaultContentsLink; From 275d3a48961f74f94243b959e063e24bf18c058e Mon Sep 17 00:00:00 2001 From: Adam Stankiewicz Date: Wed, 4 Sep 2024 18:06:15 -0400 Subject: [PATCH 5/6] chore: remove content prop from LinkExample --- example/src/pluginSlots/PluginSlotWithModifyDefaultContents.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/example/src/pluginSlots/PluginSlotWithModifyDefaultContents.jsx b/example/src/pluginSlots/PluginSlotWithModifyDefaultContents.jsx index c0aca6d7..3d74a924 100644 --- a/example/src/pluginSlots/PluginSlotWithModifyDefaultContents.jsx +++ b/example/src/pluginSlots/PluginSlotWithModifyDefaultContents.jsx @@ -6,7 +6,7 @@ import { Stack } from '@openedx/paragon'; // Example sub-components used as the default childen within a PluginSlot -const LinkExample = ({ href, content, ...rest }) => { +const LinkExample = ({ href, ...rest }) => { return Hello world; }; From 751831b91a69b6d7e013fd6eedc57e53566e0866 Mon Sep 17 00:00:00 2001 From: Adam Stankiewicz Date: Thu, 5 Sep 2024 16:01:42 -0400 Subject: [PATCH 6/6] chore: pr feedback, updating comment --- src/plugins/PluginSlot.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/PluginSlot.jsx b/src/plugins/PluginSlot.jsx index 9819b1e0..8bd87a35 100644 --- a/src/plugins/PluginSlot.jsx +++ b/src/plugins/PluginSlot.jsx @@ -61,7 +61,7 @@ const PluginSlot = forwardRef(({ if (!pluginConfig.hidden) { let container; // If default content, render children (merging any custom defined props from - // pluginConfig.content with existing props, if any, on `RenderWidget`). + // pluginConfig.content with any existing props on `RenderWidget`). if (pluginConfig.id === 'default_contents') { const propsForRenderWidget = (pluginConfig.RenderWidget && React.isValidElement(pluginConfig.RenderWidget)) ? pluginConfig.RenderWidget.props