Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/courseware/course/Course.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ const Course = ({
const SidebarProviderComponent = isNewDiscussionSidebarViewEnabled ? NewSidebarProvider : SidebarProvider;

return (
<SidebarProviderComponent courseId={courseId} unitId={unitId}>
<SidebarProviderComponent courseId={courseId} unitId={unitId} sectionId={section ? section.id : null}>
<Helmet>
<title>{`${pageTitleBreadCrumbs.join(' | ')} | ${getConfig().SITE_NAME}`}</title>
</Helmet>
Expand Down
7 changes: 4 additions & 3 deletions src/courseware/course/Course.test.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import React from 'react';

import { Factory } from 'rosie';

import { breakpoints } from '@openedx/paragon';
Expand All @@ -11,6 +9,7 @@ import * as celebrationUtils from './celebration/utils';
import { handleNextSectionCelebration } from './celebration';
import Course from './Course';
import setupDiscussionSidebar from './test-utils';
import messages from './messages';

jest.mock('@edx/frontend-platform/analytics');
jest.mock('@edx/frontend-lib-special-exams/dist/data/thunks.js', () => ({
Expand Down Expand Up @@ -194,7 +193,9 @@ describe('Course', () => {
it('handles click to open/close notification tray', async () => {
await setupDiscussionSidebar();
waitFor(() => {
const notificationShowButton = screen.findByRole('button', { name: /Show notification tray/i });
const notificationShowButton = screen.findByRole('button', {
name: messages.openNotificationTrigger.defaultMessage,
});
expect(screen.queryByRole('region', { name: /notification tray/i })).not.toBeInTheDocument();
fireEvent.click(notificationShowButton);
expect(screen.queryByRole('region', { name: /notification tray/i })).toBeInTheDocument();
Expand Down
2 changes: 1 addition & 1 deletion src/courseware/course/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const messages = defineMessages({
},
openNotificationTrigger: {
id: 'notification.open.button',
defaultMessage: 'Show notification tray',
defaultMessage: 'Notifications tray',
description: 'Button to open the notification tray and show notifications',
},
closeNotificationTrigger: {
Expand Down
5 changes: 4 additions & 1 deletion src/courseware/course/sidebar/SidebarContextProvider.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { SIDEBARS } from './sidebars';
const SidebarProvider = ({
courseId,
unitId,
sectionId,
children,
}) => {
const { verifiedMode } = useModel('courseHomeMeta', courseId);
Expand Down Expand Up @@ -72,8 +73,9 @@ const SidebarProvider = ({
shouldDisplayFullScreen,
courseId,
unitId,
sectionId,
}), [courseId, currentSidebar, notificationStatus, onNotificationSeen, shouldDisplayFullScreen,
shouldDisplaySidebarOpen, toggleSidebar, unitId, upgradeNotificationCurrentState]);
shouldDisplaySidebarOpen, toggleSidebar, unitId, upgradeNotificationCurrentState, sectionId]);

return (
<SidebarContext.Provider value={contextValue}>
Expand All @@ -85,6 +87,7 @@ const SidebarProvider = ({
SidebarProvider.propTypes = {
courseId: PropTypes.string.isRequired,
unitId: PropTypes.string.isRequired,
sectionId: PropTypes.string.isRequired,
children: PropTypes.node,
};

Expand Down
154 changes: 154 additions & 0 deletions src/courseware/course/sidebar/common/Sidebar.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
import { Factory } from 'rosie';
import userEvent from '@testing-library/user-event';
import { createRef } from 'react';

import {
initializeTestStore,
render,
screen,
fireEvent,
waitFor,
} from '@src/setupTest';
import messages from '@src/courseware/course/messages';
import SidebarContext from '../SidebarContext';
import SidebarBase from './SidebarBase';
import { useSidebarFocusAndKeyboard } from './hooks/useSidebarFocusAndKeyboard';

jest.mock('./hooks/useSidebarFocusAndKeyboard');

const SIDEBAR_ID = 'test-sidebar';

const mockUseSidebarFocusAndKeyboard = useSidebarFocusAndKeyboard;

describe('SidebarBase', () => {
let mockContextValue;
const courseMetadata = Factory.build('courseMetadata');
const user = userEvent.setup();

let mockCloseBtnRef;
let mockBackBtnRef;
let mockHandleClose;
let mockHandleKeyDown;
let mockHandleBackBtnKeyDown;

const defaultComponentProps = {
title: 'Test Sidebar Title',
ariaLabel: 'Test Sidebar Aria Label',
sidebarId: SIDEBAR_ID,
className: 'test-class',
children: <div>Sidebar Content</div>,
};

const renderSidebar = (contextProps = {}, componentProps = {}) => {
const fullContextValue = { ...mockContextValue, ...contextProps };
const defaultProps = { ...defaultComponentProps, ...componentProps };
return render(
<SidebarContext.Provider value={fullContextValue}>
<SidebarBase {...defaultProps} />
</SidebarContext.Provider>,
);
};

beforeEach(async () => {
await initializeTestStore({
courseMetadata,
excludeFetchCourse: true,
excludeFetchSequence: true,
});

mockContextValue = {
courseId: courseMetadata.id,
toggleSidebar: jest.fn(),
shouldDisplayFullScreen: false,
currentSidebar: null,
};

mockCloseBtnRef = createRef();
mockBackBtnRef = createRef();
mockHandleClose = jest.fn();
mockHandleKeyDown = jest.fn();
mockHandleBackBtnKeyDown = jest.fn();

mockUseSidebarFocusAndKeyboard.mockReturnValue({
closeBtnRef: mockCloseBtnRef,
backBtnRef: mockBackBtnRef,
handleClose: mockHandleClose,
handleKeyDown: mockHandleKeyDown,
handleBackBtnKeyDown: mockHandleBackBtnKeyDown,
});
});

afterEach(() => {
jest.clearAllMocks();
});

it('should render children, title, and close button when visible', () => {
renderSidebar({ currentSidebar: SIDEBAR_ID });
expect(screen.getByText('Sidebar Content')).toBeInTheDocument();
expect(screen.getByText(defaultComponentProps.title)).toBeInTheDocument();
expect(screen.getByRole('button', { name: messages.closeNotificationTrigger.defaultMessage })).toBeInTheDocument();
expect(screen.getByTestId(`sidebar-${SIDEBAR_ID}`)).toBeInTheDocument();
expect(screen.getByTestId(`sidebar-${SIDEBAR_ID}`)).not.toHaveClass('d-none');
});

it('should be hidden via CSS class when not the current sidebar', () => {
renderSidebar({ currentSidebar: 'another-sidebar-id' });
const sidebarElement = screen.queryByTestId(`sidebar-${SIDEBAR_ID}`);
expect(sidebarElement).toBeInTheDocument();
expect(sidebarElement).toHaveClass('d-none');
});

it('should hide title bar when showTitleBar prop is false', () => {
renderSidebar({ currentSidebar: SIDEBAR_ID }, { showTitleBar: false });
expect(screen.queryByText(defaultComponentProps.title)).not.toBeInTheDocument();
expect(screen.queryByRole('button', { name: messages.closeNotificationTrigger.defaultMessage })).not.toBeInTheDocument();
expect(screen.getByText('Sidebar Content')).toBeInTheDocument();
});

it('should render back button instead of close button in fullscreen', () => {
renderSidebar({ currentSidebar: SIDEBAR_ID, shouldDisplayFullScreen: true });
expect(screen.getByRole('button', { name: messages.responsiveCloseNotificationTray.defaultMessage })).toBeInTheDocument();
expect(screen.queryByRole('button', { name: messages.closeNotificationTrigger.defaultMessage })).not.toBeInTheDocument();
});

it('should call handleClose from hook on close button click', async () => {
renderSidebar({ currentSidebar: SIDEBAR_ID });
const closeButton = screen.getByRole('button', { name: messages.closeNotificationTrigger.defaultMessage });
await user.click(closeButton);
expect(mockHandleClose).toHaveBeenCalledTimes(1);
});

it('should call handleClose from hook on fullscreen back button click', async () => {
renderSidebar({ currentSidebar: SIDEBAR_ID, shouldDisplayFullScreen: true });
const backButton = screen.getByRole('button', { name: messages.responsiveCloseNotificationTray.defaultMessage });
await user.click(backButton);
expect(mockHandleClose).toHaveBeenCalledTimes(1);
});

it('should call handleKeyDown from hook on standard close button keydown', () => {
renderSidebar({ currentSidebar: SIDEBAR_ID });
const closeButton = screen.getByRole('button', { name: messages.closeNotificationTrigger.defaultMessage });
fireEvent.keyDown(closeButton, { key: 'Tab' });
expect(mockHandleKeyDown).toHaveBeenCalledTimes(1);
});

it('should call handleBackBtnKeyDown from hook on fullscreen back button keydown', () => {
renderSidebar({ currentSidebar: SIDEBAR_ID, shouldDisplayFullScreen: true });
const backButton = screen.getByRole('button', { name: messages.responsiveCloseNotificationTray.defaultMessage });
fireEvent.keyDown(backButton, { key: 'Enter' });
expect(mockHandleBackBtnKeyDown).toHaveBeenCalledTimes(1);
});

it('should call toggleSidebar(null) upon receiving a "close" postMessage event', async () => {
renderSidebar({ currentSidebar: SIDEBAR_ID });

fireEvent(window, new MessageEvent('message', {
data: { type: 'learning.events.sidebar.close' },
}));

await waitFor(() => {
expect(mockContextValue.toggleSidebar).toHaveBeenCalledTimes(1);
expect(mockContextValue.toggleSidebar).toHaveBeenCalledWith(null);
});
});
});
35 changes: 26 additions & 9 deletions src/courseware/course/sidebar/common/SidebarBase.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@ import { Icon, IconButton } from '@openedx/paragon';
import { ArrowBackIos, Close } from '@openedx/paragon/icons';
import classNames from 'classnames';
import PropTypes from 'prop-types';
import { useCallback, useContext } from 'react';
import {
useCallback, useContext,
} from 'react';

import { useEventListener } from '@src/generic/hooks';
import messages from '../../messages';
import messages from '@src/courseware/course/messages';
import SidebarContext from '../SidebarContext';
import { useSidebarFocusAndKeyboard } from './hooks/useSidebarFocusAndKeyboard';

const SidebarBase = ({
title,
Expand All @@ -24,13 +28,22 @@ const SidebarBase = ({
currentSidebar,
} = useContext(SidebarContext);

const {
closeBtnRef,
backBtnRef,
handleClose,
handleKeyDown,
handleBackBtnKeyDown,
} = useSidebarFocusAndKeyboard(sidebarId);

const isOpen = currentSidebar === sidebarId;

const receiveMessage = useCallback(({ data }) => {
const { type } = data;
if (type === 'learning.events.sidebar.close') {
toggleSidebar(null);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [sidebarId, toggleSidebar]);
}, [toggleSidebar]);
Copy link
Contributor

Choose a reason for hiding this comment

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

[clarify]: Is there any reason why sidebarId was removed from deps? Also, do I need to remove // ​​eslint-disable-next-line react-hooks/exhaustive-deps?

Copy link
Author

Choose a reason for hiding this comment

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

no reason, removed

Copy link
Contributor

Choose a reason for hiding this comment

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

My question is related to the fact that sidebarId was previously included in the dependency array. Why was it removed?

Copy link
Author

Choose a reason for hiding this comment

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

because sidebarId it was an unnecessary dependency


useEventListener('message', receiveMessage);

Expand All @@ -39,7 +52,7 @@ const SidebarBase = ({
className={classNames('ml-0 border border-light-400 rounded-sm h-auto align-top zindex-0', {
'bg-white m-0 border-0 fixed-top vh-100 rounded-0': shouldDisplayFullScreen,
'align-self-start': !shouldDisplayFullScreen,
'd-none': currentSidebar !== sidebarId,
'd-none': !isOpen,
}, className)}
data-testid={`sidebar-${sidebarId}`}
style={{ width: shouldDisplayFullScreen ? '100%' : width }}
Expand All @@ -49,9 +62,10 @@ const SidebarBase = ({
{shouldDisplayFullScreen ? (
<div
className="pt-2 pb-2.5 border-bottom border-light-400 d-flex align-items-center ml-2"
onClick={() => toggleSidebar(null)}
onKeyDown={() => toggleSidebar(null)}
onClick={handleClose}
onKeyDown={handleBackBtnKeyDown}
role="button"
ref={backBtnRef}
tabIndex="0"
>
<Icon src={ArrowBackIos} />
Expand All @@ -63,16 +77,19 @@ const SidebarBase = ({
{showTitleBar && (
<>
<div className="d-flex align-items-center mb-2">
<strong className="p-2.5 d-inline-block course-sidebar-title">{title}</strong>
<h2 className="p-2.5 d-inline-block m-0 text-gray-700 h4">{title}</h2>
{shouldDisplayFullScreen
? null
: (
<div className="d-inline-flex mr-2 ml-auto">
<IconButton
className="sidebar-close-btn"
src={Close}
size="sm"
ref={closeBtnRef}
iconAs={Icon}
onClick={() => toggleSidebar(null)}
onClick={handleClose}
onKeyDown={handleKeyDown}
variant="primary"
alt={intl.formatMessage(messages.closeNotificationTrigger)}
/>
Expand Down
20 changes: 15 additions & 5 deletions src/courseware/course/sidebar/common/TriggerBase.jsx
Original file line number Diff line number Diff line change
@@ -1,27 +1,37 @@
import PropTypes from 'prop-types';
import React from 'react';
import { forwardRef } from 'react';

const SidebarTriggerBase = ({
const SidebarTriggerBase = forwardRef(({
onClick,
onKeyDown,
ariaLabel,
children,
}) => (
isOpenNotificationStatusBar,
sectionId,
}, ref) => (
<button
className="border border-light-400 bg-transparent align-items-center align-content-center d-flex notification-btn"
className="border border-light-400 bg-transparent align-items-center align-content-center d-flex notification-btn sidebar-trigger-btn"
type="button"
onClick={onClick}
onKeyDown={onKeyDown}
aria-label={ariaLabel}
aria-expanded={isOpenNotificationStatusBar}
aria-controls={sectionId}
ref={ref}
>
<div className="icon-container d-flex position-relative align-items-center">
{children}
</div>
</button>
);
));

SidebarTriggerBase.propTypes = {
onClick: PropTypes.func.isRequired,
onKeyDown: PropTypes.func.isRequired,
ariaLabel: PropTypes.string.isRequired,
children: PropTypes.element.isRequired,
isOpenNotificationStatusBar: PropTypes.bool.isRequired,
sectionId: PropTypes.string.isRequired,
};

export default SidebarTriggerBase;
Loading