refactor: resolve feedback points

Author: awais-ansari

package-lock.json

@@ -37,15 +37,15 @@ const LockPaywall = ({
 
   // the following variables are set and used for responsive layout to work with
   // whether any sidebar panel is open and if there's an offer with longer text
-  const upgradePanelVisible = availableSidebarIds.includes(currentSidebar);
+  const isSidebarOpen = availableSidebarIds.includes(currentSidebar);
   const { width: windowWidth } = useWindowSize();
   const shouldDisplayBulletPointsBelowCertificate = windowWidth <= breakpoints.large.minWidth;
   const shouldDisplayGatedContentOneColumn = windowWidth <= breakpoints.extraLarge.minWidth
-    && upgradePanelVisible;
+    && isSidebarOpen;
   const shouldDisplayGatedContentTwoColumns = windowWidth < breakpoints.large.minWidth
-    && upgradePanelVisible;
+    && isSidebarOpen;
   const shouldDisplayGatedContentTwoColumnsHalf = windowWidth <= breakpoints.large.minWidth
-    && !upgradePanelVisible;
+    && !isSidebarOpen;
   const shouldWrapTextOnButton = windowWidth > breakpoints.extraSmall.minWidth;
 
   const accessExpirationDate = accessExpiration ? new Date(accessExpiration.expirationDate) : null;
@@ -98,7 +98,7 @@ const LockPaywall = ({
             </div>
           )}
 
-          <div className={classNames('d-inline-flex flex-row', { 'flex-wrap': upgradePanelVisible || shouldDisplayBulletPointsBelowCertificate })}>
+          <div className={classNames('d-inline-flex flex-row', { 'flex-wrap': isSidebarOpen || shouldDisplayBulletPointsBelowCertificate })}>
             <div style={{ float: 'left' }} className="mr-3 mb-2">
               <img
                 alt={intl.formatMessage(messages['learn.lockPaywall.example.alt'])}
@@ -128,7 +128,7 @@ const LockPaywall = ({
             <div
               className={
                 classNames('d-md-flex align-items-md-center text-right', {
-                  'col-md-5 mx-md-0': upgradePanelVisible, 'col-md-4 mx-md-3 justify-content-center': !upgradePanelVisible && !shouldDisplayGatedContentTwoColumnsHalf, 'col-md-11 justify-content-end': shouldDisplayGatedContentOneColumn && !shouldDisplayGatedContentTwoColumns, 'col-md-6 justify-content-center': shouldDisplayGatedContentTwoColumnsHalf,
+                  'col-md-5 mx-md-0': isSidebarOpen, 'col-md-4 mx-md-3 justify-content-center': !isSidebarOpen && !shouldDisplayGatedContentTwoColumnsHalf, 'col-md-11 justify-content-end': shouldDisplayGatedContentOneColumn && !shouldDisplayGatedContentTwoColumns, 'col-md-6 justify-content-center': shouldDisplayGatedContentTwoColumnsHalf,
                 })
               }
             >

src/courseware/course/sequence/lock-paywall/LockPaywall.jsx

@@ -41,7 +41,7 @@ const SequenceNavigation = ({
     sequence.gatedContent !== undefined && sequence.gatedContent.gated
   ) : undefined;
 
-  const shouldDisplayUpgradeTriggerInSequence = useWindowSize().width < breakpoints.small.minWidth;
+  const isSmallScreen = useWindowSize().width < breakpoints.small.minWidth;
 
   const renderUnitButtons = () => {
     if (isLocked) {
@@ -71,7 +71,7 @@ const SequenceNavigation = ({
       onClick={previousHandler}
       previousLink={previousLink}
       isFirstUnit={isFirstUnit}
-      buttonLabel={shouldDisplayUpgradeTriggerInSequence ? null : intl.formatMessage(messages.previousButton)}
+      buttonLabel={isSmallScreen ? null : intl.formatMessage(messages.previousButton)}
     />
   );
 
@@ -82,7 +82,7 @@ const SequenceNavigation = ({
 
     if (isLastUnit && exitText) {
       buttonText = exitText;
-    } else if (!shouldDisplayUpgradeTriggerInSequence) {
+    } else if (!isSmallScreen) {
       buttonText = intl.formatMessage(messages.nextButton);
     }
     return navigationDisabledNextSequence || (
@@ -101,7 +101,7 @@ const SequenceNavigation = ({
   };
 
   return sequenceStatus === LOADED ? (
-    <nav id="courseware-sequence-navigation" data-testid="courseware-sequence-navigation" className={classNames('sequence-navigation', className, { 'mr-2': shouldDisplayUpgradeTriggerInSequence })}>
+    <nav id="courseware-sequence-navigation" data-testid="courseware-sequence-navigation" className={classNames('sequence-navigation', className, { 'mr-2': isSmallScreen })}>
       {renderPreviousButton()}
       {renderUnitButtons()}
       {renderNextButton()}

src/courseware/course/sequence/sequence-navigation/SequenceNavigation.jsx

@@ -4,10 +4,10 @@
 
 The sidebar uses a pluggable widget framework that allows instances to customize which panels appear in the course sidebar.
 
-Widget implementations live in [`src/widgets/`](../../../widgets/):
+Widget implementations:
 
-- **[discussions](../../../widgets/discussions/README.md)** — built-in right-panel; always enabled
-- **[course-outline](../../../widgets/course-outline/README.md)** — built-in left-panel
+- **[discussions](../../../widgets/discussions/README.md)** — built-in right-panel; always enabled (in `src/widgets/`)
+- **[course-outline](sidebars/course-outline/README.md)** — built-in left-panel (in `sidebar/sidebars/`)
 
 ## Architecture
 

src/courseware/course/sidebar/README.md

@@ -109,7 +109,6 @@ const SidebarProvider = ({
     unitId,
     shouldDisplayFullScreen,
     hasUserToggledRef,
-    previousUnitIdRef,
     courseOutlineSetByUnitRef,
   });
 

src/courseware/course/sidebar/SidebarContextProvider.jsx

@@ -7,6 +7,7 @@ const SidebarTriggers = () => {
   const {
     toggleSidebar,
     currentSidebar,
+    availableSidebarIds,
     SIDEBAR_ORDER,
     SIDEBARS,
   } = useContext(SidebarContext);
@@ -20,7 +21,7 @@ const SidebarTriggers = () => {
 
   return (
     <div className="d-flex ml-auto">
-      {SIDEBAR_ORDER.map((sidebarId) => {
+      {SIDEBAR_ORDER.filter(id => availableSidebarIds.includes(id)).map((sidebarId) => {
         const { Trigger } = SIDEBARS[sidebarId];
         const isActive = sidebarId === currentSidebar;
         return (

src/courseware/course/sidebar/SidebarTriggers.jsx

@@ -39,6 +39,7 @@ function buildContext(overrides = {}) {
   return {
     toggleSidebar: mockToggleSidebar,
     currentSidebar: null,
+    availableSidebarIds: ['WIDGET_A', 'WIDGET_B'],
     SIDEBAR_ORDER: ['WIDGET_A', 'WIDGET_B'],
     SIDEBARS: {
       WIDGET_A: { Trigger: MockTriggerA },
@@ -160,6 +161,7 @@ describe('SidebarTriggers - external widget integration', () => {
         <SidebarContext.Provider value={{
           toggleSidebar: jest.fn(),
           currentSidebar: null,
+          availableSidebarIds: SIDEBAR_ORDER,
           SIDEBARS,
           SIDEBAR_ORDER,
         }}
@@ -193,6 +195,7 @@ describe('SidebarTriggers - external widget integration', () => {
         <SidebarContext.Provider value={{
           toggleSidebar: jest.fn(),
           currentSidebar: null,
+          availableSidebarIds: SIDEBAR_ORDER,
           SIDEBARS,
           SIDEBAR_ORDER,
         }}
@@ -230,6 +233,7 @@ describe('SidebarTriggers - external widget integration', () => {
         <SidebarContext.Provider value={{
           toggleSidebar,
           currentSidebar: null,
+          availableSidebarIds: SIDEBAR_ORDER,
           SIDEBARS,
           SIDEBAR_ORDER,
         }}

src/courseware/course/sidebar/SidebarTriggers.test.jsx

@@ -20,7 +20,6 @@ import {
  * @param {string} params.unitId - Current unit ID
  * @param {boolean} params.shouldDisplayFullScreen - Whether in mobile view
  * @param {Function} params.hasUserToggledRef - Ref tracking user manual toggles
- * @param {Function} params.previousUnitIdRef - Ref tracking previous unit ID
  * @param {Function} params.courseOutlineSetByUnitRef - Ref tracking COURSE_OUTLINE auto-set
  */
 export function useSidebarSync({
@@ -31,12 +30,11 @@ export function useSidebarSync({
   unitId,
   shouldDisplayFullScreen,
   hasUserToggledRef,
-  previousUnitIdRef,
   courseOutlineSetByUnitRef,
 }) {
   useEffect(() => {
-    // Skip if on mobile or if unit shift effect hasn't run yet
-    if (shouldDisplayFullScreen || previousUnitIdRef.current === null) {
+    // Skip if on mobile
+    if (shouldDisplayFullScreen) {
       return;
     }
 

src/courseware/course/sidebar/hooks/useSidebarSync.js

@@ -21,7 +21,6 @@ function buildParams(overrides = {}) {
     unitId,
     shouldDisplayFullScreen: false,
     hasUserToggledRef: { current: false },
-    previousUnitIdRef: { current: unitId }, // non-null = post-initial-load
     courseOutlineSetByUnitRef: { current: null },
     ...overrides,
   };
@@ -41,13 +40,6 @@ describe('useSidebarSync', () => {
       expect(params.setCurrentSidebar).not.toHaveBeenCalled();
     });
 
-    it('skips when previousUnitIdRef is null (initial page load)', () => {
-      const params = buildParams({ previousUnitIdRef: { current: null } });
-      renderHook(() => useSidebarSync(params));
-
-      expect(params.setCurrentSidebar).not.toHaveBeenCalled();
-    });
-
     it('skips when user has manually toggled the sidebar', () => {
       const params = buildParams({ hasUserToggledRef: { current: true } });
       renderHook(() => useSidebarSync(params));

src/courseware/course/sidebar/hooks/useSidebarSync.test.js

@@ -120,8 +120,7 @@ export function useUnitShiftBehavior({
           setCurrentSidebar(firstAvailable);
           setSidebarId(courseId, firstAvailable);
         } else if (currentWidget) {
-          // Current RIGHT sidebar panel still available and no higher priority - keep it
-          setCurrentSidebar(currentSidebar);
+          // Current sidebar still valid at same priority — no state change needed
         } else {
           // Current panel not available - switch to first available RIGHT sidebar panel
           setCurrentSidebar(firstAvailable);