From 1be6d10550f12d5c6949713ce58cfaf7ce61bb83 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 12 Sep 2026 19:04:51 +0200 Subject: [PATCH] feat: a QBank door on every article, which says why it is empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The offer to practise a topic was a paragraph under the last section: it appeared only where questions happened to be linked, explained itself at length to everybody including the reader who never wanted questions, and sat where nobody looks. Now it is a button in the controls row, on every article whether or not anything is linked — and that is the point of it. A reader's question is "are there questions on this?", and a control that vanishes when the answer is no cannot answer it. With nothing linked it opens a dialog that names the study objective, says there is nothing for *that* objective, and offers a custom session; the objective is usually the whole reason, and a learner who cannot see which one they are on has no way to tell that from "nothing exists on this topic at all". With questions behind it, it is the builder it always was. Reading pages still never show a stem, an answer or an explanation. Also: the two selects stack and the dialog scrolls inside itself under 520px, and every control is left to the site-wide 16px rule for coarse pointers — a field under 16px makes iOS Safari zoom in on focus and never zoom back out. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN --- frontend/src/components/ArticleReader.jsx | 15 +- frontend/src/components/PractiseTopic.jsx | 109 ------------ frontend/src/components/QBank.css | 79 +++++++++ frontend/src/components/QBank.jsx | 165 ++++++++++++++++++ ...{PractiseTopic.test.jsx => QBank.test.jsx} | 42 +++-- frontend/src/pages/ArticlesPage.css | 35 +++- frontend/src/pages/ArticlesPage.jsx | 24 ++- frontend/src/pages/ArticlesPage.test.jsx | 15 +- 8 files changed, 342 insertions(+), 142 deletions(-) delete mode 100644 frontend/src/components/PractiseTopic.jsx create mode 100644 frontend/src/components/QBank.css create mode 100644 frontend/src/components/QBank.jsx rename frontend/src/components/{PractiseTopic.test.jsx => QBank.test.jsx} (55%) diff --git a/frontend/src/components/ArticleReader.jsx b/frontend/src/components/ArticleReader.jsx index edb008b..246ae7c 100644 --- a/frontend/src/components/ArticleReader.jsx +++ b/frontend/src/components/ArticleReader.jsx @@ -101,6 +101,10 @@ export default function ArticleReader({ // report — is the page's business and fetches its own; the reader knows the // shape of the article and nothing about who is reading it. sectionTools = null, + // The QBank door, drawn in the controls row beside the reading switches. + // Passed in rather than built here for the same reason as the toolbar: what + // is behind it depends on who is reading, which the reader does not know. + qbank = null, children, }) { // Which sections are open. Everything starts closed: an article is a reference @@ -317,12 +321,18 @@ export default function ArticleReader({ {hasSummary && fallback && ( /* "High yield", not "Summary": the section it shows is already called Summary, and a control naming the thing it reveals reads as a label - for it rather than a switch over it. This says what it is for. */ + for it rather than a switch over it. This says what it is for. + + And no "on"/"off" after it. A switch that is lit is on — saying so + in words as well makes the reader parse a sentence to learn what the + colour already told them, and "High yield off" reads for a moment + like a thing you could be looking at. `aria-pressed` carries the + state for anyone who cannot see the lit state. */ )} {full.length > 1 && ( @@ -423,6 +433,7 @@ export default function ArticleReader({ article you have got to once the title has gone. */}
+ {qbank} - - ) - } - - return ( -
-
-

Practise this topic

- - {available} question{available === 1 ? '' : 's'} linked - -
-
- - - -
- {error &&

{error}

} -
- ) -} diff --git a/frontend/src/components/QBank.css b/frontend/src/components/QBank.css new file mode 100644 index 0000000..049eb4c --- /dev/null +++ b/frontend/src/components/QBank.css @@ -0,0 +1,79 @@ +/* The QBank door on an article, and what opens behind it. */ +.qbank-btn { + display: inline-flex; align-items: center; gap: 7px; + min-height: 34px; padding: 6px 13px; cursor: pointer; + font: inherit; font-size: 0.83rem; font-weight: 600; + color: var(--primary); background: color-mix(in srgb, var(--primary) 7%, transparent); + border: 1px solid color-mix(in srgb, var(--primary) 35%, transparent); border-radius: 8px; +} +.qbank-btn:hover { background: color-mix(in srgb, var(--primary) 13%, transparent); } +.qbank-btn span { font-size: 0.95rem; line-height: 1; } + +.qbank-overlay { + position: fixed; inset: 0; z-index: 1000; padding: 16px; + display: flex; align-items: center; justify-content: center; + background: rgba(15, 23, 42, .55); +} +.qbank { + position: relative; width: 100%; max-width: 520px; + padding: 26px 28px 22px; border-radius: 14px; + background: var(--card-bg); box-shadow: 0 24px 60px rgba(15, 23, 42, .35); +} +.qbank h2 { margin: 0 0 4px; font-size: 1.15rem; letter-spacing: -0.01em; } +.qbank-sub { margin: 0 0 18px; font-size: 0.9rem; font-weight: 600; color: var(--text-muted); } +.qbank-count { margin: 0 0 14px; font-size: 0.86rem; color: var(--text-muted); } +.qbank-close { + position: absolute; top: 12px; right: 12px; + width: 30px; height: 30px; border: 0; border-radius: 8px; cursor: pointer; + background: none; color: var(--text-muted); font-size: 0.95rem; +} +.qbank-close:hover { background: var(--bg); color: var(--text); } + +.qbank-objective { + padding-top: 14px; border-top: 1px solid var(--border); + display: flex; flex-direction: column; gap: 4px; margin-bottom: 14px; +} +.qbank-objective span { + font-size: 0.7rem; font-weight: 700; letter-spacing: 0.08em; + text-transform: uppercase; color: var(--text-muted); +} +.qbank-objective a { font-size: 0.95rem; font-weight: 600; color: var(--primary); } + +/* The explanation, boxed and quiet: it is the answer to "why is this empty", + not an announcement. */ +.qbank-note { + padding: 13px 15px; border-radius: 9px; margin-bottom: 18px; + background: color-mix(in srgb, var(--primary) 6%, transparent); + border: 1px solid color-mix(in srgb, var(--primary) 18%, transparent); +} +.qbank-note p { margin: 0; font-size: 0.86rem; line-height: 1.55; color: var(--text); } +.qbank-note p + p { margin-top: 6px; color: var(--text-muted); } + +.qbank-controls { display: flex; gap: 14px; flex-wrap: wrap; margin-bottom: 16px; } +.qbank-controls label { display: flex; flex-direction: column; gap: 5px; font-size: 0.78rem; color: var(--text-muted); } +.qbank-controls select { + min-height: 36px; padding: 6px 10px; font: inherit; font-size: 0.88rem; + border: 1px solid var(--border); border-radius: 8px; + background: var(--input-bg); color: var(--text); +} +.qbank-error { margin: 0 0 12px; font-size: 0.84rem; color: var(--wrong-fg); } +.qbank-actions { display: flex; justify-content: flex-end; } + +/* Narrow screens. The dialog keeps its own scroll rather than growing past the + window — a phone in landscape is 380px tall and the button was below it — + and the controls stack instead of squeezing two selects onto one line. + Field sizes are left alone: the site-wide `pointer: coarse` rule puts every + control at 16px, which is what stops iOS Safari zooming in on focus and + never zooming back out. */ +@media (max-width: 520px) { + .qbank-overlay { padding: 10px; align-items: flex-end; } + .qbank { + padding: 20px 18px 18px; border-radius: 14px 14px 10px 10px; + max-height: 88dvh; overflow-y: auto; + } + .qbank h2 { font-size: 1.05rem; padding-right: 28px; } + .qbank-controls { flex-direction: column; gap: 10px; } + .qbank-controls label { width: 100%; } + .qbank-controls select { width: 100%; } + .qbank-actions .btn { width: 100%; } +} diff --git a/frontend/src/components/QBank.jsx b/frontend/src/components/QBank.jsx new file mode 100644 index 0000000..e3ccda8 --- /dev/null +++ b/frontend/src/components/QBank.jsx @@ -0,0 +1,165 @@ +import { useEffect, useState } from 'react' +import { Link, useNavigate } from 'react-router-dom' +import api from '../api/client' +import './QBank.css' + +const COUNTS = [5, 10, 20, 30] + +/** + * The questions behind a topic, reached from the article itself. + * + * Reading pages must never show the questions: the linked list once printed + * every stem with its correct answer and explanation, which spoiled them + * before the learner had attempted anything. So this is a door, not a list. + * + * It is on every article, whether or not anything is linked, and that is the + * point of it. A button that appears only when there happen to be questions + * teaches a reader to scan for it; one that is always there, and says plainly + * why it has nothing for them, answers the question they actually have — + * *are* there questions on this, and if not, why not. What they are studying + * for is usually the reason, so the dialog names it and offers the way round + * it rather than leaving them to work that out. + * + * A paragraph under the article used to carry this. It explained itself at + * length to everybody, including the reader who never wanted questions, and it + * sat where nobody looks — after the last section. + */ +export default function QBank({ article }) { + //: null while unknown, then the number this learner may actually be shown — + //: the count is scoped by the same exam predicate the builder uses, so it is + //: never a promise the session cannot keep. + const [available, setAvailable] = useState(null) + const [open, setOpen] = useState(false) + const [count, setCount] = useState(10) + const [mode, setMode] = useState('learning') + const [busy, setBusy] = useState(false) + const [error, setError] = useState('') + const [objective, setObjective] = useState(null) + const navigate = useNavigate() + + useEffect(() => { + let active = true + api.get('/questions/builder/count', { params: { article_ids: article.id, state: 'all' } }) + .then(res => { if (active) setAvailable(res.data.count) }) + .catch(() => { if (active) setAvailable(0) }) + return () => { active = false } + }, [article.id]) + + // Asked for only when there is nothing to practise, because that is the only + // dialog that names it — and it is the likeliest reason for the nothing. + useEffect(() => { + if (!open || available || objective !== null) return + api.get('/exams/') + .then(res => { + const active = (res.data.exams || []).find(e => e.id === res.data.active_exam_id) + setObjective(active?.name || '') + }) + .catch(() => setObjective('')) + }, [open, available, objective]) + + useEffect(() => { + if (!open) return undefined + const onKey = e => { if (e.key === 'Escape') setOpen(false) } + document.addEventListener('keydown', onKey) + return () => document.removeEventListener('keydown', onKey) + }, [open]) + + const create = async () => { + setBusy(true) + setError('') + try { + const res = await api.post('/questions/builder', { + title: `${article.title} — practice`, + category_ids: [], state: 'all', count: Math.min(count, available), + expected_count: available, mode, time_limit_minutes: null, is_shared: false, + difficulty: null, algorithm: 'random', + article_ids: [article.id], tag_ids: [], explicit_ids: [], + }) + navigate(`/study/${res.data.id}`) + } catch (err) { + const detail = err.response?.data?.detail + setError(typeof detail === 'string' ? detail : 'Could not create a test from this topic.') + } finally { setBusy(false) } + } + + // Until the count is known the button would be a guess about what it opens. + if (available === null) return null + + return ( + <> + + + {open && ( +
e.target === e.currentTarget && setOpen(false)}> +
+ + + {available > 0 ? ( + <> +

Practise this topic

+

{article.title}

+

+ {available} question{available === 1 ? '' : 's'} linked +

+
+ + +
+ {error &&

{error}

} +
+ +
+ + ) : ( + <> +

No matching questions for this article

+

{article.title}

+ {/* Named, and linked to where it is changed. The objective is + usually the whole reason there is nothing here, and a + reader who cannot see which one they are on has no way to + tell that from "this topic has no questions at all". */} + {objective ? ( +
+ Study objective + setOpen(false)}>{objective} +
+ ) : null} +
+

+ {objective + ? <>There are no questions for this article that meet your study objective {objective} criteria. + : <>There are no questions linked to this article yet.} +

+

Create a custom session with related articles, disciplines, or systems.

+
+
+ +
+ + )} +
+
+ )} + + ) +} diff --git a/frontend/src/components/PractiseTopic.test.jsx b/frontend/src/components/QBank.test.jsx similarity index 55% rename from frontend/src/components/PractiseTopic.test.jsx rename to frontend/src/components/QBank.test.jsx index 4d38b4f..deb252a 100644 --- a/frontend/src/components/PractiseTopic.test.jsx +++ b/frontend/src/components/QBank.test.jsx @@ -2,7 +2,7 @@ import { render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { MemoryRouter, Route, Routes } from 'react-router-dom' import { expect, it, vi, beforeEach } from 'vitest' -import PractiseTopic from './PractiseTopic' +import QBank from './QBank' import api from '../api/client' vi.mock('../api/client', () => ({ default: { get: vi.fn(), post: vi.fn() } })) @@ -12,33 +12,38 @@ const ARTICLE = { id: 7, title: 'Kawasaki disease', sections: [{ id: 'sec1', tit const mount = () => render( - } /> + } /> Test ready} /> + Custom session} /> ) +const answers = (count, exams) => api.get.mockImplementation(url => + url === '/exams/' + ? Promise.resolve({ data: exams }) + : Promise.resolve({ data: { count } })) + beforeEach(() => { vi.clearAllMocks() - api.get.mockResolvedValue({ data: { count: 12 } }) + answers(12, { exams: [], active_exam_id: null }) }) it('offers a test instead of listing the questions and their answers', async () => { mount() + await userEvent.click(await screen.findByRole('button', { name: /QBank/ })) expect(await screen.findByText('12 questions linked')).toBeInTheDocument() expect(screen.getByRole('button', { name: 'Create test' })).toBeInTheDocument() // Reading the article must never reveal a stem, its answer or its explanation. expect(screen.queryByText(/A 3-year-old with 6 days of fever/)).not.toBeInTheDocument() expect(screen.queryByText(/IVIG/)).not.toBeInTheDocument() - expect(screen.queryByText(/Because/)).not.toBeInTheDocument() - // Nor is it a place to manage links; that is the article editor's job. - expect(screen.queryByRole('button', { name: /Linked questions/ })).not.toBeInTheDocument() }) it('builds a test scoped to this article', async () => { api.post.mockResolvedValue({ data: { id: 55 } }) mount() + await userEvent.click(await screen.findByRole('button', { name: /QBank/ })) await screen.findByText('12 questions linked') await userEvent.selectOptions(screen.getByLabelText('Questions'), '12') // the "All 12" option await userEvent.selectOptions(screen.getByLabelText('Mode'), 'timed') @@ -53,14 +58,31 @@ it('builds a test scoped to this article', async () => { it('surfaces a builder failure without navigating', async () => { api.post.mockRejectedValue({ response: { data: { detail: 'Available count changed. Refresh and try again' } } }) mount() + await userEvent.click(await screen.findByRole('button', { name: /QBank/ })) await screen.findByText('12 questions linked') await userEvent.click(screen.getByRole('button', { name: 'Create test' })) expect(await screen.findByRole('alert')).toHaveTextContent('Available count changed') expect(screen.queryByRole('heading', { name: 'Test ready' })).not.toBeInTheDocument() }) -it('stays hidden for a learner when nothing is linked', async () => { - api.get.mockResolvedValue({ data: { count: 0 } }) - const { container } = mount() - await waitFor(() => expect(container).toBeEmptyDOMElement()) +it('is still there with nothing linked, and names the objective that is why', async () => { + answers(0, { exams: [{ id: 3, name: 'USMLE Step 2 CK' }], active_exam_id: 3 }) + mount() + // The door is on every article. A button that appears only where there + // happen to be questions cannot answer "are there any?". + await userEvent.click(await screen.findByRole('button', { name: /QBank/ })) + expect(await screen.findByRole('heading', { name: 'No matching questions for this article' })).toBeInTheDocument() + expect(screen.getByRole('link', { name: 'USMLE Step 2 CK' })).toBeInTheDocument() + expect(screen.getByText(/no questions for this article that meet your study objective/i)).toBeInTheDocument() + + await userEvent.click(screen.getByRole('button', { name: 'Create a custom session' })) + expect(await screen.findByRole('heading', { name: 'Custom session' })).toBeInTheDocument() +}) + +it('says the plain thing when no objective is set', async () => { + answers(0, { exams: [], active_exam_id: null }) + mount() + await userEvent.click(await screen.findByRole('button', { name: /QBank/ })) + expect(await screen.findByText('There are no questions linked to this article yet.')).toBeInTheDocument() + expect(screen.queryByText(/study objective/i)).not.toBeInTheDocument() }) diff --git a/frontend/src/pages/ArticlesPage.css b/frontend/src/pages/ArticlesPage.css index d119174..7e14f40 100644 --- a/frontend/src/pages/ArticlesPage.css +++ b/frontend/src/pages/ArticlesPage.css @@ -197,12 +197,6 @@ /* ── Practise this topic ──────────────────────────────────────────── */ .article-practise { margin-top: 22px; border-top: 1px solid var(--border); padding-top: 16px; } -/* Quieter than the offer it replaces: it is an explanation, not a control. */ -.article-practise.is-empty h3 { margin: 0 0 6px; font-size: 0.95rem; } -.article-practise.is-empty p { - margin: 0 0 12px; max-width: 62ch; - font-size: 0.86rem; line-height: 1.6; color: var(--text-muted); -} .article-practise-head { display: flex; align-items: baseline; gap: 10px; flex-wrap: wrap; margin-bottom: 10px; } .article-practise-head h3 { margin: 0; font-size: 1rem; } .article-practise-count { font-size: .78rem; color: var(--text-muted); } @@ -335,7 +329,9 @@ background: color-mix(in srgb, var(--primary) 8%, transparent); } -.aview-switch { display: inline-flex; gap: 2px; padding: 3px; background: var(--bg); border: 1px solid var(--border); border-radius: 9px; } +/* Six pixels, not two. At two the selected tab's fill ran straight into the + one beside it and the pair read as one wide control with a coloured half. */ +.aview-switch { display: inline-flex; gap: 6px; padding: 3px; background: var(--bg); border: 1px solid var(--border); border-radius: 9px; } .aview { min-height: 34px; padding: 6px 13px; border: 0; border-radius: 7px; cursor: pointer; background: none; font: inherit; font-size: 0.83rem; font-weight: 600; color: var(--text-muted); @@ -371,3 +367,28 @@ /* A view that is one section is shown as prose: no heading repeating the tab above it, no chevron hiding the only thing there is to read. */ .asec-sole { padding-top: 4px; } + +/* Coming from Editorial to write one. The queue is where that person was + working; this page is a library, and without a way back the only route is + the browser's own button. */ +.articles-back { + display: inline-block; margin-bottom: 6px; + font-size: 0.82rem; font-weight: 600; color: var(--text-muted); text-decoration: none; +} +.articles-back:hover { color: var(--primary); } + +.articles-form-actions { display: flex; align-items: center; gap: 8px; margin-top: 14px; } +/* A link, not a third button: leaving is not the same weight as the two + decisions about the form in front of it. */ +.articles-leave { + margin-left: auto; font-size: 0.82rem; font-weight: 600; + color: var(--text-muted); text-decoration: none; +} +.articles-leave:hover { color: var(--primary); text-decoration: underline; } + +@media (max-width: 560px) { + /* Three controls on one line becomes two lines rather than three squeezed + ones, and the way out stops being pushed off the right edge. */ + .articles-form-actions { flex-wrap: wrap; } + .articles-leave { margin-left: 0; width: 100%; } +} diff --git a/frontend/src/pages/ArticlesPage.jsx b/frontend/src/pages/ArticlesPage.jsx index f28e396..039dbd9 100644 --- a/frontend/src/pages/ArticlesPage.jsx +++ b/frontend/src/pages/ArticlesPage.jsx @@ -12,7 +12,7 @@ import { resolveArticleId } from '../components/ArticleLink' import ArticleReader from '../components/ArticleReader' import ArticleSaveButton from '../components/ArticleSaveButton' import ArticleSplitPane from '../components/ArticleSplitPane' -import PractiseTopic from '../components/PractiseTopic' +import QBank from '../components/QBank' import SectionNote, { OrphanedNotes } from '../components/SectionNote' import ArticleFeedbackPanel from '../components/ArticleFeedbackPanel' import { plainWikiText } from '../components/RichText' @@ -39,6 +39,11 @@ export default function ArticlesPage() { const [params] = useSearchParams() const [showCreate, setShowCreate] = useState(params.get('new') === '1') const [showAi, setShowAi] = useState(params.get('draft') === '1') + // Whether this visit came from Editorial meaning to write. Read once, from + // the parameter that opened the form: somebody sent here to start an article + // needs the way back to the queue they came from, and cancelling a form is + // not the same as being finished with the errand. + const [cameToWrite] = useState(params.get('new') === '1' || params.get('draft') === '1') const [title, setTitle] = useState('') const [slug, setSlug] = useState('') const [error, setError] = useState('') @@ -97,6 +102,9 @@ export default function ArticlesPage() {
+ {cameToWrite && ( + ← Editorial + )}

Topic reading

Educator-reviewed articles linked to questions and cards.

@@ -116,9 +124,13 @@ export default function ArticlesPage() {