From bce25c462ad96dc326425869ba107af460c4c877 Mon Sep 17 00:00:00 2001 From: leangseu-edx <83240113+leangseu-edx@users.noreply.github.com> Date: Wed, 22 Nov 2023 11:46:34 -0500 Subject: [PATCH] chore: update sidebar logic (#1236) * fix: make side bar notification behave correctly --------- Co-authored-by: Awais Ansari --- src/courseware/course/Course.jsx | 12 ----- src/courseware/course/Course.test.jsx | 50 ++----------------- .../course/sidebar/SidebarContextProvider.jsx | 7 ++- .../course/sidebar/sidebars/index.js | 12 ++--- .../notifications/NotificationTray.jsx | 1 + .../notifications/NotificationTrigger.jsx | 12 +---- src/data/sessionStorage.js | 7 +-- 7 files changed, 19 insertions(+), 82 deletions(-) diff --git a/src/courseware/course/Course.jsx b/src/courseware/course/Course.jsx index 76873b06e3..459d76bee0 100644 --- a/src/courseware/course/Course.jsx +++ b/src/courseware/course/Course.jsx @@ -17,7 +17,6 @@ import SidebarProvider from './sidebar/SidebarContextProvider'; import SidebarTriggers from './sidebar/SidebarTriggers'; import { useModel } from '../../generic/model-store'; -import { getSessionStorage, setSessionStorage } from '../../data/sessionStorage'; const Course = ({ courseId, @@ -32,7 +31,6 @@ const Course = ({ const { celebrations, isStaff, - verifiedMode, } = useModel('courseHomeMeta', courseId); const sequence = useModel('sequences', sequenceId); const section = useModel('sections', sequence ? sequence.sectionId : null); @@ -55,16 +53,6 @@ const Course = ({ const shouldDisplayTriggers = windowWidth >= breakpoints.small.minWidth; const daysPerWeek = course?.courseGoals?.selectedGoal?.daysPerWeek; - // 1. open the notification tray if the user has not purchased the course and is on a desktop - // 2. default to close on the first time access - // 3. use the last state if the user has access the course before - const shouldDisplayNotificationTrayOpenOnLoad = windowWidth > breakpoints.medium.minWidth && !!verifiedMode; - if (shouldDisplayNotificationTrayOpenOnLoad) { - setSessionStorage(`notificationTrayStatus.${courseId}`, 'open'); - } else if (!getSessionStorage(`notificationTrayStatus.${courseId}`)) { - setSessionStorage(`notificationTrayStatus.${courseId}`, 'closed'); - } - useEffect(() => { const celebrateFirstSection = celebrations && celebrations.firstSection; setFirstSectionCelebrationOpen(shouldCelebrateOnSectionLoad( diff --git a/src/courseware/course/Course.test.jsx b/src/courseware/course/Course.test.jsx index d2c60d8fbe..64de33b27a 100644 --- a/src/courseware/course/Course.test.jsx +++ b/src/courseware/course/Course.test.jsx @@ -43,8 +43,6 @@ describe('Course', () => { sequenceId, unitId: Object.values(models.units)[0].id, }); - getItemSpy = jest.spyOn(Object.getPrototypeOf(window.sessionStorage), 'getItem'); - setItemSpy = jest.spyOn(Object.getPrototypeOf(window.sessionStorage), 'setItem'); global.innerWidth = breakpoints.extraLarge.minWidth; }); @@ -54,7 +52,8 @@ describe('Course', () => { }); const setupDiscussionSidebar = async () => { - const testStore = await initializeTestStore({ provider: 'openedx' }); + const courseHomeMetadata = Factory.build('courseHomeMetadata', { verified_mode: null }); + const testStore = await initializeTestStore({ provider: 'openedx', courseHomeMetadata }); const state = testStore.getState(); const { courseware: { courseId } } = state; const axiosMock = new MockAdapter(getAuthenticatedHttpClient()); @@ -136,9 +135,9 @@ describe('Course', () => { const notificationTrigger = screen.getByRole('button', { name: /Show notification tray/i }); expect(notificationTrigger).toBeInTheDocument(); - expect(notificationTrigger.parentNode).toHaveClass('mt-3'); - fireEvent.click(notificationTrigger); expect(notificationTrigger.parentNode).not.toHaveClass('mt-3', { exact: true }); + fireEvent.click(notificationTrigger); + expect(notificationTrigger.parentNode).toHaveClass('mt-3'); }); it('handles click to open/close discussions sidebar', async () => { @@ -184,54 +183,13 @@ describe('Course', () => { }); it('handles click to open/close notification tray', async () => { - sessionStorage.clear(); render(, { wrapWithRouter: true }); - expect(sessionStorage.getItem(`notificationTrayStatus.${mockData.courseId}`)).toBe('"open"'); const notificationShowButton = await screen.findByRole('button', { name: /Show notification tray/i }); expect(screen.queryByRole('region', { name: /notification tray/i })).toHaveClass('d-none'); fireEvent.click(notificationShowButton); - expect(sessionStorage.getItem(`notificationTrayStatus.${mockData.courseId}`)).toBe('"closed"'); expect(screen.queryByRole('region', { name: /notification tray/i })).not.toHaveClass('d-none'); }); - it('handles reload persisting notification tray status', async () => { - sessionStorage.clear(); - render(, { wrapWithRouter: true }); - const notificationShowButton = await screen.findByRole('button', { name: /Show notification tray/i }); - fireEvent.click(notificationShowButton); - expect(sessionStorage.getItem(`notificationTrayStatus.${mockData.courseId}`)).toBe('"closed"'); - - // Mock reload window, this doesn't happen in the Course component, - // calling the reload to check if the tray remains closed - const { location } = window; - delete window.location; - window.location = { reload: jest.fn() }; - window.location.reload(); - expect(window.location.reload).toHaveBeenCalled(); - window.location = location; - expect(sessionStorage.getItem(`notificationTrayStatus.${mockData.courseId}`)).toBe('"closed"'); - expect(screen.queryByTestId('NotificationTray')).not.toBeInTheDocument(); - }); - - it('handles sessionStorage from a different course for the notification tray', async () => { - sessionStorage.clear(); - const courseMetadataSecondCourse = Factory.build('courseMetadata', { id: 'second_course' }); - - // set sessionStorage for a different course before rendering Course - sessionStorage.setItem(`notificationTrayStatus.${courseMetadataSecondCourse.id}`, '"open"'); - - render(, { wrapWithRouter: true }); - expect(sessionStorage.getItem(`notificationTrayStatus.${mockData.courseId}`)).toBe('"open"'); - const notificationShowButton = await screen.findByRole('button', { name: /Show notification tray/i }); - fireEvent.click(notificationShowButton); - - // Verify sessionStorage was updated for the original course - expect(sessionStorage.getItem(`notificationTrayStatus.${mockData.courseId}`)).toBe('"closed"'); - - // Verify the second course sessionStorage was not changed - expect(sessionStorage.getItem(`notificationTrayStatus.${courseMetadataSecondCourse.id}`)).toBe('"open"'); - }); - it('renders course breadcrumbs as expected', async () => { const courseMetadata = Factory.build('courseMetadata'); const unitBlocks = Array.from({ length: 3 }).map(() => Factory.build( diff --git a/src/courseware/course/sidebar/SidebarContextProvider.jsx b/src/courseware/course/sidebar/SidebarContextProvider.jsx index b61c577b19..1565227619 100644 --- a/src/courseware/course/sidebar/SidebarContextProvider.jsx +++ b/src/courseware/course/sidebar/SidebarContextProvider.jsx @@ -4,7 +4,9 @@ import React, { useEffect, useState, useMemo, useCallback, } from 'react'; +import { useModel } from '../../../generic/model-store'; import { getLocalStorage, setLocalStorage } from '../../../data/localStorage'; + import SidebarContext from './SidebarContext'; import { SIDEBARS } from './sidebars'; @@ -13,6 +15,7 @@ const SidebarProvider = ({ unitId, children, }) => { + const { verifiedMode } = useModel('courseHomeMeta', courseId); const shouldDisplayFullScreen = useWindowSize().width < breakpoints.large.minWidth; const shouldDisplaySidebarOpen = useWindowSize().width > breakpoints.medium.minWidth; const query = new URLSearchParams(window.location.search); @@ -22,8 +25,8 @@ const SidebarProvider = ({ const [upgradeNotificationCurrentState, setUpgradeNotificationCurrentState] = useState(getLocalStorage(`upgradeNotificationCurrentState.${courseId}`)); useEffect(() => { - setCurrentSidebar(SIDEBARS.DISCUSSIONS.ID); - // eslint-disable-next-line react-hooks/exhaustive-deps + // if the user hasn't purchased the course, show the notifications sidebar + setCurrentSidebar(verifiedMode ? SIDEBARS.NOTIFICATIONS.ID : SIDEBARS.DISCUSSIONS.ID); }, [unitId]); const onNotificationSeen = useCallback(() => { diff --git a/src/courseware/course/sidebar/sidebars/index.js b/src/courseware/course/sidebar/sidebars/index.js index 9128fd0e48..fe89ecd66c 100644 --- a/src/courseware/course/sidebar/sidebars/index.js +++ b/src/courseware/course/sidebar/sidebars/index.js @@ -1,5 +1,5 @@ import * as notifications from './notifications'; -import * as discusssions from './discussions'; +import * as discussions from './discussions'; export const SIDEBARS = { [notifications.ID]: { @@ -7,14 +7,14 @@ export const SIDEBARS = { Sidebar: notifications.Sidebar, Trigger: notifications.Trigger, }, - [discusssions.ID]: { - ID: discusssions.ID, - Sidebar: discusssions.Sidebar, - Trigger: discusssions.Trigger, + [discussions.ID]: { + ID: discussions.ID, + Sidebar: discussions.Sidebar, + Trigger: discussions.Trigger, }, }; export const SIDEBAR_ORDER = [ - discusssions.ID, + discussions.ID, notifications.ID, ]; diff --git a/src/courseware/course/sidebar/sidebars/notifications/NotificationTray.jsx b/src/courseware/course/sidebar/sidebars/notifications/NotificationTray.jsx index 3cef46a0ca..409f3a92a6 100644 --- a/src/courseware/course/sidebar/sidebars/notifications/NotificationTray.jsx +++ b/src/courseware/course/sidebar/sidebars/notifications/NotificationTray.jsx @@ -42,6 +42,7 @@ const NotificationTray = ({ intl }) => { title={intl.formatMessage(messages.notificationTitle)} ariaLabel={intl.formatMessage(messages.notificationTray)} sidebarId={ID} + width="50rem" className={classNames({ 'h-100': !verifiedMode && !shouldDisplayFullScreen })} >
{verifiedMode diff --git a/src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.jsx b/src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.jsx index 6819e06acc..55ad5522c3 100644 --- a/src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.jsx +++ b/src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.jsx @@ -2,7 +2,6 @@ import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import PropTypes from 'prop-types'; import React, { useContext, useEffect } from 'react'; import { getLocalStorage, setLocalStorage } from '../../../../../data/localStorage'; -import { getSessionStorage, setSessionStorage } from '../../../../../data/sessionStorage'; import messages from '../../../messages'; import SidebarTriggerBase from '../../common/TriggerBase'; import SidebarContext from '../../SidebarContext'; @@ -47,17 +46,8 @@ const NotificationTrigger = ({ UpdateUpgradeNotificationLastSeen(); }); - const handleClick = () => { - if (getSessionStorage(`notificationTrayStatus.${courseId}`) === 'open') { - setSessionStorage(`notificationTrayStatus.${courseId}`, 'closed'); - } else { - setSessionStorage(`notificationTrayStatus.${courseId}`, 'open'); - } - onClick(); - }; - return ( - + ); diff --git a/src/data/sessionStorage.js b/src/data/sessionStorage.js index fe86b577d0..40ffd69aa1 100644 --- a/src/data/sessionStorage.js +++ b/src/data/sessionStorage.js @@ -7,10 +7,7 @@ function getSessionStorage(key) { try { if (global.sessionStorage) { - const rawItem = global.sessionStorage.getItem(key); - if (rawItem) { - return JSON.parse(rawItem); - } + return global.sessionStorage.getItem(key); } } catch (e) { // If this fails for some reason, just return null. @@ -21,7 +18,7 @@ function getSessionStorage(key) { function setSessionStorage(key, value) { try { if (global.sessionStorage) { - global.sessionStorage.setItem(key, JSON.stringify(value)); + global.sessionStorage.setItem(key, value); } } catch (e) { // If this fails, just bail.