Skip to content
Draft
Show file tree
Hide file tree
Changes from 3 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
77 changes: 77 additions & 0 deletions src/course-home/live-tab/LiveTab.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import React from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this import?

Copy link
Author

Choose a reason for hiding this comment

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

removed

import { render } from '@testing-library/react';
Copy link
Contributor

Choose a reason for hiding this comment

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

[question]: Is it possible to use function render from file setupTests https://github.com/openedx/frontend-app-learning/blob/master/src/setupTest.js#L222 in these tests?

Copy link
Author

Choose a reason for hiding this comment

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

yes, fixed, i used render from setupTest

import { useSelector } from 'react-redux';
import LiveTab from './LiveTab';

jest.mock('react-redux', () => ({
useSelector: jest.fn(),
}));

describe('LiveTab', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why we are adding these tests to this PR?
It looks like we are proposing accessibility improvements in this PR, but we are writing tests for the functionality of the LiveTab component.

Copy link
Author

@filippovskii09 filippovskii09 Oct 24, 2025

Choose a reason for hiding this comment

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

These tests were cherry-picked along with the related changes from a previously closed PR.
The tests themselves are not directly related to the accessibility improvements.
I can separate them if needed, but for now, they ensure we don’t lose existing coverage.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but these tests seem to test functionality that is not relevant to this PR. Will we have coverage issues if we remove them?

Copy link
Author

Choose a reason for hiding this comment

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

deleted test and no problem occurred

afterEach(() => {
jest.clearAllMocks();
document.body.innerHTML = '';
});

it('renders iframe from liveModel using dangerouslySetInnerHTML', () => {
useSelector.mockImplementation((selector) => selector({
courseHome: { courseId: 'course-v1:test+id+2024' },
models: {
live: {
'course-v1:test+id+2024': {
iframe: '<iframe id="lti-tab-embed" src="about:blank"></iframe>',
},
},
},
}));

render(<LiveTab />);

const iframe = document.getElementById('lti-tab-embed');
expect(iframe).toBeInTheDocument();
expect(iframe.src).toBe('about:blank');
});

it('adds classes to iframe after mount', () => {
document.body.innerHTML = `
<div id="live_tab">
<iframe id="lti-tab-embed" class=""></iframe>
</div>
`;

useSelector.mockImplementation((selector) => selector({
courseHome: { courseId: 'course-v1:test+id+2024' },
models: {
live: {
'course-v1:test+id+2024': {
iframe: '<iframe id="lti-tab-embed"></iframe>',
},
},
},
}));

render(<LiveTab />);

const iframe = document.getElementById('lti-tab-embed');
expect(iframe.className).toContain('vh-100');
expect(iframe.className).toContain('w-100');
expect(iframe.className).toContain('border-0');
});

it('does not throw if iframe is not found in DOM', () => {
useSelector.mockImplementation((selector) => selector({
courseHome: { courseId: 'course-v1:test+id+2024' },
models: {
live: {
'course-v1:test+id+2024': {
iframe: '<div>No iframe here</div>',
},
},
},
}));

expect(() => render(<LiveTab />)).not.toThrow();
const iframe = document.getElementById('lti-tab-embed');
expect(iframe).toBeNull();
});
});
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import React, { useState } from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this React import?

Copy link
Author

Choose a reason for hiding this comment

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

no, removed

import PropTypes from 'prop-types';
import { useIntl } from '@edx/frontend-platform/i18n';
import {
Expand All @@ -6,10 +7,11 @@ import {
OverlayTrigger,
Stack,
Tooltip,
IconButton,
} from '@openedx/paragon';
import { InfoOutline, Locked } from '@openedx/paragon/icons';
import { useContextId } from '../../../../data/hooks';

import { useContextId } from '../../../../data/hooks';
import messages from '../messages';
import { useModel } from '../../../../generic/model-store';

Expand All @@ -20,24 +22,36 @@ const GradeSummaryHeader = ({ allOfSomeAssignmentTypeIsLocked }) => {
verifiedMode,
gradesFeatureIsFullyLocked,
} = useModel('progress', courseId);
const [showTooltip, setShowTooltip] = useState(false);

const handleKeyDown = (event) => {
if (event.key === 'Escape') {
setShowTooltip(false);
}
};

return (
<Stack gap={2} className="mb-3">
<Stack direction="horizontal" gap={2}>
<h3 className="h4 m-0">{intl.formatMessage(messages.gradeSummary)}</h3>
<OverlayTrigger
trigger="hover"
trigger="click"
placement="top"
show={showTooltip}
overlay={(
<Tooltip>
{intl.formatMessage(messages.gradeSummaryTooltipBody)}
</Tooltip>
)}
>
<Icon
<IconButton
onClick={() => { setShowTooltip(!showTooltip); }}
onBlur={() => { setShowTooltip(false); }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
onClick={() => { setShowTooltip(!showTooltip); }}
onBlur={() => { setShowTooltip(false); }}
onClick={() => setShowTooltip(!showTooltip)}
onBlur={() => setShowTooltip(false)}

Copy link
Author

Choose a reason for hiding this comment

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

fixed

onKeyDown={handleKeyDown}
alt={intl.formatMessage(messages.gradeSummaryTooltipAlt)}
src={InfoOutline}
iconAs={InfoOutline}
size="sm"
disabled={gradesFeatureIsFullyLocked}
/>
</OverlayTrigger>
</Stack>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import React from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this import?

Copy link
Author

Choose a reason for hiding this comment

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

no, removed

import { render, screen, waitFor } from '@testing-library/react';
Copy link
Contributor

Choose a reason for hiding this comment

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

[question]: Is it possible to use function render from file setupTests https://github.com/openedx/frontend-app-learning/blob/master/src/setupTest.js#L222 in these tests?

Copy link
Author

Choose a reason for hiding this comment

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

yes, fixed, i used render from setupTest

import userEvent from '@testing-library/user-event';
import { useSelector } from 'react-redux';
import { IntlProvider } from 'react-intl';

import { fireEvent } from '@testing-library/dom';
import GradeSummaryHeader from './GradeSummaryHeader';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import { IntlProvider } from 'react-intl';
import { fireEvent } from '@testing-library/dom';
import GradeSummaryHeader from './GradeSummaryHeader';
import { IntlProvider } from 'react-intl';
import { fireEvent } from '@testing-library/dom';
import GradeSummaryHeader from './GradeSummaryHeader';

Copy link
Author

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

Resolved

import { useModel } from '../../../../generic/model-store';
import messages from '../messages';

jest.mock('react-redux', () => ({
useSelector: jest.fn(),
}));

jest.mock('../../../../generic/model-store', () => ({
useModel: jest.fn(),
}));

jest.mock('../../../../data/hooks', () => ({
useContextId: () => 'test-course-id',
}));

describe('GradeSummaryHeader', () => {
beforeEach(() => {
useSelector.mockImplementation((selector) => selector({
courseHome: { courseId: 'test-course-id' },
}));
useModel.mockReturnValue({ gradesFeatureIsFullyLocked: false });
});

const renderComponent = (props = {}) => {
render(
<IntlProvider locale="en" messages={messages}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this extra IntlProvider?

Copy link
Author

Choose a reason for hiding this comment

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

no, removed

<GradeSummaryHeader
allOfSomeAssignmentTypeIsLocked={false}
{...props}
/>
</IntlProvider>,
);
};

it('shows tooltip on icon button click', async () => {
renderComponent();

const iconButton = screen.getByRole('button', {
name: messages.gradeSummaryTooltipAlt.defaultMessage,
});

await userEvent.click(iconButton);

await waitFor(() => {
expect(screen.getByText(messages.gradeSummaryTooltipBody.defaultMessage)).toBeInTheDocument();
});
});

it('hides tooltip on click', async () => {
renderComponent();

const iconButton = screen.getByRole('button', {
name: messages.gradeSummaryTooltipAlt.defaultMessage,
});

fireEvent.click(iconButton);
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

fixed


await waitFor(() => {
expect(screen.getByText(messages.gradeSummaryTooltipBody.defaultMessage)).toBeVisible();
});

fireEvent.click(iconButton);
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

fixed


await waitFor(() => {
expect(screen.queryByText(messages.gradeSummaryTooltipBody.defaultMessage)).toBeNull();
});
});

it('hides tooltip on blur', async () => {
renderComponent();

const iconButton = screen.getByRole('button', {
name: messages.gradeSummaryTooltipAlt.defaultMessage,
});

await userEvent.hover(iconButton);
await userEvent.click(iconButton);

await waitFor(() => {
expect(screen.getByText(messages.gradeSummaryTooltipBody.defaultMessage)).toBeInTheDocument();
});

const blurTarget = document.createElement('button');
blurTarget.textContent = 'Outside';
document.body.appendChild(blurTarget);
blurTarget.focus();

await userEvent.unhover(iconButton);

await waitFor(() => {
expect(screen.queryByText(messages.gradeSummaryTooltipBody.defaultMessage)).not.toBeInTheDocument();
});

document.body.removeChild(blurTarget);
});

it('hides tooltip when Escape is pressed (covers handleKeyDown)', async () => {
renderComponent();

const iconButton = screen.getByRole('button', {
name: messages.gradeSummaryTooltipAlt.defaultMessage,
});

await userEvent.hover(iconButton);
await userEvent.click(iconButton);

await waitFor(() => {
expect(screen.getByText(messages.gradeSummaryTooltipBody.defaultMessage)).toBeInTheDocument();
});

fireEvent.keyDown(iconButton, { key: 'Escape', code: 'Escape' });
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

fixed


await userEvent.unhover(iconButton);

await waitFor(() => {
expect(screen.queryByText(messages.gradeSummaryTooltipBody.defaultMessage)).not.toBeInTheDocument();
});
});
});