Skip to content

feat: convert right sidebar notification CTA to frontend plugin - #49

Open
naincy128 wants to merge 4 commits into
release-teakfrom
LP-591
Open

feat: convert right sidebar notification CTA to frontend plugin#49
naincy128 wants to merge 4 commits into
release-teakfrom
LP-591

Conversation

@naincy128

@naincy128 naincy128 commented Jun 22, 2026

Copy link
Copy Markdown

Description

Scope: New sidebar only (isNewDiscussionSidebarViewEnabled = true). The legacy sidebar is untouched — its behavior is unchanged, and it never reads RIGHT_SIDEBAR_PANELS. No risk of the old sidebar showing or hiding panels differently.

This PR refactors the Learning MFE right sidebar from a single monolithic DiscussionsNotificationsSidebar into two independent, config-driven panels — Discussions and Upsell.

Changes

Sidebar Split

  • Deleted combined DiscussionsNotificationsSidebar.tsx and discussions-notifications/index.ts
  • Created sidebars/discussions/DiscussionsSidebar.tsx, DiscussionsTrigger.tsx (renders only DiscussionsWidget)
  • Created sidebars/upsell/UpsellSidebar.tsx, UpsellTrigger.tsx (renders only NotificationsWidget / upgrade CTA)
  • Renamed DiscussionsNotificationsTrigger.tsxUpsellTrigger.tsx, extracted discussions-specific logic (topic dispatch, edxProvider check) into DiscussionsTrigger.tsx

Config-Driven Panel Visibility

  • Added enabledPanels in SidebarContextProvider.tsx — reads RIGHT_SIDEBAR_PANELS from env.config.js
  • Defaults to ['DISCUSSIONS'] — Upsell is disabled by default and opt-in
  • Sidebar.tsx checks enabledPanels.includes(sidebarEntry.panelId) before rendering
  • SidebarTriggers.tsx filters SIDEBAR_ORDER by enabledPanels before rendering trigger buttons

Sidebar Registry

  • Updated sidebars/index.ts: replaced single DISCUSSIONS_NOTIFICATIONS entry with separate DISCUSSIONS and UPSELL entries, each with a panelId for config matching
  • Default sidebar changed from DISCUSSIONS_NOTIFICATIONSDISCUSSIONS

Plugin Slot Updates (in-place, no new slot files)

  • NotificationsDiscussionsSidebarSlot: primary ID updated to org.openedx.frontend.learning.right_sidebar.v1, old IDs preserved as idAliases
  • NotificationsDiscussionsSidebarTriggerSlot: primary ID updated to org.openedx.frontend.learning.right_sidebar_trigger.v1, old IDs preserved as idAliases
  • No new slot files created — Sequence.jsx import unchanged

Constants & i18n

  • WIDGETS.NOTIFICATIONSWIDGETS.UPSELL (deprecated alias NOTIFICATIONS: 'UPSELL' preserved)
  • Added rightSidebarTray message, deprecated discussionNotificationTray
  • Renamed notificationTitleupsellTitle (default: "Upgrade")

Config

RIGHT_SIDEBAR_PANELS value Result
Not set / undefined Discussions only (default)
['DISCUSSIONS'] Discussions only
['DISCUSSIONS', 'UPSELL'] Both panels
['UPSELL'] Upsell only

Impact

  • New sidebar users: Discussions and Upsell are independent panels, configurable via env.config.js
  • Old sidebar users: Zero impact — legacy codepath untouched
  • Existing plugin consumers: Zero breakage — old slot IDs preserved as idAliases

Ticket

LP-591

@naincy128
naincy128 marked this pull request as ready for review June 22, 2026 08:11
const sidebarKey = `sidebar.${courseId}`;

// Config-driven panel list — upsell disabled by default per LP-591
const enabledPanels = useMemo(() => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It looks like RIGHT_SIDEBAR_PANELS is only used by the new sidebar. The old sidebar still has Notifications and Discussions hard-coded.

Is LP-591 meant to cover only the new sidebar controlled by isNewDiscussionSidebarViewEnabled?

If yes, could we mention that clearly in the PR description? Otherwise, the old sidebar may still show the panel by default.

import Sidebar from '../../courseware/course/sidebar/Sidebar';
import NewSidebar from '../../courseware/course/new-sidebar/Sidebar';

export const RightSidebarSlot: React.FC<{ courseId: string }> = ({ courseId }) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why was RightSidebarSlot added if the app still renders NotificationsDiscussionsSidebarSlot?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I initially added a new RightSidebarSlot, but it wasn't being used because Sequence.jsx still referenced NotificationsDiscussionsSidebarSlot. I updated the existing slot with the new right_sidebar.v1 ID, kept the old IDs as aliases for backward compatibility, and removed the unused RightSidebarSlot file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants