Skip to content

Commit

Permalink
fix: address reviewer comments
Browse files Browse the repository at this point in the history
  • Loading branch information
mubbsharanwar committed Feb 7, 2024
1 parent 4cc31ca commit c5935e5
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 15 deletions.
4 changes: 2 additions & 2 deletions src/DesktopHeader.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class DesktopHeader extends React.Component {
<Menu transitionClassName="menu-dropdown" transitionTimeout={250}>
<MenuTrigger
tag="button"
aria-label={intl.formatMessage(messages['header.label.account.menu.with.username'], { username })}
aria-label={intl.formatMessage(messages['header.label.account.menu.using.username'], { username })}
className="btn btn-outline-primary d-inline-flex align-items-center pl-2 pr-3"
>
<Avatar size="1.5em" src={avatar} alt="" className="mr-2" />
Expand All @@ -58,7 +58,7 @@ class DesktopHeader extends React.Component {
<Menu transitionClassName="menu-dropdown" transitionTimeout={250}>
<MenuTrigger
tag="button"
aria-label={intl.formatMessage(messages['header.label.account.menu.without.username'], { name })}
aria-label={intl.formatMessage(messages['header.label.account.menu.using.name'], { name })}
className="btn btn-outline-primary d-inline-flex align-items-center pl-2 pr-3"
>
<Avatar size="1.5em" src={avatar} alt="" className="mr-2" />
Expand Down
8 changes: 4 additions & 4 deletions src/Header.messages.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,13 @@ const messages = defineMessages({
defaultMessage: 'Account Menu',
description: 'The aria label for the account menu trigger',
},
'header.label.account.menu.with.username': {
id: 'header.label.account.menu.with.username',
'header.label.account.menu.using.username': {
id: 'header.label.account.menu.using.username',
defaultMessage: 'Account menu for {username}',
description: 'The aria label for the account menu trigger when the username is displayed in it',
},
'header.label.account.menu.without.username': {
id: 'header.label.account.without.username',
'header.label.account.menu.using.name': {
id: 'header.label.account.using.name',
defaultMessage: 'Account menu for {name}',
description: 'The aria label for the account menu trigger when ENABLE_HEADER_WITHOUT_USERNAME is enabled',
},
Expand Down
8 changes: 5 additions & 3 deletions src/learning-header/AuthenticatedUserDropdown.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { Dropdown } from '@openedx/paragon';
import messages from './messages';

const AuthenticatedUserDropdown = ({
intl, username, avatar, name
intl, username, avatar, name,
}) => {
const dashboardMenuItem = (
<Dropdown.Item href={`${getConfig().LMS_BASE_URL}/dashboard`}>
Expand All @@ -22,11 +22,13 @@ const AuthenticatedUserDropdown = ({
const showDropdownToggle = (
<Dropdown.Toggle variant="outline-primary">
<FontAwesomeIcon icon={faUserCircle} className="d-md-none" size="lg" />
{!getConfig().ENABLE_HEADER_WITHOUT_USERNAME ? (
{getConfig().ENABLE_HEADER_WITHOUT_USERNAME ? (
<Avatar size="sm" src={avatar} alt={name} className="mr-2" />
) : (
<span data-hj-suppress className="d-none d-md-inline" data-testid="username">
{username}
</span>
) : <Avatar size="sm" src={avatar} alt={name} className="mr-2" />}
)}
</Dropdown.Toggle>
);

Expand Down
11 changes: 8 additions & 3 deletions src/studio-header/StudioHeader.test.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint-disable react/prop-types */
import React, { useMemo } from 'react';
import { getConfig, mergeConfig } from '@edx/frontend-platform';
import {
render,
fireEvent,
Expand Down Expand Up @@ -128,9 +129,13 @@ describe('Header', () => {
expect(avatarIcon).toBeVisible();
});

it.only('user menu should not contain username', async () => {
const newProps = { ...props, ENABLE_HEADER_WITHOUT_USERNAME: true };
const { container } = render(<RootWrapper {...newProps} />);
it('user menu should not contain username', async () => {
const config = getConfig();
mergeConfig({
...config,
ENABLE_HEADER_WITHOUT_USERNAME: true,
});
const { container } = render(<RootWrapper {...props} />);
const userMenue = container.querySelector('#user-dropdown-menu');
expect(userMenue.textContent).toContain('');
});
Expand Down
6 changes: 3 additions & 3 deletions src/studio-header/UserMenu.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ const UserMenu = ({
// injected
intl,
}) => {
const showUsername = !getConfig().ENABLE_HEADER_WITHOUT_USERNAME;
const avatarAlt = showUsername ? username : name;
const hideUsername = getConfig().ENABLE_HEADER_WITHOUT_USERNAME;
const avatarAlt = hideUsername ? name : username;
const avatar = authenticatedUserAvatar ? (
<img
className="d-block w-100 h-100"
Expand All @@ -36,7 +36,7 @@ const UserMenu = ({
data-testid="avatar-icon"
/>
);
const title = isMobile ? avatar : <>{avatar}{showUsername && username}</>;
const title = (isMobile || hideUsername) ? avatar : <>{avatar}{username}</>;

return (
<NavDropdownMenu
Expand Down

0 comments on commit c5935e5

Please sign in to comment.