Skip to content
Draft
Show file tree
Hide file tree
Changes from all 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
36 changes: 33 additions & 3 deletions src/learning-header/AuthenticatedUserDropdown.jsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, { useRef } from 'react';
import PropTypes from 'prop-types';

import { faUserCircle } from '@fortawesome/free-solid-svg-icons';
Expand All @@ -13,6 +13,31 @@ import messages from './messages';

const AuthenticatedUserDropdown = ({ username }) => {
const intl = useIntl();

const firstMenuItemRef = useRef(null);
const lastMenuItemRef = useRef(null);

const handleKeyDown = (event) => {
if (event.key === 'Tab') {
event.preventDefault();

const isShiftTab = event.shiftKey;
const currentElement = document.activeElement;
const focusElement = isShiftTab
? currentElement.previousElementSibling
: currentElement.nextElementSibling;

// If the element has reached the start or end of the list, loop the focus
if (isShiftTab && currentElement === firstMenuItemRef.current) {
lastMenuItemRef.current.focus();
} else if (!isShiftTab && currentElement === lastMenuItemRef.current) {
firstMenuItemRef.current.focus();
} else if (focusElement && focusElement.tagName === 'A') {
focusElement.focus();
}
}
};

const dropdownItems = [
{
message: intl.formatMessage(messages.dashboard),
Expand Down Expand Up @@ -41,8 +66,13 @@ const AuthenticatedUserDropdown = ({ username }) => {
<Dropdown.Toggle variant="outline-primary" aria-label={intl.formatMessage(messages.userOptionsDropdownLabel)}>
<LearningUserMenuToggleSlot label={username} icon={faUserCircle} />
</Dropdown.Toggle>
<Dropdown.Menu className="dropdown-menu-right">
<LearningUserMenuSlot items={dropdownItems} />
<Dropdown.Menu className="dropdown-menu-right" role="menu">
<LearningUserMenuSlot
items={dropdownItems}
firstMenuItemRef={firstMenuItemRef}
lastMenuItemRef={lastMenuItemRef}
handleKeyDown={handleKeyDown}
/>
</Dropdown.Menu>
</Dropdown>
);
Expand Down
91 changes: 91 additions & 0 deletions src/learning-header/AuthenticatedUserDropdown.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
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 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.

if we remove this somewhere, errors will occur

import AuthenticatedUserDropdown from './AuthenticatedUserDropdown';
import messages from './messages';
import {
render, screen, fireEvent, initializeMockApp,
} from '../setupTest';
Comment on lines +1 to +6
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 React from 'react';
import AuthenticatedUserDropdown from './AuthenticatedUserDropdown';
import messages from './messages';
import {
render, screen, fireEvent, initializeMockApp,
} from '../setupTest';
import React from 'react';
import {
render, screen, fireEvent, initializeMockApp,
} from '../setupTest';
import AuthenticatedUserDropdown from './AuthenticatedUserDropdown';
import messages from './messages';


describe('AuthenticatedUserDropdown', () => {
const username = 'testuser';

beforeEach(() => {
initializeMockApp();
});

const renderComponent = () => {
render(
<AuthenticatedUserDropdown username={username} />,
);
};

it('renders username in toggle button', () => {
renderComponent();

expect(screen.getByText(username)).toBeInTheDocument();
});

it('renders dropdown items after toggle click', async () => {
renderComponent();

const toggleButton = screen.getByRole('button', { name: messages.userOptionsDropdownLabel.defaultMessage });
await fireEvent.click(toggleButton);
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use userEvent

Copy link
Author

Choose a reason for hiding this comment

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

userEvent package does not use in this header-component


expect(await screen.findByText(messages.dashboard.defaultMessage))
.toHaveAttribute('href', `${process.env.LMS_BASE_URL}/dashboard`);
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
.toHaveAttribute('href', `${process.env.LMS_BASE_URL}/dashboard`);
.toHaveAttribute('href', `${getConfig().LMS_BASE_URL}/dashboard`);


expect(screen.getByText(messages.profile.defaultMessage)).toHaveAttribute('href', `${process.env.ACCOUNT_PROFILE_URL}/u/${username}`);
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
expect(screen.getByText(messages.profile.defaultMessage)).toHaveAttribute('href', `${process.env.ACCOUNT_PROFILE_URL}/u/${username}`);
expect(screen.getByText(messages.profile.defaultMessage)).toHaveAttribute('href', `${getConfig().ACCOUNT_PROFILE_URL}/u/${username}`);

expect(screen.getByText(messages.account.defaultMessage)).toHaveAttribute('href', process.env.ACCOUNT_SETTINGS_URL);
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
expect(screen.getByText(messages.account.defaultMessage)).toHaveAttribute('href', process.env.ACCOUNT_SETTINGS_URL);
expect(screen.getByText(messages.account.defaultMessage)).toHaveAttribute('href', getConfig().ACCOUNT_SETTINGS_URL);

expect(screen.getByText(messages.orderHistory.defaultMessage)).toHaveAttribute('href', process.env.ORDER_HISTORY_URL);
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
expect(screen.getByText(messages.orderHistory.defaultMessage)).toHaveAttribute('href', process.env.ORDER_HISTORY_URL);
expect(screen.getByText(messages.orderHistory.defaultMessage)).toHaveAttribute('href', getConfig().ORDER_HISTORY_URL);

expect(screen.getByText(messages.signOut.defaultMessage)).toHaveAttribute('href', process.env.LOGOUT_URL);
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
expect(screen.getByText(messages.signOut.defaultMessage)).toHaveAttribute('href', process.env.LOGOUT_URL);
expect(screen.getByText(messages.signOut.defaultMessage)).toHaveAttribute('href', getConfig().LOGOUT_URL);

});

it('loops focus from last to first and vice versa with Tab and Shift+Tab', async () => {
renderComponent();

const toggleButton = screen.getByRole('button', { name: 'User Options' });
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use translations instead of hardcoded text?

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.

await fireEvent.click(toggleButton);
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use userEvent

Copy link
Author

Choose a reason for hiding this comment

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

userEvent package does not use in this header-component


const menuItems = await screen.findAllByRole('menuitem');
const firstItem = menuItems[0];
const lastItem = menuItems[menuItems.length - 1];

lastItem.focus();
expect(lastItem).toHaveFocus();

fireEvent.keyDown(lastItem, { key: 'Tab' });
expect(firstItem).toHaveFocus();

firstItem.focus();
expect(firstItem).toHaveFocus();

fireEvent.keyDown(firstItem, { key: 'Tab', shiftKey: true });
expect(lastItem).toHaveFocus();
});

it('focuses next link when Tab is pressed on middle item', async () => {
renderComponent();

const toggleButton = screen.getByRole('button', { name: 'User Options' });
Copy link
Contributor

Choose a reason for hiding this comment

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

await fireEvent.click(toggleButton);
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use userEvent

Copy link
Author

Choose a reason for hiding this comment

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

userEvent package does not use in this header-component


const menuItems = await screen.findAllByRole('menuitem');
const secondItem = menuItems[1];
const thirdItem = menuItems[2];

secondItem.focus();
expect(secondItem).toHaveFocus();

Object.defineProperty(secondItem, 'nextElementSibling', {
value: thirdItem,
configurable: true,
});
Object.defineProperty(thirdItem, 'tagName', {
value: 'A',
configurable: true,
});

fireEvent.keyDown(secondItem, { key: 'Tab' });

expect(thirdItem).toHaveFocus();
});
});
29 changes: 24 additions & 5 deletions src/learning-header/LearningHeaderUserMenuItems.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,30 @@ import PropTypes from 'prop-types';

import { Dropdown } from '@openedx/paragon';

const LearningHeaderUserMenuItems = ({ items }) => items.map((item) => (
<Dropdown.Item href={item.href}>
{item.message}
</Dropdown.Item>
));
const LearningHeaderUserMenuItems = ({
items,
handleKeyDown,
firstMenuItemRef,
lastMenuItemRef,
}) => {
const getRefForIndex = (index, length) => {
if (index === 0) { return firstMenuItemRef; }
if (index === length - 1) { return lastMenuItemRef; }
return null;
};
Comment on lines +12 to +16
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
const getRefForIndex = (index, length) => {
if (index === 0) { return firstMenuItemRef; }
if (index === length - 1) { return lastMenuItemRef; }
return null;
};
const getRefForIndex = (index, length) => {
if (index === 0) {
return firstMenuItemRef;
}
if (index === length - 1) {
return lastMenuItemRef;
}
return null;
};


return items.map((item, index) => (
<Dropdown.Item
key={item.href}
href={item.href}
role="menuitem"
onKeyDown={handleKeyDown}
ref={getRefForIndex(index, items.length)}
>
{item.message}
</Dropdown.Item>
));
};

export const learningHeaderUserMenuDataShape = {
items: PropTypes.arrayOf(PropTypes.shape({
Expand Down
10 changes: 9 additions & 1 deletion src/plugin-slots/LearningUserMenuSlot/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ import LearningHeaderUserMenuItems, { learningHeaderUserMenuDataShape } from '..

const LearningUserMenuSlot = ({
items,
handleKeyDown,
firstMenuItemRef,
lastMenuItemRef,
}) => (
<PluginSlot
id="org.openedx.frontend.layout.header_learning_user_menu.v1"
Expand All @@ -12,7 +15,12 @@ const LearningUserMenuSlot = ({
mergeProps: true,
}}
>
<LearningHeaderUserMenuItems items={items} />
<LearningHeaderUserMenuItems
items={items}
handleKeyDown={handleKeyDown}
firstMenuItemRef={firstMenuItemRef}
lastMenuItemRef={lastMenuItemRef}
/>
</PluginSlot>
);

Expand Down
5 changes: 5 additions & 0 deletions src/setupTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ export function initializeMockApp() {
CSRF_TOKEN_API_PATH: process.env.CSRF_TOKEN_API_PATH || null,
LOGO_URL: process.env.LOGO_URL || null,
SITE_NAME: process.env.SITE_NAME || null,
ACCOUNT_PROFILE_URL: process.env.ACCOUNT_PROFILE_URL || null,
ACCOUNT_SETTINGS_URL: process.env.ACCOUNT_SETTINGS_URL || null,
ORDER_HISTORY_URL: process.env.ORDER_HISTORY_URL || null,
ECOMMERCE_BASE_URL: process.env.ECOMMERCE_BASE_URL || null,
CREDENTIALS_BASE_URL: process.env.CREDENTIALS_BASE_URL || null,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we adding these variables?

Copy link
Author

@filippovskii09 filippovskii09 Nov 9, 2025

Choose a reason for hiding this comment

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

becouse we used this vars there AuthenticatedUserDropdown.test.jsx and need to mock it


authenticatedUser: {
userId: 'abc123',
Expand Down