Skip to content

Commit 0119739

Browse files
feat: accessibility HTML attributes for Course Navigation top bar
1 parent d0a8778 commit 0119739

File tree

8 files changed

+177
-11
lines changed

8 files changed

+177
-11
lines changed

src/course-home/live-tab/LiveTab.jsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const LiveTab = () => {
1313
return (
1414
<div
1515
id="live_tab"
16+
role="region"
1617
// eslint-disable-next-line react/no-danger
1718
dangerouslySetInnerHTML={{ __html: liveModel[courseId]?.iframe }}
1819
/>

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,13 @@ const SequenceNavigation = ({
101101
};
102102

103103
return sequenceStatus === LOADED ? (
104-
<nav id="courseware-sequence-navigation" data-testid="courseware-sequence-navigation" className={classNames('sequence-navigation', className, { 'mr-2': shouldDisplayNotificationTriggerInSequence })}>
104+
<nav
105+
id="courseware-sequence-navigation"
106+
data-testid="courseware-sequence-navigation"
107+
className={classNames('sequence-navigation', className, { 'mr-2': shouldDisplayNotificationTriggerInSequence })}
108+
style={{ width: shouldDisplayNotificationTriggerInSequence ? '90%' : null }}
109+
aria-label="course sequence tabs"
110+
>
105111
{renderPreviousButton()}
106112
{renderUnitButtons()}
107113
{renderNextButton()}

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

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import React from 'react';
22
import { Factory } from 'rosie';
3+
import { useWindowSize, breakpoints } from '@openedx/paragon';
4+
35
import {
46
render, screen, fireEvent, getByText, initializeTestStore,
57
} from '../../../../setupTest';
@@ -10,6 +12,15 @@ import useIndexOfLastVisibleChild from '../../../../generic/tabs/useIndexOfLastV
1012
jest.mock('../../../../generic/tabs/useIndexOfLastVisibleChild');
1113
useIndexOfLastVisibleChild.mockReturnValue([0, null, null]);
1214

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+
1324
describe('Sequence Navigation', () => {
1425
let mockData;
1526
const courseMetadata = Factory.build('courseMetadata');
@@ -29,6 +40,7 @@ describe('Sequence Navigation', () => {
2940
onNavigate: () => {},
3041
nextHandler: () => {},
3142
};
43+
useWindowSize.mockReturnValue({ width: 1024, height: 800 });
3244
});
3345

3446
it('is empty while loading', async () => {
@@ -76,7 +88,7 @@ describe('Sequence Navigation', () => {
7688
const onNavigate = jest.fn();
7789
render(<SequenceNavigation {...mockData} {...{ onNavigate }} />, { wrapWithRouter: true });
7890

79-
const unitButtons = screen.getAllByRole('link', { name: /\d+/ });
91+
const unitButtons = screen.getAllByRole('tab', { name: /\d+/ });
8092
expect(unitButtons).toHaveLength(unitButtons.length);
8193
unitButtons.forEach(button => fireEvent.click(button));
8294
expect(onNavigate).toHaveBeenCalledTimes(unitButtons.length);
@@ -209,4 +221,22 @@ describe('Sequence Navigation', () => {
209221
);
210222
expect(screen.queryByRole('link', { name: /next/i })).not.toBeInTheDocument();
211223
});
224+
225+
it('shows previous button without label when screen is small', () => {
226+
useWindowSize.mockReturnValue({ width: breakpoints.small.minWidth - 1 });
227+
228+
render(<SequenceNavigation {...mockData} />, { wrapWithRouter: true });
229+
230+
const prevButton = screen.getByRole('button');
231+
expect(prevButton.textContent).toBe('2 of 3');
232+
});
233+
234+
it('does not set width when screen is large', () => {
235+
useWindowSize.mockReturnValue({ width: breakpoints.small.minWidth });
236+
237+
render(<SequenceNavigation {...mockData} />, { wrapWithRouter: true });
238+
239+
const nav = screen.getByRole('navigation', { name: /course sequence tabs/i });
240+
expect(nav).not.toHaveStyle({ width: '90%' });
241+
});
212242
});

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ describe('Sequence Navigation Dropdown', () => {
5050
});
5151
const dropdownMenu = container.querySelector('.dropdown-menu');
5252
// Only the current unit should be marked as active.
53-
getAllByRole(dropdownMenu, 'link', { hidden: true }).forEach(button => {
53+
getAllByRole(dropdownMenu, 'tab', { hidden: true }).forEach(button => {
5454
if (button.textContent === unit.display_name) {
5555
expect(button).toHaveClass('active');
5656
} else {
@@ -72,7 +72,7 @@ describe('Sequence Navigation Dropdown', () => {
7272
fireEvent.click(dropdownToggle);
7373
});
7474
const dropdownMenu = container.querySelector('.dropdown-menu');
75-
getAllByRole(dropdownMenu, 'link', { hidden: true }).forEach(button => fireEvent.click(button));
75+
getAllByRole(dropdownMenu, 'tab', { hidden: true }).forEach(button => fireEvent.click(button));
7676
expect(onNavigate).toHaveBeenCalledTimes(unitBlocks.length);
7777
unitBlocks.forEach((unit, index) => {
7878
expect(onNavigate).toHaveBeenNthCalledWith(index + 1, unit.id);

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ const SequenceNavigationTabs = ({
3838
<div
3939
className="sequence-navigation-tabs d-flex flex-grow-1"
4040
style={shouldDisplayDropdown ? invisibleStyle : null}
41+
role="tablist"
4142
ref={containerRef}
4243
>
4344
{unitIds.map(buttonUnitId => (

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

Lines changed: 84 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,27 @@
1-
import React from 'react';
21
import { Factory } from 'rosie';
32
import { getAllByRole } from '@testing-library/react';
43
import userEvent from '@testing-library/user-event';
54

65
import { initializeTestStore, render, screen } from '../../../../setupTest';
76
import SequenceNavigationTabs from './SequenceNavigationTabs';
87
import useIndexOfLastVisibleChild from '../../../../generic/tabs/useIndexOfLastVisibleChild';
8+
import {
9+
useIsSidebarOpen,
10+
useIsOnXLDesktop,
11+
useIsOnLargeDesktop,
12+
useIsOnMediumDesktop,
13+
} from './hooks';
914

1015
// Mock the hook to avoid relying on its implementation and mocking `getBoundingClientRect`.
1116
jest.mock('../../../../generic/tabs/useIndexOfLastVisibleChild');
1217

18+
jest.mock('./hooks', () => ({
19+
useIsSidebarOpen: jest.fn(),
20+
useIsOnXLDesktop: jest.fn(),
21+
useIsOnLargeDesktop: jest.fn(),
22+
useIsOnMediumDesktop: jest.fn(),
23+
}));
24+
1325
describe('Sequence Navigation Tabs', () => {
1426
let mockData;
1527

@@ -44,7 +56,7 @@ describe('Sequence Navigation Tabs', () => {
4456
useIndexOfLastVisibleChild.mockReturnValue([0, null, null]);
4557
render(<SequenceNavigationTabs {...mockData} />, { wrapWithRouter: true });
4658

47-
expect(screen.getAllByRole('link')).toHaveLength(unitBlocks.length);
59+
expect(screen.getAllByRole('tab')).toHaveLength(unitBlocks.length);
4860
});
4961

5062
it('renders unit buttons and dropdown button', async () => {
@@ -54,17 +66,85 @@ describe('Sequence Navigation Tabs', () => {
5466
const booyah = render(<SequenceNavigationTabs {...mockData} />, { wrapWithRouter: true });
5567

5668
// wait for links to appear so we aren't testing an empty div
57-
await screen.findAllByRole('link');
69+
await screen.findAllByRole('tab');
5870

5971
container = booyah.container;
6072

6173
const dropdownToggle = container.querySelector('.dropdown-toggle');
6274
await userEvent.click(dropdownToggle);
6375

6476
const dropdownMenu = container.querySelector('.dropdown');
65-
const dropdownButtons = getAllByRole(dropdownMenu, 'link');
77+
const dropdownButtons = getAllByRole(dropdownMenu, 'tab');
6678
expect(dropdownButtons).toHaveLength(unitBlocks.length);
6779
expect(screen.getByRole('button', { name: `${activeBlockNumber} of ${unitBlocks.length}` }))
6880
.toHaveClass('dropdown-toggle');
6981
});
82+
83+
it('adds class navigation-tab-width-xl when isOnXLDesktop and sidebar is open', () => {
84+
useIsSidebarOpen.mockReturnValue(true);
85+
useIsOnXLDesktop.mockReturnValue(true);
86+
useIsOnLargeDesktop.mockReturnValue(false);
87+
useIsOnMediumDesktop.mockReturnValue(false);
88+
useIndexOfLastVisibleChild.mockReturnValue([0, null, null]);
89+
90+
const { container } = render(<SequenceNavigationTabs {...mockData} />, { wrapWithRouter: true });
91+
92+
const wrapperDiv = container.querySelector('.navigation-tab-width-xl');
93+
expect(wrapperDiv).toBeInTheDocument();
94+
});
95+
96+
it('adds class navigation-tab-width-large when isOnLargeDesktop and sidebar is open', () => {
97+
useIsSidebarOpen.mockReturnValue(true);
98+
useIsOnXLDesktop.mockReturnValue(false);
99+
useIsOnLargeDesktop.mockReturnValue(true);
100+
useIsOnMediumDesktop.mockReturnValue(false);
101+
useIndexOfLastVisibleChild.mockReturnValue([0, null, null]);
102+
103+
const { container } = render(<SequenceNavigationTabs {...mockData} />, { wrapWithRouter: true });
104+
105+
const wrapperDiv = container.querySelector('.navigation-tab-width-large');
106+
expect(wrapperDiv).toBeInTheDocument();
107+
});
108+
109+
it('adds class navigation-tab-width-medium when isOnMediumDesktop and sidebar is open', () => {
110+
useIsSidebarOpen.mockReturnValue(true);
111+
useIsOnXLDesktop.mockReturnValue(false);
112+
useIsOnLargeDesktop.mockReturnValue(false);
113+
useIsOnMediumDesktop.mockReturnValue(true);
114+
useIndexOfLastVisibleChild.mockReturnValue([0, null, null]);
115+
116+
const { container } = render(<SequenceNavigationTabs {...mockData} />, { wrapWithRouter: true });
117+
118+
const wrapperDiv = container.querySelector('.navigation-tab-width-medium');
119+
expect(wrapperDiv).toBeInTheDocument();
120+
});
121+
122+
it('applies invisibleStyle when shouldDisplayDropdown is true', () => {
123+
useIsSidebarOpen.mockReturnValue(true);
124+
useIsOnXLDesktop.mockReturnValue(false);
125+
useIsOnLargeDesktop.mockReturnValue(false);
126+
useIsOnMediumDesktop.mockReturnValue(false);
127+
128+
useIndexOfLastVisibleChild.mockReturnValue([
129+
0,
130+
null,
131+
{
132+
position: 'absolute',
133+
left: '0px',
134+
pointerEvents: 'none',
135+
visibility: 'hidden',
136+
maxWidth: '100%',
137+
},
138+
]);
139+
140+
render(<SequenceNavigationTabs {...mockData} />, { wrapWithRouter: true });
141+
142+
const tabList = screen.getByRole('tablist');
143+
144+
expect(tabList.style.position).toBe('absolute');
145+
expect(tabList.style.left).toBe('0px');
146+
expect(tabList.style.pointerEvents).toBe('none');
147+
expect(tabList.style.visibility).toBe('hidden');
148+
expect(tabList.style.maxWidth).toBe('100%');
149+
});
70150
});

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ const UnitButton = ({
3939
variant="link"
4040
onClick={handleClick}
4141
title={title}
42+
role="tab"
43+
aria-selected={isActive}
44+
aria-controls={title}
4245
as={Link}
4346
to={unitPath}
4447
>

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

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,22 @@ import {
55
} from '../../../../setupTest';
66
import UnitButton from './UnitButton';
77

8+
jest.mock('react-router-dom', () => {
9+
const actual = jest.requireActual('react-router-dom');
10+
return {
11+
...actual,
12+
useLocation: () => ({ pathname: '/preview/anything' }),
13+
};
14+
});
15+
816
describe('Unit Button', () => {
917
let mockData;
1018
const courseMetadata = Factory.build('courseMetadata');
19+
20+
afterEach(() => {
21+
jest.resetModules();
22+
});
23+
1124
const unitBlocks = [Factory.build(
1225
'block',
1326
{ type: 'problem' },
@@ -33,12 +46,12 @@ describe('Unit Button', () => {
3346

3447
it('hides title by default', () => {
3548
render(<UnitButton {...mockData} />, { wrapWithRouter: true });
36-
expect(screen.getByRole('link')).not.toHaveTextContent(unit.display_name);
49+
expect(screen.getByRole('tab')).not.toHaveTextContent(unit.display_name);
3750
});
3851

3952
it('shows title', () => {
4053
render(<UnitButton {...mockData} showTitle />, { wrapWithRouter: true });
41-
expect(screen.getByRole('link')).toHaveTextContent(unit.display_name);
54+
expect(screen.getByRole('tab')).toHaveTextContent(unit.display_name);
4255
});
4356

4457
it('does not show completion for non-completed unit', () => {
@@ -79,7 +92,39 @@ describe('Unit Button', () => {
7992
it('handles the click', () => {
8093
const onClick = jest.fn();
8194
render(<UnitButton {...mockData} onClick={onClick} />, { wrapWithRouter: true });
82-
fireEvent.click(screen.getByRole('link'));
95+
fireEvent.click(screen.getByRole('tab'));
8396
expect(onClick).toHaveBeenCalledTimes(1);
8497
});
98+
99+
it('calls onClick with correct unitId when clicked', () => {
100+
const onClick = jest.fn();
101+
render(<UnitButton {...mockData} onClick={onClick} />, { wrapWithRouter: true });
102+
103+
fireEvent.click(screen.getByRole('tab'));
104+
105+
expect(onClick).toHaveBeenCalledWith(mockData.unitId);
106+
});
107+
108+
it('renders with no unitId and does not crash', async () => {
109+
render(<UnitButton unitId={undefined} onClick={jest.fn()} contentType="video" title="Unit" />, {
110+
wrapWithRouter: true,
111+
});
112+
113+
expect(screen.getByRole('tab')).toBeInTheDocument();
114+
});
115+
116+
it('prepends /preview to the unit path if in preview mode', () => {
117+
const onClick = jest.fn();
118+
119+
render(
120+
<UnitButton {...mockData} onClick={onClick} />,
121+
{
122+
wrapWithRouter: true,
123+
initialEntries: ['/preview/some/path'],
124+
},
125+
);
126+
127+
const button = screen.getByRole('tab');
128+
expect(button.closest('a')).toHaveAttribute('href', expect.stringContaining('/preview/course/'));
129+
});
85130
});

0 commit comments

Comments
 (0)