Skip to content

Commit

Permalink
refactor: after review
Browse files Browse the repository at this point in the history
  • Loading branch information
PKulkoRaccoonGang committed Oct 30, 2024
1 parent 12b9eac commit d5745e2
Show file tree
Hide file tree
Showing 17 changed files with 132 additions and 242 deletions.
3 changes: 2 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
"@openedx/paragon": ">= 21.5.7 < 23.0.0",
"prop-types": "^15.5.10",
"react": "^16.9.0 || ^17.0.0",
"react-dom": "^16.9.0 || ^17.0.0"
"react-dom": "^16.9.0 || ^17.0.0",
"react-router-dom": "^6.14.2"
}
}
10 changes: 3 additions & 7 deletions src/studio-header/BrandNav.jsx
Original file line number Diff line number Diff line change
@@ -1,29 +1,25 @@
import React from 'react';
import PropTypes from 'prop-types';
import { Hyperlink } from '@openedx/paragon';

import { navigateToUrl } from './utils';
import { Link } from 'react-router-dom';

const BrandNav = ({
studioBaseUrl,
logo,
logoAltText,
onNavigate,
}) => (
<Hyperlink destination={studioBaseUrl} onClick={(e) => navigateToUrl(e, studioBaseUrl, onNavigate)}>
<Link to={studioBaseUrl}>
<img
src={logo}
alt={logoAltText}
className="d-block logo"
/>
</Hyperlink>
</Link>
);

BrandNav.propTypes = {
studioBaseUrl: PropTypes.string.isRequired,
logo: PropTypes.string.isRequired,
logoAltText: PropTypes.string.isRequired,
onNavigate: PropTypes.func.isRequired,
};

export default BrandNav;
70 changes: 21 additions & 49 deletions src/studio-header/BrandNav.test.jsx
Original file line number Diff line number Diff line change
@@ -1,68 +1,40 @@
import React from 'react';
import { render, fireEvent, screen } from '@testing-library/react';
import { render, screen } from '@testing-library/react';
import '@testing-library/jest-dom/extend-expect';
import { MemoryRouter } from 'react-router-dom';

import BrandNav from './BrandNav';

describe('BrandNav Component', () => {
const mockOnNavigate = jest.fn();
const studioBaseUrl = 'https://example.com';
const logo = 'logo.png';
const logoAltText = 'Example Logo';
const studioBaseUrl = 'https://example.com/';
const logo = 'logo.png';
const logoAltText = 'Example Logo';

const RootWrapper = () => (
<MemoryRouter>
<BrandNav
studioBaseUrl={studioBaseUrl}
logo={logo}
logoAltText={logoAltText}
/>
</MemoryRouter>
);

describe('BrandNav Component', () => {
afterEach(() => {
jest.clearAllMocks();
});

it('renders the logo with the correct alt text', () => {
render(
<BrandNav
studioBaseUrl={studioBaseUrl}
logo={logo}
logoAltText={logoAltText}
onNavigate={mockOnNavigate}
/>,
);
render(<RootWrapper />);

const img = screen.getByAltText(logoAltText);
expect(img).toBeInTheDocument();
expect(img).toHaveAttribute('src', logo);
});

it('navigates to an absolute URL by changing window location', () => {
// Mock window.location.href
delete window.location;
window.location = { href: '' };

render(
<BrandNav
studioBaseUrl={studioBaseUrl}
logo={logo}
logoAltText={logoAltText}
onNavigate={mockOnNavigate}
/>,
);

const link = screen.getByRole('link');
fireEvent.click(link);

expect(window.location.href).toBe(studioBaseUrl);
});

it('calls onNavigate for relative URLs', () => {
const relativeUrl = '/studio/dashboard';

render(
<BrandNav
studioBaseUrl={relativeUrl}
logo={logo}
logoAltText={logoAltText}
onNavigate={mockOnNavigate}
/>,
);
it('displays a link that navigates to studioBaseUrl', () => {
render(<RootWrapper />);

const link = screen.getByRole('link');
fireEvent.click(link);

expect(mockOnNavigate).toHaveBeenCalledWith(relativeUrl);
expect(link.href).toBe(studioBaseUrl);
});
});
11 changes: 4 additions & 7 deletions src/studio-header/CourseLockUp.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,15 @@ import {
OverlayTrigger,
Tooltip,
} from '@openedx/paragon';
import { Link } from 'react-router-dom';

import { navigateToUrl } from './utils';
import messages from './messages';

const CourseLockUp = ({
outlineLink,
org,
number,
title,
onNavigate,
// injected
intl,
}) => (
Expand All @@ -26,16 +25,15 @@ const CourseLockUp = ({
</Tooltip>
)}
>
<a
<Link
className="course-title-lockup mr-2"
href={outlineLink}
onClick={(e) => navigateToUrl(e, outlineLink, onNavigate)}
to={outlineLink}
aria-label={intl.formatMessage(messages['header.label.courseOutline'])}
data-testid="course-lock-up-block"
>
<span className="d-block small m-0 text-gray-800" data-testid="course-org-number">{org} {number}</span>
<span className="d-block m-0 font-weight-bold text-gray-800" data-testid="course-title">{title}</span>
</a>
</Link>
</OverlayTrigger>
);

Expand All @@ -44,7 +42,6 @@ CourseLockUp.propTypes = {
org: PropTypes.string,
title: PropTypes.string,
outlineLink: PropTypes.string,
onNavigate: PropTypes.func.isRequired,
// injected
intl: intlShape.isRequired,
};
Expand Down
51 changes: 14 additions & 37 deletions src/studio-header/CourseLockUp.test.jsx
Original file line number Diff line number Diff line change
@@ -1,35 +1,34 @@
import React from 'react';
import { render, fireEvent, screen } from '@testing-library/react';
import { render, screen } from '@testing-library/react';
import '@testing-library/jest-dom/extend-expect';
import { IntlProvider } from '@edx/frontend-platform/i18n';
import { MemoryRouter } from 'react-router-dom';

import CourseLockUp from './CourseLockUp';
import messages from './messages';

const mockOnNavigate = jest.fn();
const mockProps = {
number: '101',
org: 'EDX',
title: 'Course Title',
outlineLink: 'https://example.com/course-outline',
onNavigate: mockOnNavigate,
};

const RootWrapper = (props) => (
<MemoryRouter>
<IntlProvider locale="en" messages={messages}>
<CourseLockUp {...props} />
</IntlProvider>
</MemoryRouter>
);

describe('CourseLockUp Component', () => {
afterEach(() => {
jest.clearAllMocks();
});

const renderWithIntl = (component) => (
render(
<IntlProvider locale="en" messages={messages}>
{component}
</IntlProvider>,
)
);

it('renders course org, number, and title', () => {
renderWithIntl(<CourseLockUp {...mockProps} />);
render(<RootWrapper {...mockProps} />);

const courseOrgNumber = screen.getByTestId('course-org-number');
const courseTitle = screen.getByTestId('course-title');
Expand All @@ -41,7 +40,7 @@ describe('CourseLockUp Component', () => {
});

it('renders the link with correct aria-label', () => {
renderWithIntl(<CourseLockUp {...mockProps} />);
render(<RootWrapper {...mockProps} />);

const link = screen.getByTestId('course-lock-up-block');
expect(link).toHaveAttribute(
Expand All @@ -51,31 +50,9 @@ describe('CourseLockUp Component', () => {
});

it('navigates to an absolute URL when clicked', () => {
// Mock window.location.href
delete window.location;
window.location = { href: '' };

renderWithIntl(<CourseLockUp {...mockProps} />);
render(<RootWrapper {...mockProps} />);

const link = screen.getByTestId('course-lock-up-block');
fireEvent.click(link);

expect(window.location.href).toBe(mockProps.outlineLink);
});

it('calls onNavigate for relative URLs', () => {
const relativeUrl = '/courses/edx/course-v1-101/';

renderWithIntl(
<CourseLockUp
{...mockProps}
outlineLink={relativeUrl}
/>,
);

const link = screen.getByTestId('course-lock-up-block');
fireEvent.click(link);

expect(mockOnNavigate).toHaveBeenCalledWith(relativeUrl);
expect(link.href).toBe(mockProps.outlineLink);
});
});
7 changes: 1 addition & 6 deletions src/studio-header/HeaderBody.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ const HeaderBody = ({
outlineLink,
searchButtonAction,
containerProps,
onNavigate,
}) => {
const intl = useIntl();

Expand All @@ -49,7 +48,6 @@ const HeaderBody = ({
studioBaseUrl,
logo,
logoAltText,
onNavigate,
}}
/>
);
Expand Down Expand Up @@ -90,7 +88,6 @@ const HeaderBody = ({
number,
org,
title,
onNavigate,
}}
/>
</Row>
Expand All @@ -109,7 +106,7 @@ const HeaderBody = ({
<NavDropdownMenu
key={id}
{...{
id, buttonTitle, items, onNavigate,
id, buttonTitle, items,
}}
/>
);
Expand Down Expand Up @@ -138,7 +135,6 @@ const HeaderBody = ({
logoutUrl,
authenticatedUserAvatar,
isAdmin,
onNavigate,
}}
/>
</Nav>
Expand Down Expand Up @@ -174,7 +170,6 @@ HeaderBody.propTypes = {
outlineLink: PropTypes.string,
searchButtonAction: PropTypes.func,
containerProps: PropTypes.shape(Container.propTypes),
onNavigate: PropTypes.func.isRequired,
onLogoNavigate: PropTypes.func.isRequired,
};

Expand Down
Loading

0 comments on commit d5745e2

Please sign in to comment.