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 all 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 preparedWidgets = React.useMemo(() => { | ||
const slotChanges = allPluginChanges.getDirectSlotChanges()[slotId] ?? []; | ||
return organizePlugins(defaultContents, slotChanges); | ||
}, [allPluginChanges, defaultContents, slotId]); | ||
|
||
return ( | ||
<> | ||
{preparedWidgets.map((preppedWidget) => { | ||
if (preppedWidget.hidden) { | ||
return null; | ||
} | ||
if (preppedWidget.wrappers) { | ||
// TODO: define how the reduce logic is able to wrap widgets and make it testable | ||
// eslint-disable-next-line max-len | ||
return preppedWidget.wrappers.reduce((widget, wrapper) => React.createElement(wrapper, { widget, key: preppedWidget.id }), renderWidget(preppedWidget)); | ||
} | ||
return renderWidget(preppedWidget); | ||
})} | ||
</> | ||
); | ||
}; | ||
|
||
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,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; |
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,173 @@ | ||
import '@testing-library/jest-dom'; | ||
|
||
import organizePlugins from './utils'; | ||
import { DirectPluginOperations } from './DirectPlugin'; | ||
|
||
const mockModifyWidget = (widget) => { | ||
const newContent = { | ||
url: '/search', | ||
label: 'Search', | ||
}; | ||
const modifiedWidget = widget; | ||
modifiedWidget.content = newContent; | ||
return modifiedWidget; | ||
}; | ||
|
||
function mockWrapWidget({ widget }) { | ||
const isAdmin = true; | ||
return isAdmin ? widget : null; | ||
} | ||
|
||
const mockSlotChanges = [ | ||
{ | ||
op: DirectPluginOperations.Wrap, | ||
widgetId: 'drafts', | ||
wrapper: mockWrapWidget, | ||
}, | ||
{ | ||
op: DirectPluginOperations.Hide, | ||
widgetId: 'home', | ||
}, | ||
{ | ||
op: DirectPluginOperations.Modify, | ||
widgetId: 'lookUp', | ||
fn: mockModifyWidget, | ||
}, | ||
{ | ||
op: DirectPluginOperations.Insert, | ||
widget: { | ||
id: 'login', | ||
priority: 50, | ||
content: { | ||
url: '/login', label: 'Login', | ||
}, | ||
}, | ||
}, | ||
]; | ||
|
||
const mockDefaultContent = [ | ||
{ | ||
id: 'home', | ||
priority: 5, | ||
content: { url: '/', label: 'Home' }, | ||
}, | ||
{ | ||
id: 'lookUp', | ||
priority: 25, | ||
content: { url: '/lookup', label: 'Lookup' }, | ||
}, | ||
{ | ||
id: 'drafts', | ||
priority: 35, | ||
content: { url: '/drafts', label: 'Drafts' }, | ||
}, | ||
]; | ||
|
||
describe('organizePlugins', () => { | ||
describe('when there is no defaultContent', () => { | ||
afterEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
it('should return an empty array when there are no changes or additions to slot', () => { | ||
const plugins = organizePlugins([], []); | ||
expect(plugins.length).toBe(0); | ||
expect(plugins).toEqual([]); | ||
}); | ||
|
||
it('should return an array of changes for non-default plugins', () => { | ||
const plugins = organizePlugins([], mockSlotChanges); | ||
expect(plugins.length).toEqual(1); | ||
expect(plugins[0].id).toEqual('login'); | ||
}); | ||
}); | ||
|
||
describe('when there is defaultContent', () => { | ||
afterEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
it('should return an array of defaultContent if no changes for plugins in slot', () => { | ||
const plugins = organizePlugins(mockDefaultContent, []); | ||
expect(plugins.length).toEqual(3); | ||
expect(plugins).toEqual(mockDefaultContent); | ||
}); | ||
|
||
it('should remove plugins with DirectOperation.Hide', () => { | ||
const plugins = organizePlugins(mockDefaultContent, mockSlotChanges); | ||
const widget = plugins.find((w) => w.id === 'home'); | ||
expect(plugins.length).toEqual(4); | ||
expect(widget.hidden).toBe(true); | ||
}); | ||
|
||
it('should modify plugins with DirectOperation.Modify', () => { | ||
const plugins = organizePlugins(mockDefaultContent, mockSlotChanges); | ||
const widget = plugins.find((w) => w.id === 'lookUp'); | ||
|
||
expect(plugins.length).toEqual(4); | ||
expect(widget.content.url).toEqual('/search'); | ||
}); | ||
|
||
it('should wrap plugins with DirectOperation.Wrap', () => { | ||
const plugins = organizePlugins(mockDefaultContent, mockSlotChanges); | ||
const widget = plugins.find((w) => w.id === 'drafts'); | ||
expect(plugins.length).toEqual(4); | ||
expect(widget.wrappers.length).toEqual(1); | ||
}); | ||
|
||
it('should accept several wrappers for a single plugin with DirectOperation.Wrap', () => { | ||
const newMockWrapComponent = ({ widget }) => { | ||
const isStudent = false; | ||
return isStudent ? null : widget; | ||
}; | ||
const newPluginChange = { | ||
op: DirectPluginOperations.Wrap, | ||
widgetId: 'drafts', | ||
wrapper: newMockWrapComponent, | ||
}; | ||
mockSlotChanges.push(newPluginChange); | ||
const plugins = organizePlugins(mockDefaultContent, mockSlotChanges); | ||
const widget = plugins.find((w) => w.id === 'drafts'); | ||
expect(plugins.length).toEqual(4); | ||
expect(widget.wrappers.length).toEqual(2); | ||
expect(widget.wrappers[0]).toEqual(mockWrapWidget); | ||
expect(widget.wrappers[1]).toEqual(newMockWrapComponent); | ||
}); | ||
|
||
it('should return plugins arranged by priority', () => { | ||
const newPluginChange = { | ||
op: DirectPluginOperations.Insert, | ||
widget: { | ||
id: 'profile', | ||
priority: 1, | ||
content: { | ||
url: '/profile', label: 'Profile', | ||
}, | ||
}, | ||
}; | ||
mockSlotChanges.push(newPluginChange); | ||
const plugins = organizePlugins(mockDefaultContent, mockSlotChanges); | ||
expect(plugins.length).toEqual(5); | ||
expect(plugins[0].id).toBe('profile'); | ||
expect(plugins[1].id).toBe('home'); | ||
expect(plugins[2].id).toBe('lookUp'); | ||
expect(plugins[3].id).toBe('drafts'); | ||
expect(plugins[4].id).toBe('login'); | ||
}); | ||
|
||
it('should raise an error for an operation that does not exist', async () => { | ||
const badPluginChange = { | ||
op: DirectPluginOperations.Destroy, | ||
widgetId: 'drafts', | ||
}; | ||
mockSlotChanges.push(badPluginChange); | ||
|
||
expect.assertions(1); | ||
try { | ||
await organizePlugins(mockDefaultContent, mockSlotChanges); | ||
} catch (error) { | ||
expect(error.message).toBe('unknown direct plugin change operation'); | ||
} | ||
}); | ||
}); | ||
}); |
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.