Mfrank/rename variables (#17)

* fix: rename variables

* docs: update readme

---------

Co-authored-by: Maxwell Frank <[email protected]>

README.rst

@@ -29,21 +29,24 @@ Getting Started
 1. Add Library Dependency
 -------------------------
 
-Add `@edx/frontend-plugin-framework` to the `package.json` of both Host and Child MFEs.
+Add ``@edx/frontend-plugin-framework`` to the ``package.json`` of both Host and Child MFEs.
 
 Micro-frontend configuration document (JS)
 ------------------------------------------
 
 Micro-frontends that would like to use the Plugin Framework need to be configured via a JavaScript configuration
-document and a `plugins` config. Technically, only the Host MFE would require an `env.config.js` file with a `plugins` config.
-Keep in mind that since any Child MFE can theoretically also contain its own PluginSlot, it will eventually need its own
-JavaScript configuration.
+document and a ``pluginSlots`` config. Technically, only the Host MFE requires an ``env.config.js`` file with a ``pluginSlots`` config.
+
+However, note that any Child MFE can theoretically contain one or more ``PluginSlot`` components, thereby making it both a Child MFE and a Host MFE.
+In this instance, it would have its own JavaScript file to configure the ``PluginSlot``.
+
+For more information on how JS based configuration works, see the `config.js`_ file in frontend-platform.
 
   .. code-block::
 
     const config = {
       // other existing configuration
-      plugins: {
+      pluginSlots: {
         sidebar: {
           keepDefault: false, // bool to keep default host content
           plugins: [
@@ -56,10 +59,12 @@ JavaScript configuration.
       }
     }
 
+.. _config.js: https://github.com/openedx/frontend-platform/blob/556424ee073e0629d7331046bbd7714d0d241f43/src/config.js
+
 Host Micro-frontend (JSX)
 -------------------------
 
-Hosts must define PluginSlot components in areas of the UI where they intend to accept extensions.
+Hosts must define ``PluginSlot`` components in areas of the UI where they intend to accept extensions.
 The Host MFE, and thus the owners of the Host MFE, are responsible for deciding where it is acceptable to mount a plugin.
 They also decide the dimensions, responsiveness/scrolling policy, and whether the slot supports passing any additional
 data to the plugin as part of its contract.
@@ -92,12 +97,12 @@ Plugin Micro-frontend (JSX) and Fallback Behavior
 -------------------------------------------------
 
 The plugin MFE is no different than any other MFE except that it defines a Plugin component as a child of a route.
-This component is responsible for communicating (via postMessage) with the host page and resizing its content to match
+This component is responsible for communicating (via ``postMessage``) with the host page and resizing its content to match
 the dimensions available in the host’s PluginSlot. 
 
 It’s notoriously difficult to know in the host application when an iFrame has failed to load.
 Because of security sandboxing, the host isn’t allowed to know the HTTP status of the request or to inspect what was
-loaded, so we have to rely on waiting for a postMessage event from within the iFrame to know it has successfully loaded.
+loaded, so we have to rely on waiting for a ``postMessage`` event from within the iFrame to know it has successfully loaded.
 For the fallback content, the Plugin-owning team would pass a fallback component into the Plugin tag that is wrapped around their component, as noted below. Otherwise, a default fallback component would be used.
   .. code-block::
 

src/plugins/Plugin.jsx

@@ -16,7 +16,7 @@ import {
 import { PLUGIN_RESIZE } from './data/constants';
 import messages from './Plugins.messages';
 
-// TODO: create example-plugin-app/src/PluginOne.jsx for example of customizing errorFallback
+// TODO: create example-plugin-app/src/PluginOne.jsx for example of customizing errorFallback as part of APER-3042 https://2u-internal.atlassian.net/browse/APER-3042
 const ErrorFallbackDefault = ({ intl }) => (
   <div>
     <h2>
@@ -25,9 +25,8 @@ const ErrorFallbackDefault = ({ intl }) => (
   </div>
 );
 
-// TODO: find out where "ready" comes from
 const Plugin = ({
-  children, className, intl, style, ready, ErrorFallbackComponent,
+  children, className, style, ready, ErrorFallbackComponent, intl,
 }) => {
   const [dimensions, setDimensions] = useState({
     width: null,
@@ -59,7 +58,8 @@ const Plugin = ({
   }, []);
 
   useEffect(() => {
-    // TODO: find out where "ready" comes from and when it would be true
+    /** Ready defaults to true, but can be used to defer rendering the Plugin until certain processes have
+     * occurred or conditions have been met */
     if (ready) {
       dispatchReadyEvent();
     }
@@ -68,8 +68,6 @@ const Plugin = ({
   return (
     <div className={className} style={finalStyle}>
       <ErrorBoundary
-      // Must be React Component format (<ComponentName ...props />) or it won't render
-      // TODO: update frontend-platform code to refactor <ErrorBoundary /> or include info in docs somewhere
         fallbackComponent={<ErrorFallback intl={intl} />}
       >
         {children}
@@ -81,12 +79,18 @@ const Plugin = ({
 export default injectIntl(Plugin);
 
 Plugin.propTypes = {
+  /** The content for the Plugin */
   children: PropTypes.node.isRequired,
+  /** Classes to apply to the Plugin wrapper component */
   className: PropTypes.string,
+  /** Custom error fallback component */
   ErrorFallbackComponent: PropTypes.func,
-  intl: intlShape.isRequired,
+  /** If ready is true, it will render the Plugin */
   ready: PropTypes.bool,
-  style: PropTypes.object, // eslint-disable-line
+  /** Styles to apply to the Plugin wrapper component */
+  style: PropTypes.shape({}),
+  /** i18n  */
+  intl: intlShape.isRequired,
 };
 
 Plugin.defaultProps = {

src/plugins/Plugin.test.jsx

@@ -61,7 +61,6 @@ describe('PluginContainer', () => {
     // Ensure the iframe has the proper attributes
     expect(iframeElement.attributes.getNamedItem('allow').value).toEqual(IFRAME_FEATURE_POLICY);
     expect(iframeElement.attributes.getNamedItem('src').value).toEqual(iframeConfig.url);
-    expect(iframeElement.attributes.getNamedItem('scrolling').value).toEqual('auto');
     expect(iframeElement.attributes.getNamedItem('title').value).toEqual(title);
     // The component isn't ready, since the class has 'd-none'
     expect(iframeElement.attributes.getNamedItem('class').value).toEqual('border border-0 w-100 d-none');

src/plugins/PluginContainer.jsx

@@ -35,6 +35,7 @@ const PluginContainer = ({ config, ...props }) => {
 export default PluginContainer;
 
 PluginContainer.propTypes = {
+  /** Configuration for the Plugin in this container — i.e pluginSlot[id].example_plugin */
   config: pluginConfigShape,
 };
 

src/plugins/PluginContainerIframe.jsx

@@ -35,7 +35,7 @@ const PluginContainerIframe = ({
   config, fallback, className, ...props
 }) => {
   const { url } = config;
-  const { title, scrolling } = props;
+  const { title } = props;
   const [mounted, setMounted] = useState(false);
   const [ready, setReady] = useState(false);
 
@@ -68,7 +68,6 @@ const PluginContainerIframe = ({
         title={title}
         src={url}
         allow={IFRAME_FEATURE_POLICY}
-        scrolling={scrolling}
         referrerPolicy="origin" // The sent referrer will be limited to the origin of the referring page: its scheme, host, and port.
         className={classNames(
           'border border-0 w-100',
@@ -85,17 +84,19 @@ const PluginContainerIframe = ({
 export default PluginContainerIframe;
 
 PluginContainerIframe.propTypes = {
+  /** Configuration for the Plugin in this container — i.e pluginSlot[id].example_plugin */
   config: pluginConfigShape,
+  /** Custom fallback component used when component is not ready (i.e. "loading") */
   fallback: PropTypes.node,
-  scrolling: PropTypes.oneOf(['auto', 'yes', 'no']),
+  /** Accessible label for the iframe */
   title: PropTypes.string,
+  /** Classes to apply to the iframe */
   className: PropTypes.string,
 };
 
 PluginContainerIframe.defaultProps = {
   config: null,
   fallback: null,
-  scrolling: 'auto',
   title: null,
   className: null,
 };

src/plugins/PluginSlot.jsx

@@ -16,6 +16,7 @@ import PluginContainer from './PluginContainer';
 const PluginSlot = forwardRef(({
   as, id, intl, pluginProps, children, ...props
 }, ref) => {
+  /** TODO: Examples still need to be set up as part of APER-3042 https://2u-internal.atlassian.net/browse/APER-3042 */
   /* 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
   */
@@ -62,10 +63,15 @@ const PluginSlot = forwardRef(({
 export default injectIntl(PluginSlot);
 
 PluginSlot.propTypes = {
+  /** Element type for the PluginSlot wrapper component */
   as: PropTypes.elementType,
+  /** Default content for the PluginSlot */
   children: PropTypes.node,
+  /** ID of the PluginSlot configuration */
   id: PropTypes.string.isRequired,
+  /** i18n  */
   intl: intlShape.isRequired,
+  /** Props that are passed down to each Plugin in the Slot */
   pluginProps: PropTypes.object, // eslint-disable-line
 };
 

src/plugins/data/hooks.js

@@ -15,8 +15,8 @@ import { PLUGIN_MOUNTED, PLUGIN_READY, PLUGIN_UNMOUNTED } from './constants';
  * @returns {Object} - JS configuration for the PluginSlot
  */
 export function usePluginSlot(id) {
-  if (getConfig().plugins[id] !== undefined) {
-    return getConfig().plugins[id];
+  if (getConfig().pluginSlots[id] !== undefined) {
+    return getConfig().pluginSlots[id];
   }
   return { keepDefault: true, plugins: [] };
 }