Skip to content

Commit b35cb90

Browse files
fix: fix discussions and some refactor
1 parent 0119739 commit b35cb90

File tree

5 files changed

+56
-27
lines changed

5 files changed

+56
-27
lines changed

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ const SequenceNavigation = ({
8585
} else if (!shouldDisplayNotificationTriggerInSequence) {
8686
buttonText = intl.formatMessage(messages.nextButton);
8787
}
88+
89+
if (!buttonText) { return null; }
8890
return navigationDisabledNextSequence || (
8991
<NextUnitTopNavTriggerSlot
9092
{...{
@@ -106,7 +108,7 @@ const SequenceNavigation = ({
106108
data-testid="courseware-sequence-navigation"
107109
className={classNames('sequence-navigation', className, { 'mr-2': shouldDisplayNotificationTriggerInSequence })}
108110
style={{ width: shouldDisplayNotificationTriggerInSequence ? '90%' : null }}
109-
aria-label="course sequence tabs"
111+
aria-label={intl.formatMessage(messages.sequenceNavLabel)}
110112
>
111113
{renderPreviousButton()}
112114
{renderUnitButtons()}

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

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,18 @@
11
import React from 'react';
22
import { Factory } from 'rosie';
3-
import { useWindowSize, breakpoints } from '@openedx/paragon';
3+
import { breakpoints } from '@openedx/paragon';
44

55
import {
66
render, screen, fireEvent, getByText, initializeTestStore,
77
} from '../../../../setupTest';
88
import SequenceNavigation from './SequenceNavigation';
99
import useIndexOfLastVisibleChild from '../../../../generic/tabs/useIndexOfLastVisibleChild';
10+
import messages from './messages';
1011

1112
// Mock the hook to avoid relying on its implementation and mocking `getBoundingClientRect`.
1213
jest.mock('../../../../generic/tabs/useIndexOfLastVisibleChild');
1314
useIndexOfLastVisibleChild.mockReturnValue([0, null, null]);
1415

15-
jest.mock('@openedx/paragon', () => {
16-
const original = jest.requireActual('@openedx/paragon');
17-
return {
18-
...original,
19-
breakpoints: original.breakpoints,
20-
useWindowSize: jest.fn(),
21-
};
22-
});
23-
2416
describe('Sequence Navigation', () => {
2517
let mockData;
2618
const courseMetadata = Factory.build('courseMetadata');
@@ -40,7 +32,19 @@ describe('Sequence Navigation', () => {
4032
onNavigate: () => {},
4133
nextHandler: () => {},
4234
};
43-
useWindowSize.mockReturnValue({ width: 1024, height: 800 });
35+
Object.defineProperty(window, 'innerWidth', {
36+
writable: true,
37+
configurable: true,
38+
value: 1024,
39+
});
40+
});
41+
42+
afterEach(() => {
43+
Object.defineProperty(window, 'innerWidth', {
44+
writable: true,
45+
configurable: true,
46+
value: 1024,
47+
});
4448
});
4549

4650
it('is empty while loading', async () => {
@@ -223,20 +227,28 @@ describe('Sequence Navigation', () => {
223227
});
224228

225229
it('shows previous button without label when screen is small', () => {
226-
useWindowSize.mockReturnValue({ width: breakpoints.small.minWidth - 1 });
230+
Object.defineProperty(window, 'innerWidth', {
231+
writable: true,
232+
configurable: true,
233+
value: breakpoints.small.minWidth - 1,
234+
});
227235

228-
render(<SequenceNavigation {...mockData} />, { wrapWithRouter: true });
236+
const { container } = render(<SequenceNavigation {...mockData} />, { wrapWithRouter: true });
229237

230-
const prevButton = screen.getByRole('button');
231-
expect(prevButton.textContent).toBe('2 of 3');
238+
const prevButton = container.querySelector('.previous-btn');
239+
expect(prevButton).toBeInTheDocument();
240+
expect(screen.queryByText(messages.previousButton.defaultMessage)).not.toBeInTheDocument();
232241
});
233242

234243
it('does not set width when screen is large', () => {
235-
useWindowSize.mockReturnValue({ width: breakpoints.small.minWidth });
236-
244+
Object.defineProperty(window, 'innerWidth', {
245+
writable: true,
246+
configurable: true,
247+
value: breakpoints.small.minWidth,
248+
});
237249
render(<SequenceNavigation {...mockData} />, { wrapWithRouter: true });
238250

239-
const nav = screen.getByRole('navigation', { name: /course sequence tabs/i });
251+
const nav = screen.getByRole('navigation', { name: messages.sequenceNavLabel.defaultMessage });
240252
expect(nav).not.toHaveStyle({ width: '90%' });
241253
});
242254
});

src/courseware/course/sequence/sequence-navigation/SequenceNavigationDropdown.test.jsx

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import React from 'react';
22
import { Factory } from 'rosie';
33
import { getAllByRole } from '@testing-library/dom';
44
import { act } from '@testing-library/react';
5+
import userEvent from '@testing-library/user-event';
6+
57
import SequenceNavigationDropdown from './SequenceNavigationDropdown';
68
import {
79
render, screen, fireEvent, initializeTestStore,
@@ -60,7 +62,7 @@ describe('Sequence Navigation Dropdown', () => {
6062
});
6163
});
6264

63-
it('handles the clicks', () => {
65+
it('handles the clicks', async () => {
6466
const onNavigate = jest.fn();
6567
const { container } = render(
6668
<SequenceNavigationDropdown {...mockData} onNavigate={onNavigate} />,
@@ -72,7 +74,13 @@ describe('Sequence Navigation Dropdown', () => {
7274
fireEvent.click(dropdownToggle);
7375
});
7476
const dropdownMenu = container.querySelector('.dropdown-menu');
75-
getAllByRole(dropdownMenu, 'tab', { hidden: true }).forEach(button => fireEvent.click(button));
77+
const buttons = getAllByRole(dropdownMenu, 'tab', { hidden: true });
78+
79+
for (const button of buttons) {
80+
// eslint-disable-next-line no-await-in-loop
81+
await userEvent.click(button);
82+
}
83+
7684
expect(onNavigate).toHaveBeenCalledTimes(unitBlocks.length);
7785
unitBlocks.forEach((unit, index) => {
7886
expect(onNavigate).toHaveBeenNthCalledWith(index + 1, unit.id);

src/courseware/course/sequence/sequence-navigation/UnitButton.test.jsx

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import React from 'react';
22
import { Factory } from 'rosie';
3+
import userEvent from '@testing-library/user-event';
4+
35
import {
4-
fireEvent, initializeTestStore, render, screen,
5-
} from '../../../../setupTest';
6+
initializeTestStore, render, screen,
7+
} from '@src/setupTest';
68
import UnitButton from './UnitButton';
79

810
jest.mock('react-router-dom', () => {
@@ -89,18 +91,18 @@ describe('Unit Button', () => {
8991
expect(bookmarkIcon.getAttribute('data-testid')).toBe('bookmark-icon');
9092
});
9193

92-
it('handles the click', () => {
94+
it('handles the click', async () => {
9395
const onClick = jest.fn();
9496
render(<UnitButton {...mockData} onClick={onClick} />, { wrapWithRouter: true });
95-
fireEvent.click(screen.getByRole('tab'));
97+
await userEvent.click(screen.getByRole('tab'));
9698
expect(onClick).toHaveBeenCalledTimes(1);
9799
});
98100

99-
it('calls onClick with correct unitId when clicked', () => {
101+
it('calls onClick with correct unitId when clicked', async () => {
100102
const onClick = jest.fn();
101103
render(<UnitButton {...mockData} onClick={onClick} />, { wrapWithRouter: true });
102104

103-
fireEvent.click(screen.getByRole('tab'));
105+
await userEvent.click(screen.getByRole('tab'));
104106

105107
expect(onClick).toHaveBeenCalledWith(mockData.unitId);
106108
});

src/courseware/course/sequence/sequence-navigation/messages.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ const messages = defineMessages({
1616
defaultMessage: 'Previous',
1717
description: 'Button to return to the previous section',
1818
},
19+
sequenceNavLabel: {
20+
id: 'learn.sequence.navigation.aria.label',
21+
defaultMessage: 'Course sequence tabs',
22+
description: 'Accessibility label for the courseware sequence navigation bar',
23+
},
1924
});
2025

2126
export default messages;

0 commit comments

Comments
 (0)