-
Notifications
You must be signed in to change notification settings - Fork 124
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: Added PluginSlot to wrap IDVerificationPage component #1097
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
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
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
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,45 @@ | ||
# Footer Slot | ||
|
||
### Slot ID: `id_verification_page_plugin` | ||
|
||
## Description | ||
|
||
This slot is used to replace/modify the IDV Page. | ||
|
||
The implementation of the `IdVerificationPageSlot` component lives in `src/plugin-slots/IdVerificationPageSlot/index.jsx`. | ||
|
||
## Example | ||
|
||
The following `env.config.jsx` will replace the default IDV Page. | ||
|
||
![Screenshot of Default IDV Page](./images/default_id-verification-page.png) | ||
|
||
```jsx | ||
import { DIRECT_PLUGIN, PLUGIN_OPERATIONS } from '@openedx/frontend-plugin-framework'; | ||
|
||
const config = { | ||
pluginSlots: { | ||
id_verification_page_plugin: { | ||
plugins: [ | ||
{ | ||
// Insert a custom IDV Page | ||
op: PLUGIN_OPERATIONS.Insert, | ||
widget: { | ||
id: 'id_verification_page_plugin', | ||
type: DIRECT_PLUGIN, | ||
RenderWidget: () => ( | ||
<div> | ||
<p>This is the new IDV page</p> | ||
<a href="/">Go Home</a> | ||
</div> | ||
), | ||
}, | ||
}, | ||
], | ||
}, | ||
}, | ||
}; | ||
|
||
export default config; | ||
|
||
``` |
Binary file added
BIN
+8.41 KB
src/plugin-slots/IdVerificationPageSlot/images/custom_id-verification-page.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added
BIN
+220 KB
src/plugin-slots/IdVerificationPageSlot/images/default_id-verification-page.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
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 @@ | ||
import { PluginSlot } from '@openedx/frontend-plugin-framework'; | ||
import IdVerificationPage from '../../id-verification'; | ||
|
||
const IdVerificationPageSlot = () => ( | ||
<PluginSlot id="id_verification_page_plugin"> | ||
<IdVerificationPage /> | ||
</PluginSlot> | ||
); | ||
|
||
export default IdVerificationPageSlot; |
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 |
---|---|---|
@@ -1,3 +1,4 @@ | ||
# `frontend-app-account` Plugin Slots | ||
|
||
* [`footer_slot`](./FooterSlot/) | ||
* [`id_verification_page_plugin`](./IdVerificationPageSlot/) |
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 |
---|---|---|
@@ -1,3 +1,11 @@ | ||
import 'core-js/stable'; | ||
import 'regenerator-runtime/runtime'; | ||
import '@testing-library/jest-dom'; | ||
|
||
import MockedPluginSlot from './tests/MockedPluginSlot'; | ||
|
||
jest.mock('@openedx/frontend-plugin-framework', () => ({ | ||
...jest.requireActual('@openedx/frontend-plugin-framework'), | ||
Plugin: () => 'Plugin', | ||
PluginSlot: MockedPluginSlot, | ||
})); |
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,26 @@ | ||
import React from 'react'; | ||
import PropTypes from 'prop-types'; | ||
|
||
const MockedPluginSlot = ({ children, id }) => ( | ||
<div data-testid={id}> | ||
PluginSlot_{id} | ||
{ children && <div>{children}</div> } | ||
</div> | ||
); | ||
|
||
MockedPluginSlot.displayName = 'PluginSlot'; | ||
|
||
MockedPluginSlot.propTypes = { | ||
children: PropTypes.oneOfType([ | ||
PropTypes.arrayOf(PropTypes.node), | ||
PropTypes.node, | ||
]), | ||
id: PropTypes.string, | ||
}; | ||
|
||
MockedPluginSlot.defaultProps = { | ||
children: undefined, | ||
id: undefined, | ||
}; | ||
|
||
export default MockedPluginSlot; |
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,43 @@ | ||
import React from 'react'; | ||
import { render, screen } from '@testing-library/react'; | ||
import MockedPluginSlot from './MockedPluginSlot'; | ||
|
||
describe('MockedPluginSlot', () => { | ||
it('renders mock plugin with "PluginSlot" text', () => { | ||
render(<MockedPluginSlot id="test_plugin" />); | ||
|
||
const component = screen.getByText('PluginSlot_test_plugin'); | ||
expect(component).toBeInTheDocument(); | ||
}); | ||
|
||
it('renders as the slot children directly if there is content within', () => { | ||
render( | ||
<div role="article"> | ||
<MockedPluginSlot> | ||
<q role="note">How much wood could a woodchuck chuck if a woodchuck could chuck wood?</q> | ||
</MockedPluginSlot> | ||
</div>, | ||
); | ||
|
||
const component = screen.getByRole('article'); | ||
expect(component).toBeInTheDocument(); | ||
|
||
// Direct children | ||
const quote = component.querySelector(':scope > q'); | ||
expect(quote.getAttribute('role')).toBe('note'); | ||
}); | ||
|
||
it('renders mock plugin with a data-testid ', () => { | ||
render( | ||
<MockedPluginSlot id="guybrush"> | ||
<q role="note">I am selling these fine leather jackets.</q> | ||
</MockedPluginSlot>, | ||
); | ||
|
||
const component = screen.getByTestId('guybrush'); | ||
expect(component).toBeInTheDocument(); | ||
|
||
const quote = component.querySelector('[role=note]'); | ||
expect(quote).toBeInTheDocument(); | ||
}); | ||
}); |
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.
With a change like this, if we deploy this to production, the existing IDV flow will be gone.
I think we need a period of this code where the plugin slot and the existing IDV flow co-exist for a while. This way, while we build out the plugin slot, the existing IDV flow still serves our learners.
Then we need a waffle flag to do the switch from the entry link, which is probably 3 different places.
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.
Whatever component is wrapped by the
PluginSlot
(i.e.IdVerificationPage
) is the "default component" and will be rendered by default if there is no plugin configuration specified (e.g. in anenv.config.[js|jsx]
file inedx-internal
) or no overriding plugin in the said plugin configuration, so this deployment strategy isn't necessary. If we merge this as-is, there will be no changes to the IDV page in the learning MFE for 2U or any other deployments. There are more details the frontend-plugin-framework docs. Does that assuage your concerns?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.
Indeed. I tested it locally but with no configuration and the
<IdVerificationPage>
is rendered by default.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.
Nice. I didn't know plugin slot handles this the way it should be. Thank you for pointing it out.