INVESTIGATE: Does notification panel (not tray) in right sidebar exist in a plugin?

[ayub02]

This ticket is to answer a following questions about the notifications (upsell) panel in right sidebar:

1. Notification panel should exist within a plugin slot. Was that plugin slot created?
2. If so, was the upsell panel moved into the slot?

[awais-ansari]

Context
There are two key components: the NotificationsDiscussionsSidebarSlot and the NotificationsDiscussionsSidebarTriggerSlot. These allow for the customization of the right sidebar panel and the corresponding icon. By default, these slots display the entire right panel (combining Notifications and Discussions into one sidebar) along with the associated icons, without requiring any additional configuration. However, for the current use case, these slots are not applicable.

In this context, there are two types of sidebars: the Sidebar and the New Sidebar. The isNewDiscussionSidebarViewEnabled waffle flag is used to determine which layout should be displayed.

Sidebar

1. The Sidebar features notifications and discussions icons displayed side by side, with each panel opening independently. The NotificationTraySlot is utilized for customizing the content of the notification panel, which still has an associated trigger icon and window.
2. 

New Sidebar

1. The New Sidebar features only one icon, and the notifications and discussions panels are vertically stacked.
2. There is a NotificationWidgetSlot available for customizing the notification panel content. However, the external border and padding of the notification panel will remain intact, as this slot serves the same purpose as changing the notification panel's content.

Conclusion Currently, there is no slot available that allows for independent configuration of notifications within the right sidebar.

[sarina]

Hi @awais-ansari - sorry I can't quite follow what you're saying, but I'm curious: Could we alter the plugin slot (with jsx code) to hide the upsell panel only?

[awais-ansari]

Hello @sarina, @ayub02, and I discussed this today. The current implementation is complex to understand because there are 2 right sidebars in the code. I can not alter the current plugin slot to hide the upsell panel, because that will not fulfill our use case. We need to implement a new plugin slot to handle this use case, and that is possible.

[sarina]

What is "our use case" that you refer to?

[awais-ansari]

What is "our use case" that you refer to?

our use case = only hide the upsell panel Current implement only supports customizing the upsell panel content using the plugin slot.

[sarina]

Weird, I thought you could hide things with plugin slots. Is that true @brian-smith-tcril ?

[awais-ansari]

The NotificationTraySlot is located within the upsell panel. Hiding elements using this slot will only conceal the content in the upsell panel, while the panel itself will remain visible.

[sarina]

I wonder if this slot could be backported to Ulmo, with a default value of hiding the upsell panel (do we have time for this?) - the fact that it's called "Notifications" is very confusing with the rollout of the new notifications tray.

[brian-smith-tcril]

It's not quite clear to me what is being requested here, but I'll describe what I understand so far

---

Sidebar Slot

We currently have the NotificationsDiscussionsSidebarSlot

That slot has the following as default content

https://github.com/openedx/frontend-app-learning/blob/f43ac7bcc3c8b4081996adb043859026f56aa3ec/src/plugin-slots/NotificationsDiscussionsSidebarSlot/index.tsx#L26

Sidebar comes from here, NewSidebar comes from here

The NotificationWidgetSlot is rendered within the NotificationWidget component

https://github.com/openedx/frontend-app-learning/blob/f43ac7bcc3c8b4081996adb043859026f56aa3ec/src/courseware/course/new-sidebar/sidebars/discussions-notifications/notifications/NotificationsWidget.tsx#L70-L75

The NotificationsWidget component is imported into the "new" DiscussionsNotificationsSidebar as NotificationTray

https://github.com/openedx/frontend-app-learning/blob/f43ac7bcc3c8b4081996adb043859026f56aa3ec/src/courseware/course/new-sidebar/sidebars/discussions-notifications/DiscussionsNotificationsSidebar.tsx#L9

and rendered

https://github.com/openedx/frontend-app-learning/blob/f43ac7bcc3c8b4081996adb043859026f56aa3ec/src/courseware/course/new-sidebar/sidebars/discussions-notifications/DiscussionsNotificationsSidebar.tsx#L24

---

Sidebar Trigger Slot

We currently have the NotificationsDiscussionsSidebarTriggerSlot

That slot has the following as default content

https://github.com/openedx/frontend-app-learning/blob/f43ac7bcc3c8b4081996adb043859026f56aa3ec/src/plugin-slots/NotificationsDiscussionsSidebarTriggerSlot/index.tsx#L26

SidebarTriggers comes from here, NewSidebarTriggers comes from here

SidebarTriggers renders multiple buttons by mapping sidebarId

https://github.com/openedx/frontend-app-learning/blob/f43ac7bcc3c8b4081996adb043859026f56aa3ec/src/courseware/course/sidebar/SidebarTriggers.jsx#L17-L18

https://github.com/openedx/frontend-app-learning/blob/f43ac7bcc3c8b4081996adb043859026f56aa3ec/src/courseware/course/sidebar/sidebars/index.js#L4-L15

https://github.com/openedx/frontend-app-learning/blob/f43ac7bcc3c8b4081996adb043859026f56aa3ec/src/courseware/course/sidebar/SidebarTriggers.jsx#L26

NewSidebarTriggers renders buttons by mapping sidebarId

https://github.com/openedx/frontend-app-learning/blob/f43ac7bcc3c8b4081996adb043859026f56aa3ec/src/courseware/course/new-sidebar/SidebarTriggers.tsx#L11-L12

But only works with an array of 1

https://github.com/openedx/frontend-app-learning/blob/f43ac7bcc3c8b4081996adb043859026f56aa3ec/src/courseware/course/new-sidebar/sidebars/index.ts#L11-L13

https://github.com/openedx/frontend-app-learning/blob/f43ac7bcc3c8b4081996adb043859026f56aa3ec/src/courseware/course/new-sidebar/SidebarTriggers.tsx#L14

---

Could you clarify what part of the code the upsell panel is?

[sarina]

The "Upsell Panel" is the clock icon + confusingly named "Notifications" panel in this screenshot, it displays alongside the Discussions sidebar (talk bubble icon)

[brian-smith-tcril]

Is the goal to just hide the notifications trigger (🕓 icon button)? If so that can be accomplished now with the following env.config.jsx

import { DIRECT_PLUGIN, PLUGIN_OPERATIONS } from '@openedx/frontend-plugin-framework';

import { useContext } from 'react';
import SidebarContext from './src/courseware/course/sidebar/SidebarContext';
import { SIDEBARS } from './src/courseware/course/sidebar/sidebars';

const SidebarTriggers = () => {
  const { toggleSidebar } = useContext(SidebarContext);
  const sidebarId = 'DISCUSSIONS';
  const { Trigger } = SIDEBARS[sidebarId];

  return (
    <div className="d-flex ml-auto">
      <Trigger onClick={() => toggleSidebar(sidebarId)} key={sidebarId} />
    </div>
  );
};

const config = {
  pluginSlots: {
    'org.openedx.frontend.learning.notifications_discussions_sidebar_trigger.v1': {
      keepDefault: false,
      plugins: [
        {
          op: PLUGIN_OPERATIONS.Insert,
          widget: {
            id: 'custom_sidebar_trigger_component',
            type: DIRECT_PLUGIN,
            RenderWidget: () => (
              <SidebarTriggers />
            ),
          },
        },
      ]
    }
  },
}

export default config;

---

I do think it would be nice to have a simpler way to accomplish that, likely by updating the trigger components to take in SIDEBAR_ORDER and SIDEBARS as props instead of directly importing them.

https://github.com/openedx/frontend-app-learning/blob/f43ac7bcc3c8b4081996adb043859026f56aa3ec/src/courseware/course/sidebar/SidebarTriggers.jsx#L5

If we imported SIDEBAR_ORDER and SIDEBARS in NotificationsDiscussionsSidebarTriggerSlot, then we could update

https://github.com/openedx/frontend-app-learning/blob/f43ac7bcc3c8b4081996adb043859026f56aa3ec/src/plugin-slots/NotificationsDiscussionsSidebarTriggerSlot/index.tsx#L26

to have something like

<SidebarTriggers sidebars={SIDEBARS} sidebarOrder={SIDEBAR_ORDER} />

and then use the pattern that org.openedx.frontend.layout.header_desktop_logged_out_items.v1 does for modification, so we could do something like

import { PLUGIN_OPERATIONS } from '@openedx/frontend-plugin-framework';

const modifyTriggers = ( widget ) => {
  widget.content.sidebarOrder = ['DISCUSSIONS'];
  return widget;
};

const config = {
  pluginSlots: {
    'org.openedx.frontend.learning.notifications_discussions_sidebar_trigger.v1': {
      keepDefault: true,
      plugins: [
        {
          op: PLUGIN_OPERATIONS.Modify,
          widgetId: 'default_contents',
          fn: modifyTriggers,
        },
      ]
    },
  },
}

export default config;

[sarina]

Is the goal to just hide the notifications trigger (🕓 icon button)?

Yes, and the associated "Notifications" sidebar.

[sarina]

Would a possible halfway solution be to include content in the slot with Ulmo that contains this jsx to hide the trigger & sidebar, and work on a better fix for Verawood? Or is that too sloppy?

[brian-smith-tcril]

This is what it looks like with the top example env.config.jsx I posted (no code changes to the MFE itself)

---

Yes, and the associated "Notifications" sidebar.

Without the trigger there's no way to open it, so it's effectively "gone" - I can look into ways to fully remove it using the existing slots and env.config.jsx if that's required.

Would a possible halfway solution be to include content in the slot with Ulmo that contains this jsx to hide the trigger & sidebar

I'm not sure I'm following. Is the goal to have the default content of the slot not include the notifications trigger/sidebar?

[sarina]

I can look into ways to fully remove it using the existing slots and env.config.jsx if that's required.

Nah, that's not necessary.

Is the goal to have the default content of the slot not include the notifications trigger/sidebar?

Yes

[brian-smith-tcril]

Is the goal to have the default content of the slot not include the notifications trigger/sidebar?

Yes

Has the current default behavior been DEPR'd?

[sarina]

Has the current default behavior been DEPR'd?

😭 no. oops.

[sarina]

I guess, could we provide something in the operator notes around disabling this for Ulmo? Like a prewritten plugin. This is very confusing when it comes to conflating the Notifications Tray with the "Notifications" (upsell)