fix: legible section bar, and a real page for editing a question
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XeFQJXJTfHKTfbfsdxv57Z
This commit is contained in:
parent
bbc35ce5d8
commit
2d2845fa0a
8 changed files with 147 additions and 63 deletions
|
|
@ -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 && (
|
||||
<em>{(article.sections || []).find(s => s.id === q.section_id)?.title || 'removed section'}</em>
|
||||
)}
|
||||
{/* Reading a question here and having no way to fix it was
|
||||
the gap: the editor is one click away, and comes back. */}
|
||||
<Link className="btn btn-sm btn-secondary" to={`/questions/${q.question_id}`}
|
||||
state={{ from: `/articles/${article.id}`, label: article.title }}>Edit</Link>
|
||||
<button type="button" className="btn btn-sm btn-secondary"
|
||||
onClick={() => onUnlink(q.question_id, q.section_id)}>Unlink</button>
|
||||
</li>
|
||||
|
|
|
|||
|
|
@ -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')
|
||||
})
|
||||
|
|
|
|||
62
frontend/src/components/QuestionEditors.test.jsx
Normal file
62
frontend/src/components/QuestionEditors.test.jsx
Normal file
|
|
@ -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(
|
||||
<MemoryRouter>
|
||||
<QuestionEditModal question={q} categories={CATS} onSaved={() => {}} onClose={() => {}} />
|
||||
</MemoryRouter>)
|
||||
|
||||
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' },
|
||||
})))
|
||||
})
|
||||
})
|
||||
|
|
@ -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; } }
|
||||
|
|
|
|||
|
|
@ -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 && <QuestionStudyModal question={studyQuestion} collections={collections} onClose={() => setStudyQuestion(null)}
|
||||
isFavorited={favorites.includes(studyQuestion.id)} onToggleFavorite={toggleFavorite} />}
|
||||
{editQuestion && <QuestionEditModal question={editQuestion} categories={categories}
|
||||
onSaved={updated => setQuestions(prev => prev.map(q => q.id === updated.id ? updated : q))}
|
||||
onClose={() => setEditQuestion(null)} />}
|
||||
{showCreateQuiz && <CreateQuizModal selectedIds={selectedIds} categories={categories} onClose={() => setShowCreateQuiz(false)} onCreated={() => {}} />}
|
||||
{showCreateQuestion && <CreateQuestionModal categories={categories} onCreated={(q) => { 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'}</button>
|
||||
)}
|
||||
{isModerator && <button className="btn btn-sm btn-secondary" onClick={() => setEditQuestion(q)}>Edit</button>}
|
||||
{/* The whole question, on its own page: the modal could not show
|
||||
option explanations, images, versions or categories at once. */}
|
||||
{isModerator && <Link className="btn btn-sm btn-secondary" to={`/questions/${q.id}`}
|
||||
state={{ from: `${location.pathname}${location.search}`, label: 'Question bank' }}>Edit</Link>}
|
||||
{isModerator && <button className="btn btn-sm btn-secondary" style={{ color: 'var(--wrong-fg)' }}
|
||||
onClick={async () => {
|
||||
if (!await openAlert(`Delete "${stripHtml(q.question_text).slice(0, 80)}..."? This removes it from all quizzes.`, { title: 'Delete Question', confirmLabel: 'Delete', cancelLabel: 'Cancel' })) return
|
||||
|
|
|
|||
|
|
@ -144,47 +144,18 @@ describe('QuestionBankPage edit modal multi-category', () => {
|
|||
mockInitialRequests()
|
||||
})
|
||||
|
||||
it('saves additional subcategories and excludes the primary from extras', async () => {
|
||||
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 = { id: 7, question_text: 'Edit me', question_type: 'mcq', options: ['A', 'B'], correct_answer: 'A', explanation: '', question_category_id: 1, category_ids: [1, 2] }
|
||||
const initialGet = api.get.getMockImplementation()
|
||||
api.get.mockImplementation(url => url === '/question-categories/' ? Promise.resolve({ data: cats })
|
||||
: url === '/questions/bank' ? Promise.resolve({ data: { questions: [question], total: 1 } }) : initialGet(url))
|
||||
api.patch = vi.fn().mockResolvedValue({ data: {} })
|
||||
renderPage()
|
||||
await userEvent.click(await screen.findByRole('button', { name: 'Edit' }))
|
||||
const modal = await screen.findByRole('dialog', { name: 'Edit Question' })
|
||||
expect(within(modal).getByLabelText(/Cardiology/)).toBeChecked()
|
||||
expect(within(modal).getByLabelText(/Renal/)).not.toBeChecked()
|
||||
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 () => {
|
||||
const cats = [
|
||||
{ id: 1, name: 'Neonatology', parent_id: null, breadcrumbs: [{ id: 1, name: 'Neonatology' }] },
|
||||
]
|
||||
it('opens the full editor rather than a modal, and says where to come back to', async () => {
|
||||
const question = { id: 7, question_text: 'Edit me', question_type: 'mcq', options: ['A', 'B'], correct_answer: 'A', explanation: '', question_category_id: 1, category_ids: [1] }
|
||||
const initialGet = api.get.getMockImplementation()
|
||||
api.get.mockImplementation(url => url === '/question-categories/' ? Promise.resolve({ data: cats })
|
||||
: url === '/questions/bank' ? Promise.resolve({ data: { questions: [question], total: 1 } }) : initialGet(url))
|
||||
api.patch = vi.fn().mockResolvedValue({ data: {} })
|
||||
api.get.mockImplementation(url => url === '/questions/bank'
|
||||
? Promise.resolve({ data: { questions: [question], total: 1 } }) : initialGet(url))
|
||||
renderPage()
|
||||
await userEvent.click(await screen.findByRole('button', { name: 'Edit' }))
|
||||
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' },
|
||||
})))
|
||||
|
||||
// A modal could not show option explanations, images, versions and
|
||||
// categories at once, which is what editing a question actually needs.
|
||||
const edit = await screen.findByRole('link', { name: 'Edit' })
|
||||
expect(edit).toHaveAttribute('href', '/questions/7')
|
||||
await userEvent.click(edit)
|
||||
expect(screen.queryByRole('dialog', { name: 'Edit Question' })).not.toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -3,7 +3,15 @@
|
|||
|
||||
.qe-page { max-width: 1180px; margin: 0 auto; padding-bottom: 90px; }
|
||||
|
||||
.qe-top { display: flex; align-items: flex-start; gap: 12px; flex-wrap: wrap; margin-bottom: 14px; }
|
||||
/* The way out stays reachable. This page is long enough that scrolling to the
|
||||
stem loses the back link entirely, and the save bar is pinned at the bottom
|
||||
for the same reason. */
|
||||
.qe-top {
|
||||
display: flex; align-items: flex-start; gap: 12px; flex-wrap: wrap;
|
||||
position: sticky; top: 52px; z-index: 20;
|
||||
margin: 0 -16px 14px; padding: 12px 16px;
|
||||
background: var(--bg); border-bottom: 1px solid var(--border);
|
||||
}
|
||||
.qe-back { font-size: 0.85rem; color: var(--primary); text-decoration: none; }
|
||||
.qe-title { flex: 1; min-width: 200px; }
|
||||
.qe-title h1 { margin: 6px 0 4px; font-size: 1.3rem; }
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import { useState, useEffect, useCallback, useMemo } from 'react'
|
||||
import { Link, useNavigate, useParams } from 'react-router-dom'
|
||||
import { Link, useLocation, useNavigate, useParams } from 'react-router-dom'
|
||||
import api from '../api/client'
|
||||
import CategoryDrilldown from '../components/CategoryDrilldown'
|
||||
import ImagePicker from '../components/ImagePicker'
|
||||
|
|
@ -40,6 +40,11 @@ export default function QuestionEditPage({ mode = 'edit' }) {
|
|||
const [showVersions, setShowVersions] = useState(false)
|
||||
// Which image field the picker is filling, or null when it is closed.
|
||||
const [picking, setPicking] = useState(null)
|
||||
// Back to wherever you opened this from — the bank with its filters, an
|
||||
// article, the manager — rather than always to the bank you may not have used.
|
||||
const { state } = useLocation()
|
||||
const backTo = state?.from || '/question-bank'
|
||||
const backLabel = state?.label || 'Question bank'
|
||||
|
||||
useEffect(() => {
|
||||
api.get('/question-categories/').then(res => setCategories(res.data || [])).catch(() => setCategories([]))
|
||||
|
|
@ -171,7 +176,7 @@ export default function QuestionEditPage({ mode = 'edit' }) {
|
|||
<div className="qe-page">
|
||||
<div className="qe-top">
|
||||
<div className="qe-title">
|
||||
<Link className="qe-back" to="/question-bank">← Question bank</Link>
|
||||
<Link className="qe-back" to={backTo}>← {backLabel}</Link>
|
||||
<h1>{isCreate ? 'New question' : 'Edit question'}</h1>
|
||||
{!isCreate && (
|
||||
<div className="qe-idline">
|
||||
|
|
@ -399,7 +404,7 @@ export default function QuestionEditPage({ mode = 'edit' }) {
|
|||
<span className="qe-bar-status" role="status" aria-live="polite">
|
||||
{status || (isCreate ? 'Not saved yet' : `Question #${id}`)}
|
||||
</span>
|
||||
<Link className="btn btn-secondary" to="/question-bank">Cancel</Link>
|
||||
<Link className="btn btn-secondary" to={backTo}>Cancel</Link>
|
||||
<button className="btn btn-primary qe-save" disabled={saving} onClick={save}>
|
||||
{saving ? 'Saving…' : isCreate ? 'Create question' : 'Save changes'}
|
||||
</button>
|
||||
|
|
|
|||
Loading…
Reference in a new issue