Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Mfrank/rename variables #17

Merged
merged 5 commits into from
Jan 18, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,21 @@ 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
document and a ``pluginSlots`` config. Technically, only the Host MFE would require an ``env.config.js`` file with a ``pluginSlots`` config.
Keep in mind that since any Child MFE can theoretically also contain its own ``PluginSlot``, it will eventually need its own
JavaScript configuration.

.. code-block::

const config = {
// other existing configuration
plugins: {
pluginSlots: {
sidebar: {
keepDefault: false, // bool to keep default host content
plugins: [
Expand All @@ -59,7 +59,7 @@ JavaScript configuration.
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.
Expand Down Expand Up @@ -92,12 +92,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::

Expand Down
31 changes: 31 additions & 0 deletions src/example/example.env.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
The Frontend Plugin Framework uses the env.config.js file to extract the layout configuration for the plugins.

The PluginSlot will find the configuration by the provided ID which matches a key inside the `pluginSlots` object

Note: having both .env and env.config.js files will follow a predictable order, in which non-empty values in the
JS-based config will overwrite the .env environment variables.

frontend-platform's getConfig loads configuration in the following sequence:
- .env file config
- optional handlers (commonly used to merge MFE-specific config in via additional process.env variables)
- env.config.js file config
- runtime config
*/

/** TODO: Examples still need to be set up as part of APER-3042 https://2u-internal.atlassian.net/browse/APER-3042 */

Choose a reason for hiding this comment

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

Do we still need this TODO and commented out code in here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point on this commented out code in here. Could a lot of this exist in the README since there's already a section there about how the config should be set up?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes good call outs. I will move it to the README. I blame my mucus infused brain for attempting to push a full file of comments 😅


// module.exports = {
// pluginSlots: {
// example_plugin_slot: {
// keepDefault: false,
// plugins: [
// {
// id: 'example_plugin',
// type: 'IFRAME_PLUGIN',
// url: 'http://localhost:{PORT}/{ROUTE}',
// },
// ],
// },
// },
// };
20 changes: 12 additions & 8 deletions src/plugins/Plugin.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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,
Expand Down Expand Up @@ -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();
}
Expand All @@ -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}
Expand All @@ -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 = {
Expand Down
1 change: 0 additions & 1 deletion src/plugins/Plugin.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
1 change: 1 addition & 0 deletions src/plugins/PluginContainer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down
9 changes: 5 additions & 4 deletions src/plugins/PluginContainerIframe.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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',
Expand All @@ -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']),
Copy link
Member Author

Choose a reason for hiding this comment

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

This attribute has been deprecated for iframes. I think it may have been causing some of the scrollbars we were seeing before.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/iframe#deprecated_attributes

/** 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,
};
6 changes: 6 additions & 0 deletions src/plugins/PluginSlot.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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
};

Expand Down
4 changes: 2 additions & 2 deletions src/plugins/data/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
* @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];

Check warning on line 19 in src/plugins/data/hooks.js

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/hooks.js#L19

Added line #L19 was not covered by tests
}
return { keepDefault: true, plugins: [] };
}
Expand Down