From 2d2845fa0a6f50151da793c17f361f38bb2c8954 Mon Sep 17 00:00:00 2001 From: Daniel Date: Thu, 10 Sep 2026 12:43:02 +0200 Subject: [PATCH] fix: legible section bar, and a real page for editing a question MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Colours `.sr-only` was used in four places and never defined, so every label meant for a screen reader was printed on the page: "Studying for" beside the exam switcher, "Difficulty", "Questions", "Category for …". On the old dark bar the stray text passed for a label; on the new light one it was grey-on-grey and looked broken. Defined it, and restyled the exam switcher for the surface it actually sits on now rather than for the dark bar it used to. The section bar itself was white on white, reading as a stray row of links rather than a surface: it now sits a shade off the page with a hairline shadow, links take the text colour at 78% instead of the muted grey, and the current section gets a tinted pill with an underline. When the links outrun the width the strip fades at the right edge — a hard cut just looks like a broken layout, and "Courses" was being sliced in half. Editing a question The bank's Edit opened a modal that could not show option explanations, images, versions and categories at once, and the linked-questions list on an article offered only "Unlink" — you could read a question there and have no way to fix it. Both now open /questions/:id, the full page, which already existed and was reachable from almost nowhere. The page carries where you came from: back and Cancel return to the bank with its filters, or to the article, rather than always to a bank you may not have used. Its header is sticky, because this page is long enough that scrolling to the stem loses the way out entirely. The quick modal stays where a quick correction belongs — the question manager — and its two tests moved onto the component itself rather than reaching it through a page that no longer opens it. 198 frontend tests green. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XeFQJXJTfHKTfbfsdxv57Z --- frontend/src/components/PractiseTopic.jsx | 6 +- .../src/components/PractiseTopic.test.jsx | 8 +++ .../src/components/QuestionEditors.test.jsx | 62 +++++++++++++++++++ frontend/src/index.css | 49 +++++++++++---- frontend/src/pages/QuestionBankPage.jsx | 15 ++--- frontend/src/pages/QuestionBankPage.test.jsx | 49 +++------------ frontend/src/pages/QuestionEditPage.css | 10 ++- frontend/src/pages/QuestionEditPage.jsx | 11 +++- 8 files changed, 147 insertions(+), 63 deletions(-) create mode 100644 frontend/src/components/QuestionEditors.test.jsx diff --git a/frontend/src/components/PractiseTopic.jsx b/frontend/src/components/PractiseTopic.jsx index 5b1770e..21221f8 100644 --- a/frontend/src/components/PractiseTopic.jsx +++ b/frontend/src/components/PractiseTopic.jsx @@ -1,5 +1,5 @@ import { useState, useEffect } from 'react' -import { useNavigate } from 'react-router-dom' +import { Link, useNavigate } from 'react-router-dom' import api from '../api/client' const COUNTS = [5, 10, 20, 30] @@ -100,6 +100,10 @@ export default function PractiseTopic({ article, canEdit, questions, onUnlink }) {q.section_id && ( {(article.sections || []).find(s => s.id === q.section_id)?.title || 'removed section'} )} + {/* Reading a question here and having no way to fix it was + the gap: the editor is one click away, and comes back. */} + Edit diff --git a/frontend/src/components/PractiseTopic.test.jsx b/frontend/src/components/PractiseTopic.test.jsx index b918497..ea2c6b0 100644 --- a/frontend/src/components/PractiseTopic.test.jsx +++ b/frontend/src/components/PractiseTopic.test.jsx @@ -82,3 +82,11 @@ it('lets an educator review and unlink without showing answers', async () => { await userEvent.click(screen.getAllByRole('button', { name: 'Unlink' })[0]) expect(onUnlink).toHaveBeenCalledWith(1, null) }) + +it('offers the editor from a linked question, and says where to come back to', async () => { + mount({ canEdit: true }) + await userEvent.click(await screen.findByRole('button', { name: /Linked questions \(2\)/ })) + // Reading a question here and having no way to fix it was the gap. + const [edit] = screen.getAllByRole('link', { name: 'Edit' }) + expect(edit).toHaveAttribute('href', '/questions/1') +}) diff --git a/frontend/src/components/QuestionEditors.test.jsx b/frontend/src/components/QuestionEditors.test.jsx new file mode 100644 index 0000000..928452c --- /dev/null +++ b/frontend/src/components/QuestionEditors.test.jsx @@ -0,0 +1,62 @@ +import { render, screen, waitFor, within } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { MemoryRouter } from 'react-router-dom' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { QuestionEditModal } from './QuestionEditors' +import api from '../api/client' + +vi.mock('../api/client', () => ({ default: { get: vi.fn(), post: vi.fn(), patch: vi.fn(), delete: vi.fn() } })) + +const CATS = [ + { id: 1, name: 'Neonatology', parent_id: null, breadcrumbs: [{ id: 1, name: 'Neonatology' }] }, + { id: 2, name: 'Cardiology', parent_id: null, breadcrumbs: [{ id: 2, name: 'Cardiology' }] }, + { id: 3, name: 'Renal', parent_id: null, breadcrumbs: [{ id: 3, name: 'Renal' }] }, +] + +const question = (overrides = {}) => ({ + id: 7, question_text: 'Edit me', question_type: 'mcq', options: ['A', 'B'], + correct_answer: 'A', explanation: '', question_category_id: 1, category_ids: [1], ...overrides, +}) + +// The quick modal the question manager uses for a small correction; the full +// page at /questions/:id is where a question is edited properly. +const mount = (q = question()) => render( + + {}} onClose={() => {}} /> + ) + +describe('quick question edit', () => { + beforeEach(() => { + vi.clearAllMocks() + api.get.mockResolvedValue({ data: [] }) + api.patch.mockResolvedValue({ data: {} }) + }) + + it('saves additional subcategories and excludes the primary from extras', async () => { + mount(question({ category_ids: [1, 2] })) + const modal = await screen.findByRole('dialog', { name: 'Edit Question' }) + expect(within(modal).getByLabelText(/Cardiology/)).toBeChecked() + expect(within(modal).getByLabelText(/Renal/)).not.toBeChecked() + // The primary category cannot also be an extra, so it is not offered as one. + expect(within(modal).getByLabelText(/Neonatology/)).toBeDisabled() + + await userEvent.click(within(modal).getByLabelText(/Renal/)) + await userEvent.click(screen.getByRole('button', { name: 'Save Changes' })) + await waitFor(() => expect(api.patch).toHaveBeenCalledWith('/questions/7', expect.objectContaining({ + additional_category_ids: [2, 3], + question_category_id: 1, + }))) + }) + + it('saves per-option explanations keyed by option text', async () => { + mount() + await screen.findByRole('dialog', { name: 'Edit Question' }) + await userEvent.click(screen.getAllByText('Explain this option')[0]) + await userEvent.type(screen.getByLabelText('Explanation for option A'), 'Right because of this') + await userEvent.click(screen.getByRole('button', { name: 'Save Changes' })) + await waitFor(() => expect(api.patch).toHaveBeenCalledWith('/questions/7', expect.objectContaining({ + option_explanations: { A: 'Right because of this' }, + }))) + }) +}) diff --git a/frontend/src/index.css b/frontend/src/index.css index f74af73..e042e36 100644 --- a/frontend/src/index.css +++ b/frontend/src/index.css @@ -125,8 +125,11 @@ body { .navbar-account { display: flex; align-items: center; gap: 8px; } .navbar-sections { - background: var(--card-bg); + /* A shade off the page, so the bar reads as a surface rather than as the top + of the content — white on white left it looking like a stray row of links. */ + background: linear-gradient(var(--card-bg), var(--bg)); border-bottom: 1px solid var(--border); + box-shadow: 0 1px 3px rgba(15, 23, 42, 0.05); height: 46px; overflow: hidden; transition: height 0.18s ease; @@ -137,14 +140,25 @@ body { .navbar-sections:focus-within { height: 46px; } .navbar-sections-inner { display: flex; align-items: center; gap: 14px; height: 46px; } -.nav-sections { display: flex; align-items: center; gap: 2px; overflow-x: auto; scrollbar-width: none; } +/* The strip scrolls when the links outrun the width; the fade on the right is + what says so, since a hard cut just looks like a broken layout. */ +.navbar-sections-inner { position: relative; } +.nav-sections { + display: flex; align-items: center; gap: 2px; flex: 1; min-width: 0; + overflow-x: auto; scrollbar-width: none; + -webkit-mask-image: linear-gradient(90deg, #000 calc(100% - 28px), transparent); + mask-image: linear-gradient(90deg, #000 calc(100% - 28px), transparent); +} .nav-sections::-webkit-scrollbar { display: none; } .navbar .nav-sections a { - color: var(--text-muted); font-size: 0.84rem; padding: 7px 11px; - border-radius: 7px; white-space: nowrap; + color: var(--text); font-size: 0.84rem; font-weight: 500; opacity: 0.78; + padding: 7px 11px; border-radius: 7px; white-space: nowrap; +} +.navbar .nav-sections a:hover { background: var(--option-hover); opacity: 1 !important; } +.navbar .nav-sections a.is-current { + color: var(--primary); font-weight: 700; opacity: 1; + background: var(--option-sel-bg); box-shadow: inset 0 -2px 0 var(--primary); } -.navbar .nav-sections a:hover { background: var(--bg); color: var(--text); opacity: 1 !important; } -.navbar .nav-sections a.is-current { color: var(--primary); font-weight: 650; background: var(--option-sel-bg); } @media (prefers-reduced-motion: reduce) { .navbar-sections { transition: none; } } [data-theme="markdown"] .navbar .logo { color: #d4a96a; } @@ -804,11 +818,22 @@ body { } .deck-category select:hover { color: var(--text); } -/* Exam switcher — the study objective, above systems and disciplines. */ +/* Exam switcher — the study objective, above systems and disciplines. + It sits on the light section bar now, so it is styled like every other + control on a light surface rather than for a dark one. */ +.exam-switcher { display: flex; align-items: center; flex-shrink: 0; } .exam-switcher select { - background: rgba(255,255,255,.12); color: var(--navbar-fg); - border: 1px solid rgba(255,255,255,.22); border-radius: 8px; - padding: 5px 9px; font-size: .8rem; font-weight: 600; max-width: 210px; + background: var(--input-bg); color: var(--text); + border: 1px solid var(--border); border-radius: 8px; + padding: 6px 10px; font-size: .8rem; font-weight: 600; max-width: 220px; cursor: pointer; +} +.exam-switcher select:hover { border-color: var(--primary); color: var(--primary); } +.exam-switcher select option { color: var(--text); background: var(--card-bg); } +@media (max-width: 720px) { .exam-switcher select { max-width: 150px; font-size: .74rem; } } + +/* Visible only to a screen reader. This was used before it existed, which is + why "Studying for" was printed next to the control it labels. */ +.sr-only { + position: absolute; width: 1px; height: 1px; padding: 0; margin: -1px; + overflow: hidden; clip: rect(0, 0, 0, 0); white-space: nowrap; border: 0; } -.exam-switcher select option { color: #1e293b; background: #fff; } -@media (max-width: 720px) { .exam-switcher select { max-width: 140px; font-size: .74rem; } } diff --git a/frontend/src/pages/QuestionBankPage.jsx b/frontend/src/pages/QuestionBankPage.jsx index 2d4f186..9734095 100644 --- a/frontend/src/pages/QuestionBankPage.jsx +++ b/frontend/src/pages/QuestionBankPage.jsx @@ -1,5 +1,5 @@ import { useState, useEffect, useRef, lazy, Suspense } from 'react' -import { useNavigate, Link } from 'react-router-dom' +import { useLocation, useNavigate, Link } from 'react-router-dom' import { useAuth } from '../context/AuthContext' import api from '../api/client' import Dialog from '../components/Dialog' @@ -13,7 +13,7 @@ import { useDialog } from '../hooks/useDialog' const TeachChat = lazy(() => import('../components/TeachChat')) const RichEditor = lazy(() => import('../components/RichEditor')) import QuestionReadingLinks from '../components/QuestionReadingLinks' -import { QuestionEditModal, CreateQuestionModal } from '../components/QuestionEditors' +import { CreateQuestionModal } from '../components/QuestionEditors' const DIFFICULTY_LABEL = { '': 'Any', easy: 'Easy', medium: 'Medium', hard: 'Hard' } @@ -277,6 +277,8 @@ function TagBrowser({ tags, selectedTagIds, toggleTag }) { export default function QuestionBankPage() { + // Where the editor should send you back to, filters and all. + const location = useLocation() const { dialogProps, openAlert } = useDialog() const [questions, setQuestions] = useState([]) const [total, setTotal] = useState(0) @@ -299,7 +301,6 @@ export default function QuestionBankPage() { const [favorites, setFavorites] = useState([]) const [offset, setOffset] = useState(0) const [studyQuestion, setStudyQuestion] = useState(null) - const [editQuestion, setEditQuestion] = useState(null) const [selectedIds, setSelectedIds] = useState(new Set()) const [showCreateQuiz, setShowCreateQuiz] = useState(false) const [newCatName, setNewCatName] = useState('') @@ -615,9 +616,6 @@ export default function QuestionBankPage() { {studyQuestion && setStudyQuestion(null)} isFavorited={favorites.includes(studyQuestion.id)} onToggleFavorite={toggleFavorite} />} - {editQuestion && setQuestions(prev => prev.map(q => q.id === updated.id ? updated : q))} - onClose={() => setEditQuestion(null)} />} {showCreateQuiz && setShowCreateQuiz(false)} onCreated={() => {}} />} {showCreateQuestion && { setQuestions(prev => [q, ...prev]); setTotal(t => t + 1) }} onClose={() => setShowCreateQuestion(false)} />} @@ -855,7 +853,10 @@ export default function QuestionBankPage() { } catch {} }}>{q.is_shared ? 'Unshare' : 'Share'} )} - {isModerator && } + {/* The whole question, on its own page: the modal could not show + option explanations, images, versions or categories at once. */} + {isModerator && Edit} {isModerator &&