Skip to content

Commit

Permalink
fix: modifies sidebar state such that it remains open (#1131)
Browse files Browse the repository at this point in the history 
* fix: modifies sidebar state such that it remains open

* refactor: removed localstorage for discussions sideba
  • Loading branch information
@ayesha-waris
ayesha-waris authored Jun 27, 2023
1 parent 340580c commit 8ac9745
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 59 deletions.
88 changes: 48 additions & 40 deletions src/courseware/course/Course.test.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ describe('Course', () => {
setItemSpy.mockRestore();
});

const setupDiscussionSidebar = async (storageValue = false) => {
localStorage.clear();
const setupDiscussionSidebar = async () => {
const testStore = await initializeTestStore({ provider: 'openedx' });
const state = testStore.getState();
const { courseware: { courseId } } = state;
Expand All @@ -65,9 +64,7 @@ describe('Course', () => {
mockData.unitId = firstUnitId;
const [firstSequenceId] = Object.keys(state.models.sequences);
mockData.sequenceId = firstSequenceId;
if (storageValue !== null) {
localStorage.setItem('showDiscussionSidebar', storageValue);
}

await render(<Course {...mockData} />, { store: testStore });
};

Expand Down Expand Up @@ -131,26 +128,66 @@ describe('Course', () => {
});

it('displays notification trigger and toggles active class on click', async () => {
localStorage.setItem('showDiscussionSidebar', false);
render(<Course {...mockData} />);

const notificationTrigger = screen.getByRole('button', { name: /Show notification tray/i });
expect(notificationTrigger).toBeInTheDocument();
expect(notificationTrigger.parentNode).toHaveClass('border-primary-700');
expect(notificationTrigger.parentNode).toHaveClass('mt-3');
fireEvent.click(notificationTrigger);
expect(notificationTrigger.parentNode).not.toHaveClass('border-primary-700');
expect(notificationTrigger.parentNode).not.toHaveClass('mt-3', { exact: true });
});

it('handles click to open/close discussions sidebar', async () => {
await setupDiscussionSidebar();
const discussionsTrigger = await screen.getByRole('button', { name: /Show discussions tray/i });
const discussionsSideBar = await waitFor(() => screen.findByTestId('sidebar-DISCUSSIONS'));

expect(discussionsSideBar).not.toHaveClass('d-none');

await act(async () => {
fireEvent.click(discussionsTrigger);
});
await expect(discussionsSideBar).toHaveClass('d-none');

await act(async () => {
fireEvent.click(discussionsTrigger);
});
await expect(discussionsSideBar).not.toHaveClass('d-none');
});

it('displays discussions sidebar when unit changes', async () => {
const testStore = await initializeTestStore();
const { courseware, models } = testStore.getState();
const { courseId, sequenceId } = courseware;
const testData = {
...mockData,
courseId,
sequenceId,
unitId: Object.values(models.units)[0].id,
};

await setupDiscussionSidebar();

const { rerender } = render(<Course {...testData} />, { store: testStore });
loadUnit();

await waitFor(() => {
expect(screen.getByTestId('sidebar-DISCUSSIONS')).toBeInTheDocument();
expect(screen.getByTestId('sidebar-DISCUSSIONS')).not.toHaveClass('d-none');
});

rerender(null);
});

it('handles click to open/close notification tray', async () => {
sessionStorage.clear();
localStorage.setItem('showDiscussionSidebar', false);
render(<Course {...mockData} />);
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 })).not.toHaveClass('d-none');
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 })).toHaveClass('d-none');
expect(screen.queryByRole('region', { name: /notification tray/i })).not.toHaveClass('d-none');
});

it('handles reload persisting notification tray status', async () => {
Expand All @@ -174,7 +211,6 @@ describe('Course', () => {

it('handles sessionStorage from a different course for the notification tray', async () => {
sessionStorage.clear();
localStorage.setItem('showDiscussionSidebar', false);
const courseMetadataSecondCourse = Factory.build('courseMetadata', { id: 'second_course' });

// set sessionStorage for a different course before rendering Course
Expand Down Expand Up @@ -217,34 +253,6 @@ describe('Course', () => {
expect(screen.getByText(Object.values(models.sequences)[0].title)).toBeInTheDocument();
});

[
{ value: true, visible: true },
{ value: false, visible: false },
{ value: null, visible: true },
].forEach(async ({ value, visible }) => (
it(`discussion sidebar is ${visible ? 'shown' : 'hidden'} when localstorage value is ${value}`, async () => {
await setupDiscussionSidebar(value);
const element = await waitFor(() => screen.findByTestId('sidebar-DISCUSSIONS'));
if (visible) {
expect(element).not.toHaveClass('d-none');
} else {
expect(element).toHaveClass('d-none');
}
})));

[
{ value: true, result: 'false' },
{ value: false, result: 'true' },
].forEach(async ({ value, result }) => (
it(`Discussion sidebar storage value is ${!value} when sidebar is ${value ? 'closed' : 'open'}`, async () => {
await setupDiscussionSidebar(value);
await act(async () => {
const button = await screen.queryByRole('button', { name: /Show discussions tray/i });
button.click();
});
expect(localStorage.getItem('showDiscussionSidebar')).toBe(result);
})));

it('passes handlers to the sequence', async () => {
const nextSequenceHandler = jest.fn();
const previousSequenceHandler = jest.fn();
Expand Down
24 changes: 5 additions & 19 deletions src/courseware/course/sidebar/SidebarContextProvider.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import React, {
} from 'react';

import { getLocalStorage, setLocalStorage } from '../../../data/localStorage';
import { getSessionStorage } from '../../../data/sessionStorage';
import { useModel } from '../../../generic/model-store';
import SidebarContext from './SidebarContext';
import { SIDEBARS } from './sidebars';
Expand All @@ -18,29 +17,21 @@ const SidebarProvider = ({
const { verifiedMode } = useModel('courseHomeMeta', courseId);
const shouldDisplayFullScreen = useWindowSize().width < breakpoints.large.minWidth;
const shouldDisplaySidebarOpen = useWindowSize().width > breakpoints.medium.minWidth;
const showNotificationsOnLoad = getSessionStorage(`notificationTrayStatus.${courseId}`) !== 'closed';
const query = new URLSearchParams(window.location.search);
if (query.get('sidebar') === 'true') {
localStorage.setItem('showDiscussionSidebar', true);
}
const showDiscussionSidebar = localStorage.getItem('showDiscussionSidebar') !== 'false';
const showNotificationSidebar = (verifiedMode && shouldDisplaySidebarOpen && showNotificationsOnLoad)
? SIDEBARS.NOTIFICATIONS.ID
: null;
const initialSidebar = showDiscussionSidebar
const initialSidebar = ((verifiedMode && shouldDisplaySidebarOpen) || query.get('sidebar') === 'true')
? SIDEBARS.DISCUSSIONS.ID
: showNotificationSidebar;
: null;
const [currentSidebar, setCurrentSidebar] = useState(initialSidebar);
const [notificationStatus, setNotificationStatus] = useState(getLocalStorage(`notificationStatus.${courseId}`));
const [upgradeNotificationCurrentState, setUpgradeNotificationCurrentState] = useState(getLocalStorage(`upgradeNotificationCurrentState.${courseId}`));

useEffect(() => {
// As a one-off set initial sidebar if the verified mode data has just loaded
if (verifiedMode && currentSidebar === null && initialSidebar) {
setCurrentSidebar(initialSidebar);
if (verifiedMode) {
setCurrentSidebar(SIDEBARS.DISCUSSIONS.ID);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [initialSidebar, verifiedMode]);
}, [verifiedMode, unitId]);

const onNotificationSeen = useCallback(() => {
setNotificationStatus('inactive');
Expand All @@ -49,11 +40,6 @@ const SidebarProvider = ({

const toggleSidebar = useCallback((sidebarId) => {
// Switch to new sidebar or hide the current sidebar
if (currentSidebar === SIDEBARS.DISCUSSIONS.ID) {
localStorage.setItem('showDiscussionSidebar', false);
} else if (sidebarId === SIDEBARS.DISCUSSIONS.ID) {
localStorage.setItem('showDiscussionSidebar', true);
}
setCurrentSidebar(sidebarId === currentSidebar ? null : sidebarId);
}, [currentSidebar]);

Expand Down

0 comments on commit 8ac9745

Please sign in to comment.