-
Notifications
You must be signed in to change notification settings - Fork 107
feat: add CORE_PLUGINS hook and bundle the notifications tray #292
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| - [Feature] Introduce the ``CORE_PLUGINS`` filter, a new extension point for bundled frontend plugin packages that ship with tutor-mfe. Operators can disable any core plugin by popping its name from the filter, symmetrically to how ``MFE_APPS`` works. (by @arbrandes) | ||
| - [Feature] Bundle the notifications tray (``@edx/frontend-plugin-notifications``) as the first core plugin, enabled by default. This replaces the need for a standalone ``tutor-contrib-platform-notifications`` plugin. (by @arbrandes) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| {% if is_core_plugin_enabled("notifications") %} | ||
| NOTIFICATIONS_DEFAULT_FROM_EMAIL: '{{ NOTIFICATIONS_DEFAULT_FROM_EMAIL }}' | ||
| {% endif %} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| {% if is_core_plugin_enabled("notifications") %} | ||
| NOTIFICATIONS_DEFAULT_FROM_EMAIL = ENV_TOKENS.get("NOTIFICATIONS_DEFAULT_FROM_EMAIL", ENV_TOKENS["CONTACT_EMAIL"]) | ||
| {% endif %} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,9 +25,32 @@ async function setConfig () { | |
| * needs to be inside the `try{}` block. | ||
| */ | ||
| const { DIRECT_PLUGIN, PLUGIN_OPERATIONS } = await import('@openedx/frontend-plugin-framework'); | ||
| {%- if is_core_plugin_enabled("notifications") %} | ||
| const { NotificationsTray } = await import('@edx/frontend-plugin-notifications'); | ||
| {%- endif %} | ||
|
|
||
| {{- patch("mfe-env-config-runtime-definitions") }} | ||
|
|
||
| {%- if is_core_plugin_enabled("notifications") %} | ||
| {%- for slot_name in [ | ||
| "org.openedx.frontend.layout.header_desktop_secondary_menu.v1", | ||
| "org.openedx.frontend.layout.header_learning_help.v1", | ||
| "org.openedx.frontend.layout.studio_header_search_button_slot.v1", | ||
| ] %} | ||
| addPlugins(config, '{{ slot_name }}', [ | ||
| { | ||
| op: PLUGIN_OPERATIONS.Insert, | ||
| widget: { | ||
| id: 'notification-drawer-widget', | ||
| priority: 10, | ||
| type: DIRECT_PLUGIN, | ||
| RenderWidget: NotificationsTray, | ||
| }, | ||
| }, | ||
| ]); | ||
| {%- endfor %} | ||
| {%- endif %} | ||
|
Comment on lines
+34
to
+52
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we also move this to
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually tried, but it doesn't work well: we're consuming the hook (PLUGIN_SLOTS) we're defining ourselves, so there are timing/race conditions that result in the core plugin not being picked up here. I know it looks funny, but defining this directly in the template is the right place.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
makes sense, |
||
|
|
||
| {%- for slot_name, plugin_config in iter_plugin_slots("all") %} | ||
| addPlugins(config, '{{ slot_name }}', [{{ plugin_config }}]); | ||
| {%- endfor %} | ||
|
|
||
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.
can we add this to
mfe-env-config-runtime-definitionspatch again like it was in tutor-contrib-platform-notifications instead of filling inenv.config.jsx?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.
We could, but not cleanly: the patches tutor-mfe defines (such as
mfe-env-config-runtime-definitions) are meant for other plugins to consume. That's what thetemplatesare for: we put the stuff we own in there, and patches are for external use.