generated from openedx/frontend-template-application
-
Notifications
You must be signed in to change notification settings - Fork 15
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
feat: add direct plugin framework #16
Merged
Merged
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
3ec478f
feat: WIP add file structure and write mocks for tests
jsnwesson 1da6b94
test: add testing for the DirectPluginOperations logic
jsnwesson af931ed
feat: WIP add file structure and write mocks for tests
jsnwesson ee48f04
test: add testing for the DirectPluginOperations logic
jsnwesson 10add12
Merge branch 'jwesson/add-direct-plugin-framework' of github.com:open…
jsnwesson 17d1fd9
test: add tests for DirectOperation.Wrap and error handling
jsnwesson 6379a9c
test: add test for priority sort of organizePlugins
jsnwesson c6d63ce
refactor: reorganize and add/remove comments for next steps and defin…
jsnwesson ead0181
feat: add Direct Plugin Slot logic
jsnwesson 967e674
fix: import issue
jsnwesson 75423f4
docs: clarify some language around defining what plugin slot changes …
jsnwesson 7b0326e
refactor: change some variable naming and remove unused code
jsnwesson File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
import React from 'react'; | ||
|
||
/** | ||
* Context which makes the list of all plugin changes (allPluginChanges) available to the <DirectSlot> components | ||
* below it in the React tree | ||
*/ | ||
export const DirectPluginContext = React.createContext([]); | ||
|
||
/** | ||
* @description DirectPluginOperation defines the changes to be made to either the default widget(s) or to any | ||
* that are inserted | ||
* @property {string} Insert - inserts a new widget into the DirectPluginSlot | ||
* @property {string} Hide - used to hide a default widget based on the widgetId | ||
* @property {string} Modify - used to modify/replace a widget's content | ||
* @property {string} Wrap - wraps a widget with a React element or fragment | ||
* | ||
*/ | ||
|
||
export const DirectPluginOperations = { | ||
Insert: 'insert', | ||
Hide: 'hide', | ||
Modify: 'modify', | ||
Wrap: 'wrap', | ||
}; | ||
|
||
/** | ||
This is what the allSlotChanges configuration should look like when passed into DirectPluginContext | ||
{ | ||
id: "allDirectPluginChanges", | ||
getDirectSlotChanges() { | ||
return { | ||
"main-nav": [ | ||
// Hide the "Drafts" link, except for administrators: | ||
{ | ||
op: DirectPluginChangeOperation.Wrap, | ||
widgetId: "drafts", | ||
wrapper: HideExceptForAdmin, | ||
}, | ||
// Add a new login link: | ||
{ | ||
op: DirectPluginChangeOperation.Insert, | ||
widget: { id: "login", priority: 50, content: { | ||
url: "/login", icon: "person-fill", label: <FormattedMessage defaultMessage="Login" /> | ||
}}, | ||
}, | ||
], | ||
}; | ||
}, | ||
}; | ||
*/ | ||
|
||
/** | ||
This is what a slotChanges configuration should include depending on the operation: | ||
slotChanges = [ | ||
{ op: DirectPluginOperation.Insert; widget: <DirectSlotWidget object> }, | ||
{ op: DirectPluginOperation.Hide; widgetId: string }, | ||
{ op: DirectPluginOperation.Modify; widgetId: string, fn: (widget: <DirectSlotWidget>) => <DirectSlotWidget> }, | ||
{ op: DirectPluginOperation.Wrap; widgetId: string, wrapper: React.FC<{widget: React.ReactElement }> }, | ||
] | ||
*/ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
import React from 'react'; | ||
import PropTypes from 'prop-types'; | ||
|
||
import { DirectPluginContext } from './DirectPlugin'; | ||
import organizePlugins from './utils'; | ||
|
||
const DirectPluginSlot = ({ defaultContents, slotId, renderWidget }) => { | ||
const allPluginChanges = React.useContext(DirectPluginContext); | ||
|
||
const contents = React.useMemo(() => { | ||
const slotChanges = allPluginChanges.getDirectSlotChanges()[slotId] ?? []; | ||
return organizePlugins(defaultContents, slotChanges); | ||
}, [allPluginChanges, defaultContents, slotId]); | ||
|
||
return ( | ||
<> | ||
{contents.map((c) => { | ||
jsnwesson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (c.hidden) { | ||
return null; | ||
} | ||
if (c.wrappers) { | ||
// TODO: define how the reduce logic is able to wrap widgets and make it testable | ||
// eslint-disable-next-line max-len | ||
return c.wrappers.reduce((widget, wrapper) => React.createElement(wrapper, { widget, key: c.id }), renderWidget(c)); | ||
} | ||
return renderWidget(c); | ||
})} | ||
</> | ||
); | ||
}; | ||
|
||
DirectPluginSlot.propTypes = { | ||
defaultContents: PropTypes.shape([]), | ||
slotId: PropTypes.string.isRequired, | ||
renderWidget: PropTypes.func.isRequired, | ||
}; | ||
|
||
DirectPluginSlot.defaultProps = { | ||
defaultContents: [], | ||
}; | ||
|
||
export default DirectPluginSlot; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
export { | ||
default as DirectPluginSlot, | ||
} from './DirectPluginSlot'; | ||
export { | ||
DirectPluginContext, | ||
DirectPluginOperations, | ||
} from './DirectPlugin'; | ||
export { | ||
default as organizePlugins, | ||
} from './utils'; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
// eslint-disable-next-line import/no-extraneous-dependencies | ||
import { Icon } from '@edx/paragon'; | ||
import { DirectPluginSlot, DirectPluginContext } from '..'; | ||
import { navLinksPlugin } from './PluginComponentsMock'; | ||
|
||
// TODO: remove DirectPluginsContext and enabledPlugins from here once we have an example app | ||
// these simply demonstrate how the root App would have needed this setup in order to pass in the plugin config | ||
// and make it available to a given DirectPluginSlot | ||
|
||
const enabledPlugins = [ | ||
navLinksPlugin, | ||
]; | ||
|
||
const MyApp = () => ( | ||
<DirectPluginContext value={enabledPlugins}> | ||
<div> | ||
<DirectPluginSlot | ||
slotId="side-bar-nav" | ||
defaultContents={navLinksPlugin.defaultLinks} | ||
jsnwesson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
renderWidget={(link) => ( | ||
<a | ||
href={link.content.url} | ||
key={link.id} | ||
> | ||
<Icon src={link.content.icon} /> {link.content.label} | ||
</a> | ||
)} | ||
/> | ||
</div> | ||
</DirectPluginContext> | ||
); | ||
|
||
export default MyApp; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
/* eslint-disable react/prop-types */ | ||
// eslint-disable-next-line import/no-extraneous-dependencies | ||
import React from 'react'; | ||
import { | ||
House, Star, InsertDriveFile, Login, | ||
} from '@edx/paragon/icons'; | ||
import { DirectPluginOperations } from '..'; | ||
|
||
/** This is for us to be able to mock in tests */ | ||
const isAdminHelper = () => true; | ||
jsnwesson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/** This is a React widget that wraps its children and makes them visible only to administrators */ | ||
const HideExceptForAdmin = ({ widget }) => { | ||
const isAdmin = isAdminHelper(); | ||
return <React.Fragment key={widget.key}>{isAdmin ? widget : null}</React.Fragment>; | ||
}; | ||
|
||
const navLinksPlugin = { | ||
id: 'links-demo', // id isn't used anywhere, but can be extended to | ||
defaultComponentProps: [ | ||
{ | ||
id: 'home', | ||
priority: 5, | ||
content: { url: '/', icon: House, label: 'Home' }, | ||
}, | ||
{ | ||
id: 'lookup', | ||
priority: 25, | ||
content: { url: '/lookup', icon: Star, label: 'Lookup' }, | ||
}, | ||
{ | ||
id: 'drafts', | ||
priority: 35, | ||
content: { url: '/drafts', icon: InsertDriveFile, label: 'Drafts' }, | ||
}, | ||
], | ||
getDirectSlotChanges() { | ||
return { | ||
'side-bar-nav': [ // slot id that is used by directpluginslot | ||
// Hide the "Drafts" link, except for administrators: | ||
{ | ||
op: DirectPluginOperations.Wrap, | ||
widgetId: 'drafts', | ||
wrapper: HideExceptForAdmin, | ||
}, | ||
// Add a new login link after the rest of default plugins: | ||
{ | ||
op: DirectPluginOperations.Insert, | ||
widget: { | ||
id: 'login', | ||
priority: 50, | ||
content: { | ||
url: '/login', icon: Login, label: 'Login', | ||
}, | ||
}, | ||
}, | ||
], | ||
}; | ||
}, | ||
}; | ||
|
||
export { | ||
isAdminHelper, | ||
navLinksPlugin, | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
import { DirectPluginOperations } from './DirectPlugin'; | ||
|
||
/** | ||
* Called by DirectPluginSlot to prepare the plugin changes for the given slot | ||
* | ||
* @param {Array} defaultContents - The default widgets where the plugin slot exists. | ||
* @param {Array} slotChanges - All of the changes assigned to the specific plugin slot | ||
* @returns {Array} - A sorted list of widgets with any additional properties needed to render them in the plugin slot | ||
*/ | ||
const organizePlugins = (defaultContents, slotChanges) => { | ||
const newContents = [...defaultContents]; | ||
|
||
slotChanges.forEach(change => { | ||
if (change.op === DirectPluginOperations.Insert) { | ||
newContents.push(change.widget); | ||
} else if (change.op === DirectPluginOperations.Hide) { | ||
const widget = newContents.find((w) => w.id === change.widgetId); | ||
if (widget) { widget.hidden = true; } | ||
} else if (change.op === DirectPluginOperations.Modify) { | ||
const widgetIdx = newContents.findIndex((w) => w.id === change.widgetId); | ||
if (widgetIdx >= 0) { | ||
const widget = { ...newContents[widgetIdx] }; | ||
newContents[widgetIdx] = change.fn(widget); | ||
} | ||
} else if (change.op === DirectPluginOperations.Wrap) { | ||
const widgetIdx = newContents.findIndex((w) => w.id === change.widgetId); | ||
if (widgetIdx >= 0) { | ||
const newWidget = { wrappers: [], ...newContents[widgetIdx] }; | ||
newWidget.wrappers.push(change.wrapper); | ||
newContents[widgetIdx] = newWidget; | ||
} | ||
} else { | ||
throw new Error('unknown direct plugin change operation'); | ||
} | ||
}); | ||
|
||
newContents.sort((a, b) => (a.priority - b.priority) * 10_000 + a.id.localeCompare(b.id)); | ||
return newContents; | ||
}; | ||
|
||
export default organizePlugins; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The one thing that I decided to change in order to steer away from Braden's approach and move closer to eventually using a JS-based config is that his configuration assumed that there'd be multiple
allSlotChanges
for a Host MFE, and I think whatever benefit that had could've eventually led to confusion/oversight.I'd argue that keeping all known changes in this one object is easier to track, especially as they'll ideally live in a single JS config anyways.