Skip to content

Commit c607da0

Browse files
committed
fix: fixes from review
1 parent 32fd819 commit c607da0

10 files changed

Lines changed: 54 additions & 45 deletions

File tree

src/course-outline/outline-sidebar/OutlineHelpSidebar.test.tsx

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,30 @@
1-
import { initializeMocks, render, waitFor } from '../../testUtils';
2-
import { helpUrls } from '../../help-urls/__mocks__';
3-
import { getHelpUrlsApiUrl } from '../../help-urls/data/api';
1+
import { initializeMocks, render, waitFor } from '@src/testUtils';
2+
import { helpUrls } from '@src/help-urls/__mocks__';
3+
import { getHelpUrlsApiUrl } from '@src/help-urls/data/api';
4+
import { CourseAuthoringProvider } from '@src/CourseAuthoringContext';
5+
46
import OutlineHelpSidebar from './OutlineHelpSidebar';
57
import messages from './messages';
68

9+
const courseId = '123';
10+
711
jest.mock('@edx/frontend-platform/i18n', () => ({
812
...jest.requireActual('@edx/frontend-platform/i18n'),
913
useIntl: () => ({
1014
formatMessage: (message) => message.defaultMessage,
1115
}),
1216
}));
1317

18+
const extraWrapper = ({ children }) => (
19+
<CourseAuthoringProvider courseId={courseId}>
20+
{children}
21+
</CourseAuthoringProvider>
22+
);
23+
1424
let axiosMock;
1525
const mockPathname = '/foo-bar';
16-
const courseId = '123';
1726

18-
const renderComponent = () => render(<OutlineHelpSidebar courseId={courseId} />, { path: mockPathname });
27+
const renderComponent = () => render(<OutlineHelpSidebar />, { path: mockPathname, extraWrapper });
1928

2029
describe('<OutlineSidebar />', () => {
2130
beforeEach(() => {

src/course-outline/outline-sidebar/OutlineHelpSidebar.tsx

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,18 @@ import { useIntl } from '@edx/frontend-platform/i18n';
33

44
import { HelpSidebar } from '@src/generic/help-sidebar';
55
import { useHelpUrls } from '@src/help-urls/hooks';
6+
import { useCourseAuthoringContext } from '@src/CourseAuthoringContext';
67

78
import { getFormattedSidebarMessages } from './utils';
89

9-
interface OutlineHelpSidebarProps {
10-
courseId: string;
11-
}
12-
13-
const OutlineHelpSideBar = ({ courseId }: OutlineHelpSidebarProps) => {
10+
const OutlineHelpSideBar = () => {
1411
const intl = useIntl();
1512
const {
1613
visibility: learnMoreVisibilityUrl,
1714
grading: learnMoreGradingUrl,
1815
outline: learnMoreOutlineUrl,
1916
} = useHelpUrls(['visibility', 'grading', 'outline']);
17+
const { courseId } = useCourseAuthoringContext();
2018

2119
const sidebarMessages = getFormattedSidebarMessages(
2220
{

src/course-outline/outline-sidebar/OutlineInfoSidebar.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,16 @@ import { SchoolOutline, Tag } from '@openedx/paragon/icons';
55
import { ContentTagsDrawerSheet, ContentTagsSnippet } from '@src/content-tags-drawer';
66
import { ComponentCountSnippet } from '@src/generic/block-type-utils';
77
import { useGetBlockTypes } from '@src/search-manager';
8+
import { useCourseAuthoringContext } from '@src/CourseAuthoringContext';
89

910
import { SidebarContent, SidebarSection, SidebarTitle } from '@src/generic/sidebar';
1011
import { useCourseDetails } from '../data/apiHooks';
1112

1213
import messages from './messages';
1314

14-
export const OutlineInfoSidebar = ({ courseId }: { courseId: string }) => {
15+
export const OutlineInfoSidebar = () => {
1516
const intl = useIntl();
17+
const { courseId } = useCourseAuthoringContext();
1618
const { data: courseDetails } = useCourseDetails(courseId);
1719

1820
const { data: componentData } = useGetBlockTypes(

src/course-outline/outline-sidebar/OutlineSidebar.test.tsx

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,29 @@ import { userEvent } from '@testing-library/user-event';
44
import {
55
initializeMocks, render, screen, waitFor, within,
66
} from '@src/testUtils';
7+
import { CourseAuthoringProvider } from '@src/CourseAuthoringContext';
78

89
import { OutlineSidebarProvider } from './OutlineSidebarContext';
910
import OutlineSidebar from './OutlineSidebar';
1011

11-
const courseId = 'course-v1:TestOrg+TestCourse+2023_1';
12-
1312
// Mock the useCourseDetails hook
1413
jest.mock('@src/course-outline/data/apiHooks', () => ({
1514
useCourseDetails: jest.fn().mockReturnValue({ isPending: false, data: { title: 'Test Course' } }),
1615
}));
1716

17+
const courseId = '123';
18+
19+
const extraWrapper = ({ children }) => (
20+
<CourseAuthoringProvider courseId={courseId}>
21+
<OutlineSidebarProvider>
22+
{children}
23+
</OutlineSidebarProvider>
24+
</CourseAuthoringProvider>
25+
);
26+
1827
const renderComponent = () => render(
19-
<OutlineSidebar courseId={courseId} />,
20-
{ extraWrapper: OutlineSidebarProvider },
28+
<OutlineSidebar />,
29+
{ extraWrapper },
2130
);
2231

2332
describe('<OutlineSidebar>', () => {

src/course-outline/outline-sidebar/OutlineSidebar.tsx

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,7 @@ import { Sidebar } from '@src/generic/sidebar';
77
import OutlineHelpSidebar from './OutlineHelpSidebar';
88
import { useOutlineSidebarContext } from './OutlineSidebarContext';
99

10-
interface OutlineSideBarProps {
11-
courseId: string;
12-
}
13-
14-
const OutlineSideBar = ({ courseId }: OutlineSideBarProps) => {
10+
const OutlineSideBar = () => {
1511
const isMedium = useMediaQuery({ maxWidth: breakpoints.medium.maxWidth });
1612
const showNewSidebar = getConfig().ENABLE_COURSE_OUTLINE_NEW_DESIGN?.toString().toLowerCase() === 'true';
1713

@@ -29,7 +25,7 @@ const OutlineSideBar = ({ courseId }: OutlineSideBarProps) => {
2925
const colSpan = isMedium ? 'col-12' : 'col-3';
3026
return (
3127
<div className={colSpan}>
32-
<OutlineHelpSidebar courseId={courseId} />
28+
<OutlineHelpSidebar />
3329
</div>
3430
);
3531
}
@@ -41,7 +37,6 @@ const OutlineSideBar = ({ courseId }: OutlineSideBarProps) => {
4137
setCurrentPageKey={setCurrentPageKey}
4238
isOpen={isOpen}
4339
toggle={toggle}
44-
contentProps={{ courseId }}
4540
/>
4641
);
4742
};

src/course-outline/outline-sidebar/messages.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,12 @@ const messages = defineMessages({
6868
sidebarButtonHelp: {
6969
id: 'course-authoring.course-outline.sidebar.sidebar-button-help',
7070
defaultMessage: 'Help',
71+
description: 'Button label for the help sidebar',
7172
},
7273
sidebarButtonInfo: {
7374
id: 'course-authoring.course-outline.sidebar.sidebar-button-info',
7475
defaultMessage: 'Info',
76+
description: 'Button label for the info sidebar',
7577
},
7678
sidebarSectionSummary: {
7779
id: 'course-authoring.course-outline.sidebar.sidebar-section-summary',

src/generic/sidebar/Sidebar.test.tsx

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import { useToggle } from '@openedx/paragon';
88

99
import { Sidebar } from '.';
1010

11-
const Component1 = ({ text }: { text: string }) => <div>Component 1 ({text})</div>;
12-
const Component2 = ({ text }: { text: string }) => <div>Component 2 ({text})</div>;
11+
const Component1 = () => <div>Component 1</div>;
12+
const Component2 = () => <div>Component 2</div>;
1313
const Icon1 = () => <div>Icon 1</div>;
1414
const Icon2 = () => <div>Icon 2</div>;
1515
const pages = {
@@ -36,7 +36,6 @@ const TestSidebar = () => {
3636
setCurrentPageKey={setPageKey}
3737
isOpen={isOpen}
3838
toggle={toggle}
39-
contentProps={{ text: 'content props' }}
4039
/>
4140
);
4241
};
@@ -50,7 +49,7 @@ describe('<Sidebar>', () => {
5049
render(<TestSidebar />);
5150

5251
// Check the Page 1 content
53-
expect(screen.getByText('Component 1 (content props)')).toBeInTheDocument();
52+
expect(screen.getByText('Component 1')).toBeInTheDocument();
5453

5554
// Check the IconButtonToggle
5655
const sidebarToggle = screen.getByTestId('sidebar-toggle');
@@ -69,22 +68,22 @@ describe('<Sidebar>', () => {
6968
render(<TestSidebar />);
7069

7170
// Check the Page 1 content
72-
expect(screen.getByText('Component 1 (content props)')).toBeInTheDocument();
71+
expect(screen.getByText('Component 1')).toBeInTheDocument();
7372

7473
const page2Button = screen.getByRole('button', { name: 'Page 2' });
7574
await userEvent.click(page2Button);
7675

7776
expect(page2Button).toHaveAttribute('aria-selected', 'true');
7877

7978
// Check the Page 2 content
80-
expect(screen.getByText('Component 2 (content props)')).toBeInTheDocument();
79+
expect(screen.getByText('Component 2')).toBeInTheDocument();
8180

8281
const page1Button = screen.getByRole('button', { name: 'Page 1' });
8382
expect(page1Button).toHaveAttribute('aria-selected', 'false');
8483
await userEvent.click(page1Button);
8584

8685
// Check the Page 1 content
87-
expect(screen.getByText('Component 1 (content props)')).toBeInTheDocument();
86+
expect(screen.getByText('Component 1')).toBeInTheDocument();
8887
});
8988

9089
it('should change pages using the dropdown button', async () => {
@@ -94,7 +93,7 @@ describe('<Sidebar>', () => {
9493
expect(sidebarDropdown).toBeInTheDocument();
9594

9695
// Check the Page 1 content
97-
expect(screen.getByText('Component 1 (content props)')).toBeInTheDocument();
96+
expect(screen.getByText('Component 1')).toBeInTheDocument();
9897

9998
// Click on the dropdown button
10099
await userEvent.click(within(sidebarDropdown).getByRole('button', { name: 'Page 1 Icon 1' }));
@@ -104,7 +103,7 @@ describe('<Sidebar>', () => {
104103
await userEvent.click(page2Button);
105104

106105
// Check the Page 2 content
107-
expect(screen.getByText('Component 2 (content props)')).toBeInTheDocument();
106+
expect(screen.getByText('Component 2')).toBeInTheDocument();
108107

109108
// Click on the dropdown button again
110109
await userEvent.click(within(sidebarDropdown).getByRole('button', { name: 'Page 2 Icon 2' }));
@@ -114,7 +113,7 @@ describe('<Sidebar>', () => {
114113
await userEvent.click(page1Button);
115114

116115
// Check the Page 1 content
117-
expect(screen.getByText('Component 1 (content props)')).toBeInTheDocument();
116+
expect(screen.getByText('Component 1')).toBeInTheDocument();
118117
});
119118

120119
it('should toggle the sidebar', async () => {
@@ -124,20 +123,20 @@ describe('<Sidebar>', () => {
124123
expect(sidebarToggle).toBeInTheDocument();
125124

126125
// Check the Page 1 content
127-
expect(screen.getByText('Component 1 (content props)')).toBeInTheDocument();
126+
expect(screen.getByText('Component 1')).toBeInTheDocument();
128127

129128
// Hide the sidebar
130129
const toggleButton = within(sidebarToggle).getByRole('button', { name: 'Toggle' });
131130
expect(toggleButton).toBeInTheDocument();
132131
await userEvent.click(toggleButton);
133132

134133
// Check the Page 1 content is hidden
135-
expect(screen.queryByText('Component 1 (content props)')).not.toBeInTheDocument();
134+
expect(screen.queryByText('Component 1')).not.toBeInTheDocument();
136135

137136
// Show the sidebar
138137
await userEvent.click(toggleButton);
139138

140139
// Check the Page 1 content
141-
expect(screen.getByText('Component 1 (content props)')).toBeInTheDocument();
140+
expect(screen.getByText('Component 1')).toBeInTheDocument();
142141
});
143142
});

src/generic/sidebar/Sidebar.tsx

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,6 @@ interface SidebarProps<T extends SidebarPages> {
3939
isOpen: boolean;
4040
/** Function that toggles the sidebar */
4141
toggle: () => void;
42-
/** Props that are passed to the component that is rendered in the sidebar.
43-
* This component is defined in the pages object */
44-
contentProps: React.ComponentProps<T[keyof T]['component']>;
4542
}
4643

4744
/**
@@ -74,7 +71,6 @@ interface SidebarProps<T extends SidebarPages> {
7471
* currentPageKey="help"
7572
* isOpen={isOpen}
7673
* toggle={toggle}
77-
* contentProps={{ courseId }}
7874
* />
7975
*);
8076
* ```
@@ -86,7 +82,6 @@ export function Sidebar<T extends SidebarPages>({
8682
setCurrentPageKey,
8783
isOpen,
8884
toggle,
89-
contentProps,
9085
}: SidebarProps<T>) {
9186
const intl = useIntl();
9287

@@ -120,7 +115,7 @@ export function Sidebar<T extends SidebarPages>({
120115
))}
121116
</Dropdown.Menu>
122117
</Dropdown>
123-
<SidebarComponent {...contentProps} />
118+
<SidebarComponent />
124119
</div>
125120
)}
126121
<div className="sidebar-toggle" data-testid="sidebar-toggle">

src/generic/sidebar/SidebarContent.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import React from 'react';
22
import { Stack } from '@openedx/paragon';
33

44
interface SidebarContentProps {
5-
children: React.ReactNode[];
5+
children: React.ReactNode | React.ReactNode[],
66
}
77

88
/**
@@ -29,12 +29,12 @@ interface SidebarContentProps {
2929
*/
3030
export const SidebarContent = ({ children } : SidebarContentProps) => (
3131
<Stack gap={1}>
32-
{children.map((child, index) => (
32+
{Array.isArray(children) ? children.map((child, index) => (
3333
// eslint-disable-next-line react/no-array-index-key
3434
<React.Fragment key={index}>
3535
{child}
3636
{index !== children.length - 1 && <hr className="w-100" />}
3737
</React.Fragment>
38-
))}
38+
)) : children}
3939
</Stack>
4040
);

src/plugin-slots/CourseAuthoringOutlineSidebarSlot/index.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ export const CourseAuthoringOutlineSidebarSlot = ({
1616
sections,
1717
}}
1818
>
19-
<OutlineSideBar courseId={courseId} />
19+
<OutlineSideBar />
2020
</PluginSlot>
2121
);
2222

0 commit comments

Comments
 (0)