Skip to content

Commit 2461187

Browse files
payamnjpayamnj
authored
Merge pull request #538 from AvaCodeSolutions/feat/537-sidebar-sections
feat: #537 add sidebar section labels/dividers and remove Settings accordion
2 parents 0230bcf + c6d88f3 commit 2461187

2 files changed

Lines changed: 90 additions & 132 deletions

File tree

‎frontend/src/components/MenuBar.jsx‎

Lines changed: 59 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { useState, useEffect } from 'react'
2-
import { AppBar, Chip, Drawer, Box, Typography, MenuList, MenuItem, ListItemIcon, ListItemText, Tooltip, Link, Select, Stack, Collapse } from '@mui/material'
2+
import { AppBar, Chip, Divider, Drawer, Box, Typography, MenuList, MenuItem, ListItemIcon, ListItemText, Tooltip, Link, Select, Stack } from '@mui/material'
33
import IconButton from '@mui/material/IconButton';
44
import SchoolOutlinedIcon from '@mui/icons-material/SchoolOutlined';
55
import PeopleOutlinedIcon from '@mui/icons-material/PeopleOutlined';
@@ -8,9 +8,6 @@ import WarningIcon from '@mui/icons-material/Warning';
88
import ErrorIcon from '@mui/icons-material/Error';
99
import VpnKeyOutlinedIcon from '@mui/icons-material/VpnKeyOutlined';
1010
import CorporateFareOutlinedIcon from '@mui/icons-material/CorporateFareOutlined';
11-
import SettingsOutlinedIcon from '@mui/icons-material/SettingsOutlined';
12-
import ExpandLess from '@mui/icons-material/ExpandLess';
13-
import ExpandMore from '@mui/icons-material/ExpandMore';
1411
import MenuIcon from '@mui/icons-material/Menu';
1512
import logoHorizontalLightUrl from '../assets/logo-h-light.png'
1613
import logoHorizontalDarkUrl from '../assets/logo-h-dark.png'
@@ -52,6 +49,29 @@ function OrganizationsSelect({organizations, activeOrganizationId, changeOrganiz
5249
)
5350
}
5451

52+
function NavItem({ page, isActive }) {
53+
return (
54+
<MenuItem sx={(theme) => ({
55+
backgroundColor: isActive ? (theme.palette.mode === 'dark' ? theme.palette.deepPurple[800] : theme.palette.deepPurple[50]) : 'transparent',
56+
'& .MuiTouchRipple-root': { color: theme.palette.secondary.main },
57+
padding: 0,
58+
'&:hover .MuiListItemIcon-root': { color: theme.palette.primary.dark },
59+
})}>
60+
<Link
61+
href={page.href}
62+
underline="none"
63+
color="inherit"
64+
sx={{ display: 'flex', alignItems: 'center', width: '100%', py: '8px', px: '16px' }}
65+
>
66+
<ListItemIcon sx={(theme) => ({ minWidth: 35, color: theme.palette.mode === 'dark' ? theme.palette.deepPurple[300] : theme.palette.deepPurple[500] })}>
67+
{page.icon}
68+
</ListItemIcon>
69+
<ListItemText primary={page.name} slotProps={{ primary: { fontSize: '0.95rem' } }} />
70+
</Link>
71+
</MenuItem>
72+
);
73+
}
74+
5575
function MenuBar({activeOrganizationId, changeOrganizationCallback, showOrganizationSwitcher, drawerWidth}) {
5676
const [menuOpen, setMenuOpen] = useState(false)
5777
const [organizations, setOrganizations] = useState([])
@@ -63,7 +83,6 @@ function MenuBar({activeOrganizationId, changeOrganizationCallback, showOrganiza
6383

6484
const drawerVariant = isMdUpScreen ? "permanent" : "temporary";
6585
const currentPath = typeof window !== 'undefined' ? window.location.pathname.replace(/\/+$/, '') || '/' : '/';
66-
const [settingsOpen, setSettingsOpen] = useState(() => /\/settings(\/|$)/.test(currentPath));
6786
let logoHorizontalUrl, logoVerticalUrl;
6887
if (customLogo) {
6988
logoHorizontalUrl = theme.palette.mode === 'light' ? (customLogo.horizontalLight ? customLogo.horizontalLight : customLogo.horizontalDark) : (customLogo.horizontalDark ? customLogo.horizontalDark : customLogo.horizontalLight);
@@ -128,20 +147,20 @@ function MenuBar({activeOrganizationId, changeOrganizationCallback, showOrganiza
128147
});
129148
}, []);
130149

131-
let pages = []
132-
150+
const adminPages = []
133151
if (isOrganizationAdmin) {
134-
pages.push(
135-
{ name: localeMessages["organizations"], icon: <CorporateFareOutlinedIcon fontSize="small" />, href: platformBaseUrl + '/organizations/'},
152+
adminPages.push(
153+
{ name: localeMessages["organizations"], icon: <CorporateFareOutlinedIcon fontSize="small" />, href: platformBaseUrl + '/organizations/' },
136154
);
137155
}
138156

139-
pages.push({ name: localeMessages["course_management"], icon: <SchoolOutlinedIcon fontSize="small" />, href: platformBaseUrl + '/courses/' });
157+
const platformPages = []
158+
platformPages.push({ name: localeMessages["course_management"], icon: <SchoolOutlinedIcon fontSize="small" />, href: platformBaseUrl + '/courses/' });
140159
if (isOrganizationAdmin || isPlatformAdmin || isInstructor) {
141-
pages.push({ name: localeMessages["learners"], icon: <PeopleOutlinedIcon fontSize="small" />, href: platformBaseUrl + '/learners/' });
160+
platformPages.push({ name: localeMessages["learners"], icon: <PeopleOutlinedIcon fontSize="small" />, href: platformBaseUrl + '/learners/' });
142161
}
143-
// pages.push({ name: 'Analytics', icon: <BarChartIcon fontSize="small" />, href: platformBaseUrl + '/analytics/' });
144-
let settingsPages = []
162+
163+
const settingsPages = []
145164
if (isPlatformAdmin) {
146165
settingsPages.push({ name: localeMessages["api_keys"], icon: <VpnKeyOutlinedIcon fontSize="small" />, href: platformBaseUrl + '/settings/api_keys' });
147166
}
@@ -159,8 +178,6 @@ function MenuBar({activeOrganizationId, changeOrganizationCallback, showOrganiza
159178
return currentPath === pagePath || (pagePath !== '/' && currentPath.startsWith(`${pagePath}/`));
160179
};
161180

162-
const isSettingsSectionActive = settingsPages.some((page) => isActivePage(page.href));
163-
164181
const healthStatus = deliverContentsJobStatus?.job_health_status || 'healthy';
165182
const statusConfig = jobsStatusMap[healthStatus] || jobsStatusMap.healthy;
166183
const executionTime = deliverContentsJobStatus?.last_execution_started_at
@@ -239,112 +256,33 @@ function MenuBar({activeOrganizationId, changeOrganizationCallback, showOrganiza
239256
showOrganizationSwitcher && <OrganizationsSelect organizations={organizations} activeOrganizationId={activeOrganizationId} changeOrganizationCallback={changeOrganizationCallback} sx={{ m: 2 }} />
240257
}
241258
<MenuList>
242-
{ pages.map((page) => {
243-
const isActive = isActivePage(page.href);
244-
return (
245-
<MenuItem key={page.name} sx={(theme) => ({
246-
backgroundColor: isActive ? (theme.palette.mode === 'dark' ? theme.palette.deepPurple[800] : theme.palette.deepPurple[50]) : 'transparent',
247-
'& .MuiTouchRipple-root': {
248-
color: theme.palette.secondary.main,
249-
},
250-
padding: 0,
251-
'&:hover .MuiListItemIcon-root': { color: theme.palette.primary.dark }})}>
252-
<Link
253-
href={page.href}
254-
underline="none"
255-
color="inherit"
256-
sx={{
257-
display: 'flex',
258-
alignItems: 'center',
259-
width: '100%',
260-
py: '8px',
261-
px: '16px',
262-
}}
263-
>
264-
<ListItemIcon sx={(theme) => ({ minWidth: 35, color: theme.palette.mode === 'dark' ? theme.palette.deepPurple[300] : theme.palette.deepPurple[500] })}>
265-
{page.icon}
266-
</ListItemIcon>
267-
<ListItemText
268-
primary={page.name}
269-
slotProps={{
270-
primary: {
271-
fontSize: '0.95rem',
272-
},
273-
}}
274-
/>
275-
</Link>
276-
</MenuItem>
277-
)}) }
278-
{settingsPages.length > 0 && [
279-
<MenuItem
280-
key="settings-toggle"
281-
onClick={() => setSettingsOpen(!settingsOpen)}
282-
sx={(theme) => ({
283-
backgroundColor: isSettingsSectionActive ? (theme.palette.mode === 'dark' ? theme.palette.deepPurple[800] : theme.palette.deepPurple[50]) : 'transparent',
284-
'& .MuiTouchRipple-root': {
285-
color: theme.palette.secondary.main,
286-
},
287-
'&:hover .MuiListItemIcon-root': { color: theme.palette.primary.dark },
288-
})}
289-
>
290-
<ListItemIcon sx={(theme) => ({ minWidth: 35, color: theme.palette.mode === 'dark' ? theme.palette.deepPurple[300] : theme.palette.deepPurple[500] })}>
291-
<SettingsOutlinedIcon fontSize="small" />
292-
</ListItemIcon>
293-
<ListItemText
294-
primary={localeMessages["settings"] || 'Settings'}
295-
slotProps={{
296-
primary: {
297-
fontSize: '0.95rem',
298-
},
299-
}}
300-
/>
301-
{settingsOpen ? <ExpandLess /> : <ExpandMore />}
302-
</MenuItem>,
303-
<Collapse key="settings-collapse" in={settingsOpen} timeout="auto" unmountOnExit>
304-
<MenuList disablePadding>
305-
{settingsPages.map((page) => {
306-
const isActive = isActivePage(page.href);
307-
return (
308-
<MenuItem
309-
key={page.name}
310-
sx={(theme) => ({
311-
pl: 4,
312-
backgroundColor: isActive ? (theme.palette.mode === 'dark' ? theme.palette.deepPurple[800] : theme.palette.deepPurple[50]) : 'transparent',
313-
'& .MuiTouchRipple-root': {
314-
color: theme.palette.secondary.main,
315-
},
316-
'&:hover .MuiListItemIcon-root': { color: theme.palette.primary.dark },
317-
})}
318-
>
319-
<Link
320-
href={page.href}
321-
underline="none"
322-
color="inherit"
323-
sx={{
324-
display: 'flex',
325-
alignItems: 'center',
326-
width: '100%',
327-
py: 0.1,
328-
}}
329-
>
330-
<ListItemIcon sx={(theme) => ({ minWidth: 35, color: theme.palette.mode === 'dark' ? theme.palette.deepPurple[300] : theme.palette.deepPurple[500] })}>
331-
{page.icon}
332-
</ListItemIcon>
333-
<ListItemText
334-
primary={page.name}
335-
slotProps={{
336-
primary: {
337-
fontSize: '0.95rem',
338-
},
339-
}}
340-
/>
341-
</Link>
342-
</MenuItem>
343-
);
344-
})}
345-
</MenuList>
346-
</Collapse>
347-
]}
259+
{/* ── Administration ── (org admin only) */}
260+
{adminPages.length > 0 && <>
261+
<Divider textAlign="left" sx={{ mt: 1, mb: 0.5 }}>
262+
<Typography variant="caption" sx={{ color: 'text.disabled', textTransform: 'uppercase', letterSpacing: '0.08em', fontSize: '0.7rem' }}>
263+
{localeMessages['administration'] || 'Administration'}
264+
</Typography>
265+
</Divider>
266+
{adminPages.map((page) => <NavItem key={page.name} page={page} isActive={isActivePage(page.href)} />)}
267+
</>}
268+
269+
{/* ── Platform ── */}
270+
<Divider textAlign="left" sx={{ mt: 1, mb: 0.5 }}>
271+
<Typography variant="caption" sx={{ color: 'text.disabled', textTransform: 'uppercase', letterSpacing: '0.08em', fontSize: '0.7rem' }}>
272+
{localeMessages['platform_section'] || 'Platform'}
273+
</Typography>
274+
</Divider>
275+
{platformPages.map((page) => <NavItem key={page.name} page={page} isActive={isActivePage(page.href)} />)}
276+
277+
{/* ── Settings ── (platform admin only, always expanded) */}
278+
{settingsPages.length > 0 && <>
279+
<Divider textAlign="left" sx={{ mt: 1, mb: 0.5 }}>
280+
<Typography variant="caption" sx={{ color: 'text.disabled', textTransform: 'uppercase', letterSpacing: '0.08em', fontSize: '0.7rem' }}>
281+
{localeMessages['settings'] || 'Settings'}
282+
</Typography>
283+
</Divider>
284+
{settingsPages.map((page) => <NavItem key={page.name} page={page} isActive={isActivePage(page.href)} />)}
285+
</>}
348286
</MenuList>
349287
{sidebarCustomComponent && <Box sx={{ height: "100px", width: "100%" }} dangerouslySetInnerHTML={{ __html: sidebarCustomComponent.componentTag }} />}
350288
</Drawer>

‎frontend/src/test/MenuBar.test.jsx‎

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { describe, it, expect, vi, beforeEach } from 'vitest';
22
import { screen, waitFor } from '@testing-library/react';
3-
import userEvent from '@testing-library/user-event';
43
import { renderWithProviders } from './test-utils';
54
import MenuBar from '../components/MenuBar';
65

@@ -100,13 +99,43 @@ describe('MenuBar', () => {
10099
expect(screen.getByText('Learners')).toBeInTheDocument();
101100
});
102101

103-
it('shows Settings menu item with API Keys for platform admin', () => {
102+
it('shows Settings section label for platform admin', () => {
104103
renderWithProviders(<MenuBar {...defaultProps} />, {
105104
appContext: { isPlatformAdmin: true },
106105
});
107106
expect(screen.getByText('Settings')).toBeInTheDocument();
108107
});
109108

109+
it('shows API Keys immediately without needing a click for platform admin', () => {
110+
renderWithProviders(<MenuBar {...defaultProps} />, {
111+
appContext: { isPlatformAdmin: true },
112+
});
113+
expect(screen.getByText('API Keys')).toBeInTheDocument();
114+
});
115+
116+
it('does not show Settings section for non-platform-admin', () => {
117+
renderWithProviders(<MenuBar {...defaultProps} />);
118+
expect(screen.queryByText('Settings')).not.toBeInTheDocument();
119+
expect(screen.queryByText('API Keys')).not.toBeInTheDocument();
120+
});
121+
122+
it('shows Administration section label for org admin', () => {
123+
renderWithProviders(<MenuBar {...defaultProps} />, {
124+
appContext: { isOrganizationAdmin: true },
125+
});
126+
expect(screen.getByText('Administration')).toBeInTheDocument();
127+
});
128+
129+
it('does not show Administration section label for regular user', () => {
130+
renderWithProviders(<MenuBar {...defaultProps} />);
131+
expect(screen.queryByText('Administration')).not.toBeInTheDocument();
132+
});
133+
134+
it('always shows Platform section label', () => {
135+
renderWithProviders(<MenuBar {...defaultProps} />);
136+
expect(screen.getByText('Platform')).toBeInTheDocument();
137+
});
138+
110139
it('populates the organization selector after fetch', async () => {
111140
global.fetch.mockImplementation((url) => {
112141
if (url.includes('/organizations/')) {
@@ -164,13 +193,4 @@ describe('MenuBar', () => {
164193
);
165194
});
166195

167-
it('expands Settings sub-menu on click to reveal API Keys', async () => {
168-
const user = userEvent.setup();
169-
renderWithProviders(<MenuBar {...defaultProps} />, {
170-
appContext: { isPlatformAdmin: true },
171-
});
172-
173-
await user.click(screen.getByText('Settings'));
174-
expect(screen.getByText('API Keys')).toBeInTheDocument();
175-
});
176196
});

0 commit comments

Comments
 (0)