From 6b6c5e1b490dfdb63d89509b0e32e79be08a8de1 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 11 Sep 2026 20:03:11 +0200 Subject: [PATCH] fix: the objective menu was clipped out of existence; one door to settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The section bar is 46px tall with overflow hidden, so an absolutely positioned menu inside it was cropped to a strip and never appeared — the same reason it looked wrong in a settings panel, which also clips its own overflow. The menu is fixed now and the component measures the button and places it, following resize and scroll. In a settings panel there is no menu at all: the objective is stated plainly with a Change button that opens the picker, which is an overlay fixed to the viewport and so cannot be clipped by anything. The taxonomy page no longer searches the question bank. Filing a question is done where the question is; a search box on a taxonomy row was a second, worse question bank. The account page is gone. Settings opens on the account, so Account and Settings were two doors to the same room; /account redirects. Admins get an Administration entry in the person menu. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN --- frontend/src/App.jsx | 4 +- frontend/src/components/ExamSwitcher.css | 28 ++- frontend/src/components/ExamSwitcher.jsx | 164 +++++++++++------- frontend/src/components/ExamSwitcher.test.jsx | 24 +++ frontend/src/components/Navbar.jsx | 8 +- frontend/src/components/Navbar.test.jsx | 4 +- frontend/src/components/SiteFooter.jsx | 1 - frontend/src/components/SiteFooter.test.jsx | 2 +- frontend/src/pages/AccountPage.jsx | 129 -------------- frontend/src/pages/CategoriesPage.jsx | 77 +------- frontend/src/pages/CategoriesPage.test.jsx | 37 +--- frontend/src/pages/SettingsPage.jsx | 3 +- frontend/src/pages/SettingsPage.test.jsx | 8 +- 13 files changed, 177 insertions(+), 312 deletions(-) delete mode 100644 frontend/src/pages/AccountPage.jsx diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index 29aca0d..a5144c9 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -15,7 +15,6 @@ const DocumentDetailPage = lazyPage(() => import('./pages/DocumentDetailPage')) const QuizPage = lazyPage(() => import('./pages/QuizPage')) const CustomQuizPage = lazyPage(() => import('./pages/CustomQuizPage')) const ResultsPage = lazyPage(() => import('./pages/ResultsPage')) -const AccountPage = lazyPage(() => import('./pages/AccountPage')) const SettingsPage = lazyPage(() => import('./pages/SettingsPage')) const QuestionBankPage = lazyPage(() => import('./pages/QuestionBankPage')) const QuestionManagerPage = lazyPage(() => import('./pages/QuestionManagerPage')) @@ -133,7 +132,8 @@ function AppRoutes() { } /> } /> } /> - } /> + {/* Settings opens on the account; one door, not two. */} + } /> } /> } /> } /> diff --git a/frontend/src/components/ExamSwitcher.css b/frontend/src/components/ExamSwitcher.css index 796886a..85d3195 100644 --- a/frontend/src/components/ExamSwitcher.css +++ b/frontend/src/components/ExamSwitcher.css @@ -22,10 +22,13 @@ /* ── The short menu ─────────────────────────────────────────────────── What you are on, where you have been lately, and a way to the full list — so changing objective does not start with a full-screen dialog. */ -.exam-switcher { position: relative; display: inline-flex; } +.exam-switcher { display: inline-flex; } .exm { - position: absolute; top: calc(100% + 6px); left: 0; z-index: 1090; - min-width: 260px; max-width: min(320px, calc(100vw - 24px)); + /* Fixed, not absolute. The section bar it lives in is 46px tall with + overflow hidden, which clipped an absolutely positioned menu out of + existence; the component measures the button and places this itself. */ + position: fixed; top: 0; left: 0; z-index: 1090; + width: 280px; max-width: calc(100vw - 24px); padding: 6px; background: var(--card-bg); border: 1px solid var(--border); border-radius: 10px; box-shadow: 0 14px 34px rgba(15, 23, 42, 0.18); @@ -55,10 +58,23 @@ } .exm-error { margin: 6px 11px 2px; font-size: 0.78rem; color: var(--wrong-fg); } -/* On a phone the menu is the width of the screen rather than of the button. */ -@media (max-width: 520px) { - .exm { left: auto; right: 0; min-width: 240px; } +/* ── In a settings panel ────────────────────────────────────────────── + Plain, full width, and no pop-out menu: the panel clips anything that + hangs out of it, and there is room to just say what the objective is. */ +.exam-switcher-inline { + display: flex; align-items: center; justify-content: space-between; + gap: 16px; flex-wrap: wrap; + padding: 14px 16px; + border: 1px solid var(--border); border-radius: 10px; + background: var(--bg); } +.exam-switcher-inline small { + display: block; + font-size: 0.64rem; font-weight: 700; letter-spacing: 0.08em; + text-transform: uppercase; color: var(--text-subtle); +} +.exam-switcher-inline strong { display: block; margin-top: 3px; font-size: 1rem; font-weight: 650; } +.exam-switcher-inline > div > span { font-size: 0.8rem; color: var(--text-muted); } /* ── The dialog ───────────────────────────────────────────────────── */ .exo-overlay { diff --git a/frontend/src/components/ExamSwitcher.jsx b/frontend/src/components/ExamSwitcher.jsx index 287e836..b94c2ef 100644 --- a/frontend/src/components/ExamSwitcher.jsx +++ b/frontend/src/components/ExamSwitcher.jsx @@ -37,7 +37,7 @@ function rememberRecent(id) { localStorage.setItem(RECENT_KEY, JSON.stringify(next)) } catch { /* private browsing, or storage turned off */ } } -export default function ExamSwitcher({ onChange }) { +export default function ExamSwitcher({ onChange, inline = false }) { const [exams, setExams] = useState([]) const [activeId, setActiveId] = useState(null) const [menu, setMenu] = useState(false) @@ -48,6 +48,8 @@ export default function ExamSwitcher({ onChange }) { const [error, setError] = useState('') const search = useRef(null) const wrap = useRef(null) + const trigger = useRef(null) + const [anchor, setAnchor] = useState(null) const load = () => api.get('/exams/') .then(res => { setExams(res.data.exams || []); setActiveId(res.data.active_exam_id ?? null) }) @@ -57,11 +59,29 @@ export default function ExamSwitcher({ onChange }) { useEffect(() => { if (!menu) return undefined + // The menu is fixed rather than absolute. It lives inside the section bar, + // which is 46px tall with overflow hidden — an absolutely positioned menu + // is clipped to that strip and simply never appears. Fixed positioning + // escapes every overflowing ancestor, so it has to be told where to go. + const place = () => { + const box = trigger.current?.getBoundingClientRect() + if (!box) return + const width = 280 + setAnchor({ + top: Math.round(box.bottom + 6), + left: Math.round(Math.min(box.left, window.innerWidth - width - 12)), + }) + } + place() + window.addEventListener('resize', place) + window.addEventListener('scroll', place, true) const away = e => { if (!wrap.current?.contains(e.target)) setMenu(false) } const onKey = e => { if (e.key === 'Escape') setMenu(false) } document.addEventListener('mousedown', away) document.addEventListener('keydown', onKey) return () => { + window.removeEventListener('resize', place) + window.removeEventListener('scroll', place, true) document.removeEventListener('mousedown', away) document.removeEventListener('keydown', onKey) } @@ -114,9 +134,88 @@ export default function ExamSwitcher({ onChange }) { .filter(e => e && e.question_count > 0) .slice(0, RECENT_MAX) + // The full picker. Shared by both shapes: an overlay is fixed to the + // viewport, so nothing that clips the trigger can clip it. + const dialog = open ? ( +
e.target === e.currentTarget && setOpen(false)}> +
+
+

Current study objective

+ +
+

+ Scopes the question bank, the filters and your performance analysis. + Material linked to no exam stays visible whichever you pick. +

+ + setQuery(e.target.value)} /> + +
+ {/* There is no unscoped choice. Studying for nothing in + particular is not an objective, and the whole bank at once + makes the filters and the analysis mean less, not more. */} + {families.map(([family, list]) => ( +
+

{family}

+
+ {list.map(exam => { + // An objective with nothing behind it would empty the + // bank. Shown, so an educator can see it exists, but not + // selectable until it has questions. + const empty = exam.question_count === 0 + return ( + + ) + })} +
+
+ ))} + {families.length === 0 &&

Nothing matches that.

} +
+ + {error &&

{error}

} +
+ + +
+
+
+ ) : null + + // In a settings panel there is room to say the thing plainly, and the panel + // clips anything that hangs out of it — so the short menu, which is a + // navbar affordance, is skipped and the picker opens directly. + if (inline) { + return ( +
+
+ Current objective + {active ? active.name : 'None chosen'} + {active && {active.question_count} questions} +
+ + {dialog} +
+ ) + } + return (
- {menu && ( -
+
{active ? (
Current objective @@ -158,63 +258,7 @@ export default function ExamSwitcher({ onChange }) {
)} - {open && ( -
e.target === e.currentTarget && setOpen(false)}> -
-
-

Current study objective

- -
-

- Scopes the question bank, the filters and your performance analysis. - Material linked to no exam stays visible whichever you pick. -

- - setQuery(e.target.value)} /> - -
- {/* There is no unscoped choice. Studying for nothing in - particular is not an objective, and the whole bank at once - makes the filters and the analysis mean less, not more. */} - {families.map(([family, list]) => ( -
-

{family}

-
- {list.map(exam => { - // An objective with nothing behind it would empty the - // bank. Shown, so an educator can see it exists, but not - // selectable until it has questions. - const empty = exam.question_count === 0 - return ( - - ) - })} -
-
- ))} - {families.length === 0 &&

Nothing matches that.

} -
- - {error &&

{error}

} -
- - -
-
-
- )} + {dialog}
) } diff --git a/frontend/src/components/ExamSwitcher.test.jsx b/frontend/src/components/ExamSwitcher.test.jsx index 76df576..0874428 100644 --- a/frontend/src/components/ExamSwitcher.test.jsx +++ b/frontend/src/components/ExamSwitcher.test.jsx @@ -44,6 +44,30 @@ describe('ExamSwitcher', () => { expect(api.put).toHaveBeenCalledWith('/exams/active', { exam_id: 2 }) }) + it('places the menu itself, because the bar it sits in clips its own overflow', async () => { + // .navbar-sections is 46px tall with overflow:hidden. An absolutely + // positioned menu was clipped to that strip and never appeared at all. + await openMenu() + const menu = screen.getByRole('menu') + // The stylesheet is not loaded here, so `position: fixed` cannot be read + // back. What can be checked is the part the component owns: it measured + // the trigger and wrote coordinates, which is only needed because the + // menu is taken out of the clipped flow. + expect(menu.style.top).not.toBe('') + expect(menu.style.left).not.toBe('') + }) + + it('says the objective plainly in a settings panel, with no menu to clip', async () => { + render() + expect(await screen.findByText('Pediatrics Boards')).toBeInTheDocument() + expect(screen.getByText('2948 questions')).toBeInTheDocument() + // No pop-out at all: the panel has overflow:hidden, so Change opens the + // full picker, which is an overlay fixed to the viewport. + expect(screen.queryByRole('menu')).toBeNull() + await userEvent.click(screen.getByRole('button', { name: 'Change' })) + expect(await screen.findByRole('dialog', { name: 'Current study objective' })).toBeInTheDocument() + }) + it('offers no unscoped choice, and no objective with nothing behind it', async () => { await openMenu() await userEvent.click(screen.getByRole('menuitem', { name: 'Choose a new study objective' })) diff --git a/frontend/src/components/Navbar.jsx b/frontend/src/components/Navbar.jsx index 497f45c..41632b4 100644 --- a/frontend/src/components/Navbar.jsx +++ b/frontend/src/components/Navbar.jsx @@ -164,8 +164,14 @@ function AccountMenu({ user, onLogout }) { {user?.email}
setOpen(false)}>Dashboard - setOpen(false)}>Account + {/* Settings opens on the account, so a separate Account entry was + two doors to the same room. */} setOpen(false)}>Settings + {user?.role === 'admin' && ( + setOpen(false)}> + Administration + + )}
diff --git a/frontend/src/components/Navbar.test.jsx b/frontend/src/components/Navbar.test.jsx index 56b0135..328eb9a 100644 --- a/frontend/src/components/Navbar.test.jsx +++ b/frontend/src/components/Navbar.test.jsx @@ -125,7 +125,9 @@ describe('two-bar header', () => { const menu = screen.getByRole('menu') expect(within(menu).getByRole('menuitem', { name: 'Dashboard' })).toHaveAttribute('href', '/') expect(within(menu).getByRole('menuitem', { name: 'Settings' })).toHaveAttribute('href', '/settings') - expect(within(menu).getByRole('menuitem', { name: 'Account' })).toHaveAttribute('href', '/account') + // Settings opens on the account, so there is no second door to it. + expect(within(menu).queryByRole('menuitem', { name: 'Account' })).toBeNull() + expect(within(menu).getByRole('menuitem', { name: 'Settings' })).toHaveAttribute('href', '/settings') expect(within(menu).getByRole('menuitem', { name: 'Sign out' })).toBeInTheDocument() }) diff --git a/frontend/src/components/SiteFooter.jsx b/frontend/src/components/SiteFooter.jsx index d3fb007..6801483 100644 --- a/frontend/src/components/SiteFooter.jsx +++ b/frontend/src/components/SiteFooter.jsx @@ -38,7 +38,6 @@ const COLUMNS = [ links: [ { to: '/home', label: 'About' }, { to: '/home#contact', label: 'Contact' }, - { to: '/account', label: 'Account' }, { to: '/settings', label: 'Settings' }, ], }, diff --git a/frontend/src/components/SiteFooter.test.jsx b/frontend/src/components/SiteFooter.test.jsx index 67f1e67..ccc569e 100644 --- a/frontend/src/components/SiteFooter.test.jsx +++ b/frontend/src/components/SiteFooter.test.jsx @@ -5,7 +5,7 @@ import SiteFooter from './SiteFooter' const ROUTES = ['/home', '/login', '/register', '/', '/sessions', '/question-bank', '/questions/manage', '/flashcards', '/search', '/ai', '/media', '/study-plans', '/articles', - '/courses', '/account', '/settings', '/categories', '/editorial'] + '/courses', '/settings', '/categories', '/editorial'] describe('site footer', () => { it('is a way around, grouped by what you are trying to do', () => { diff --git a/frontend/src/pages/AccountPage.jsx b/frontend/src/pages/AccountPage.jsx deleted file mode 100644 index 72b235f..0000000 --- a/frontend/src/pages/AccountPage.jsx +++ /dev/null @@ -1,129 +0,0 @@ -import { useState } from 'react' -import { useAuth } from '../context/AuthContext' -import api from '../api/client' - -export default function AccountPage() { - const { user, login } = useAuth() - const [name, setName] = useState(user?.name || '') - const [currentPassword, setCurrentPassword] = useState('') - const [newPassword, setNewPassword] = useState('') - const [confirmPassword, setConfirmPassword] = useState('') - const [loading, setLoading] = useState(false) - const [error, setError] = useState('') - const [success, setSuccess] = useState('') - - const handleSubmit = async (e) => { - e.preventDefault() - setError('') - setSuccess('') - - if (newPassword && newPassword !== confirmPassword) { - setError('New passwords do not match') - return - } - if (newPassword && newPassword.length < 8) { - setError('New password must be at least 8 characters') - return - } - - setLoading(true) - try { - const payload = {} - if (name !== user.name) payload.name = name - if (newPassword) { - payload.current_password = currentPassword - payload.new_password = newPassword - } - - if (!Object.keys(payload).length) { - setError('No changes to save') - setLoading(false) - return - } - - await api.put('/auth/me', payload) - setSuccess('Account updated successfully') - setCurrentPassword('') - setNewPassword('') - setConfirmPassword('') - // Refresh user info - await api.get('/auth/me') - // Update local token display by refreshing - window.location.reload() - } catch (err) { - setError(err.response?.data?.detail || 'Failed to update account') - } finally { - setLoading(false) - } - } - - return ( -
-
-

My Account

-

- {user?.email} · - {user?.role} -

-
- - {error &&
{error}
} - {success &&
{success}
} - -
-
-
- - setName(e.target.value)} - required - /> -
- -
-

- Change Password (leave blank to keep current) -

- -
- - setCurrentPassword(e.target.value)} - placeholder="Required to change password" - /> -
-
- - setNewPassword(e.target.value)} - placeholder="At least 8 characters" - /> -
-
- - setConfirmPassword(e.target.value)} - /> -
- - -
-
-
- ) -} diff --git a/frontend/src/pages/CategoriesPage.jsx b/frontend/src/pages/CategoriesPage.jsx index c7cf1e4..b2c34fe 100644 --- a/frontend/src/pages/CategoriesPage.jsx +++ b/frontend/src/pages/CategoriesPage.jsx @@ -57,12 +57,6 @@ export default function CategoriesPage() { const [newParent, setNewParent] = useState('') const [busy, setBusy] = useState(false) - // Attaching questions to whichever entry is open for it. - const [attaching, setAttaching] = useState(null) - const [attachQuery, setAttachQuery] = useState('') - const [attachResults, setAttachResults] = useState([]) - const [attachPicked, setAttachPicked] = useState([]) - const [attachSearching, setAttachSearching] = useState(false) const facet = FACETS.find(f => f.key === facetKey) || FACETS[0] @@ -90,7 +84,7 @@ export default function CategoriesPage() { // search does not follow you onto a different axis. useEffect(() => { setQuery(''); setExpanded({}); setEditing(null); setDeleting(null) - setCreating(false); setAttaching(null); setError(''); setNotice('') + setCreating(false); setError(''); setNotice('') }, [facetKey]) /** Rows for the open facet, normalised to one shape whichever table they came from. */ @@ -173,7 +167,7 @@ export default function CategoriesPage() { const endpoint = facet.source === 'category' ? '/question-categories' : '/tags' const startEdit = (row) => { - setDeleting(null); setAttaching(null) + setDeleting(null) setEditing(row.id); setDraftName(row.name); setDraftParent(row.parent_id ?? '') } @@ -221,37 +215,6 @@ export default function CategoriesPage() { finally { setBusy(false) } } - const startAttach = (row) => { - setEditing(null); setDeleting(null) - setAttaching(row.id); setAttachQuery(''); setAttachResults([]); setAttachPicked([]) - } - - const searchQuestions = async () => { - if (!attachQuery.trim()) return - setAttachSearching(true); setError('') - try { - const res = await api.get('/questions/bank', { params: { q: attachQuery.trim(), limit: 25 } }) - setAttachResults(res.data?.questions || res.data?.items || []) - } catch (err) { setError(apiError(err, 'Could not search questions')) } - finally { setAttachSearching(false) } - } - - const attach = async (row) => { - if (!attachPicked.length) return - setBusy(true); setError('') - try { - if (facet.source === 'category') { - await api.post('/questions/bulk-category', - { question_ids: attachPicked, category_id: row.id }) - } else { - await api.post(`/tags/${row.id}/questions`, { question_ids: attachPicked }) - } - setNotice(`Added ${attachPicked.length} question${attachPicked.length === 1 ? '' : 's'} to “${row.name}”.`) - setAttaching(null); setAttachPicked([]); setAttachResults([]) - load() - } catch (err) { setError(apiError(err, 'Could not attach those questions')) } - finally { setBusy(false) } - } // Deleting a category with subcategories is refused server-side; tags instead // pull their children up a level, so only one facet needs the guard. @@ -268,11 +231,9 @@ export default function CategoriesPage() { <> - + onClick={() => { setEditing(null); setDeleting(row.id); setMoveTo('') }}>Delete )} @@ -309,38 +270,6 @@ export default function CategoriesPage() { )} - {attaching === row.id && ( -
-
- setAttachQuery(e.target.value)} - onKeyDown={e => { if (e.key === 'Enter') searchQuestions() }} /> - - -
- {attachResults.length > 0 && ( -
    - {attachResults.map(q => ( -
  • - -
    • - ))} -
- )} - {attachPicked.length > 0 && ( - - )} -
- )} ) diff --git a/frontend/src/pages/CategoriesPage.test.jsx b/frontend/src/pages/CategoriesPage.test.jsx index b4f8dcd..2ec4348 100644 --- a/frontend/src/pages/CategoriesPage.test.jsx +++ b/frontend/src/pages/CategoriesPage.test.jsx @@ -213,38 +213,11 @@ it('deletes a tag, saying its children rise rather than refusing', async () => { await waitFor(() => expect(api.delete).toHaveBeenCalledWith('/tags/21', { params: {} })) }) -it('attaches searched questions to a tag', async () => { +it('does not search the question bank from here', async () => { mount() await screen.findByText('Root') - await openFacet('Diseases') - api.get.mockResolvedValueOnce({ data: { questions: [ - { id: 5, question_text: 'A 4-year-old with wheeze…' }, - { id: 6, question_text: 'An infant with stridor…' }, - ] } }) - api.post.mockResolvedValue({ data: { added: 1 } }) - - await userEvent.click(await screen.findByRole('button', { name: 'Add questions to Asthma' })) - await userEvent.type(screen.getByLabelText('Search questions to add to Asthma'), 'wheeze') - await userEvent.click(screen.getByRole('button', { name: 'Search' })) - - await userEvent.click(await screen.findByRole('checkbox', { name: /wheeze/ })) - await userEvent.click(screen.getByRole('button', { name: 'Add 1 question' })) - - await waitFor(() => expect(api.post).toHaveBeenCalledWith('/tags/30/questions', { question_ids: [5] })) -}) - -it('files questions into a topic through the category endpoint instead', async () => { - mount() - await screen.findByText('Root') - api.get.mockResolvedValueOnce({ data: { questions: [{ id: 5, question_text: 'A newborn…' }] } }) - api.post.mockResolvedValue({ data: { updated: 1 } }) - - await userEvent.click(screen.getByRole('button', { name: 'Add questions to Root' })) - await userEvent.type(screen.getByLabelText('Search questions to add to Root'), 'newborn') - await userEvent.click(screen.getByRole('button', { name: 'Search' })) - await userEvent.click(await screen.findByRole('checkbox', { name: /newborn/ })) - await userEvent.click(screen.getByRole('button', { name: 'Add 1 question' })) - - await waitFor(() => expect(api.post).toHaveBeenCalledWith('/questions/bulk-category', - { question_ids: [5], category_id: 1 })) + // Filing a question is done where the question is. Searching the whole + // bank from a taxonomy row was a second, worse question bank. + expect(screen.queryByRole('button', { name: /Add questions to/ })).toBeNull() + expect(screen.queryByPlaceholderText('Search questions…')).toBeNull() }) diff --git a/frontend/src/pages/SettingsPage.jsx b/frontend/src/pages/SettingsPage.jsx index 504ad99..9a83230 100644 --- a/frontend/src/pages/SettingsPage.jsx +++ b/frontend/src/pages/SettingsPage.jsx @@ -123,7 +123,7 @@ function StudySection() { return (
- +
) } @@ -461,7 +461,6 @@ export default function SettingsPage() {

Settings

-

Your account, how the site looks, and — if it is yours to set — how the site behaves.