diff --git a/backend/app/routers/articles.py b/backend/app/routers/articles.py index 1381e8d..076693e 100644 --- a/backend/app/routers/articles.py +++ b/backend/app/routers/articles.py @@ -61,7 +61,6 @@ class ArticleReference(BaseModel): journal: str | None = Field(default=None, max_length=150) year: str | None = Field(default=None, max_length=10) pmid: str | None = Field(default=None, max_length=20) - url: str | None = Field(default=None, max_length=300) #: "Barski L, et al. Management of diabetic ketoacidosis. Eur J Intern Med. @@ -88,9 +87,7 @@ def _reference_json(entry) -> dict: pmid = found.group(1) if found else None return { "title": (_PMID_TAIL.sub("", line).strip() or line)[:300], - "author": None, "pages": [], - "pmid": pmid, - "url": f"https://pubmed.ncbi.nlm.nih.gov/{pmid}/" if pmid else None, + "author": None, "pages": [], "pmid": pmid, } diff --git a/backend/app/services/pubmed.py b/backend/app/services/pubmed.py index 9268842..775ff2d 100644 --- a/backend/app/services/pubmed.py +++ b/backend/app/services/pubmed.py @@ -225,5 +225,4 @@ def as_references(results: list[dict]) -> list[dict]: "journal": record["journal"] or None, "year": record["year"] or None, "pmid": record["pmid"], - "url": record["url"], } for record in results] diff --git a/backend/app/services/question_figures.py b/backend/app/services/question_figures.py index 53deb69..d54950d 100644 --- a/backend/app/services/question_figures.py +++ b/backend/app/services/question_figures.py @@ -22,14 +22,24 @@ def figure_json(link: QuestionMedia, asset: MediaAsset) -> dict: "id": link.id, "media_id": link.media_id, "role": link.role, - # The label the prose refers to. Falls back to a number so a figure is - # never nameless, which is what makes "see the figure" ambiguous. # An educator's label, or nothing. "Figure 1" and "Figure from question # #3360" told a learner only that an image was an image, and the second # one told them the internal path it came from as well. "label": link.label or None, - "caption": link.caption or getattr(asset, "caption", None), - "title": getattr(asset, "title", None), + # What this *question* says about the image, and nothing else. + # + # It used to fall back to the library's own description, which is + # written to catalogue an image, not to sit beside a question — so a + # radiograph attached to a stem about a limping child carried + # "abnormalities in the right hip joint" underneath it, and the + # question was answered before it was read. Every one of the 343 + # figures in the bank was inheriting that description; not one had a + # caption of its own. Most question figures want no caption at all, + # and the ones that do want words chosen for the question they are on. + # + # The library description is still the library's, and still shown + # there and in an article. It is not shown in a quiz. + "caption": link.caption or None, "path": getattr(asset, "path", None), "position": link.position, } diff --git a/backend/tests/test_question_figures.py b/backend/tests/test_question_figures.py new file mode 100644 index 0000000..a57ef87 --- /dev/null +++ b/backend/tests/test_question_figures.py @@ -0,0 +1,52 @@ +"""What a question says about its own figures — and what it must not say. + +Run: DATABASE_URL=sqlite:///:memory: PYTHONPATH=backend python -m unittest discover -s backend/tests +""" +import os +os.environ.setdefault("DATABASE_URL", "sqlite:///:memory:") + +import unittest + +from app.models.media import MediaAsset +from app.models.question_media import QuestionMedia +from app.services.question_figures import figure_json + + +class FigureCaptionTests(unittest.TestCase): + """A caption beside a stem is read before the question is answered. + + The library's description of an image is written to catalogue it — "an + X-ray of a child's pelvis and hips, showing abnormalities in the right hip + joint" is a perfectly good catalogue entry and a complete giveaway under a + stem about a limping five-year-old. Every one of the bank's 343 figures was + inheriting one; not one had a caption of its own. + """ + + def setUp(self): + self.asset = MediaAsset(id=5, path="uploads/hip.png", + title="Right SCFE, AP pelvis", + caption="An X-ray of a child's pelvis and hips, " + "showing abnormalities in the right hip joint.") + + def test_the_library_description_never_reaches_a_question(self): + link = QuestionMedia(id=1, question_id=9, media_id=5, role="stem", + label=None, caption=None, position=0) + figure = figure_json(link, self.asset) + self.assertIsNone(figure["caption"]) + self.assertIsNone(figure["label"]) + # Nor by another name: the catalogue title is a giveaway of its own, + # and nothing renders it. + self.assertNotIn("title", figure) + self.assertEqual(figure["path"], "uploads/hip.png") + + def test_what_the_question_itself_says_is_shown(self): + link = QuestionMedia(id=2, question_id=9, media_id=5, role="stem", + label="Figure 1", caption="AP pelvis at presentation.", + position=0) + figure = figure_json(link, self.asset) + self.assertEqual(figure["caption"], "AP pelvis at presentation.") + self.assertEqual(figure["label"], "Figure 1") + + +if __name__ == "__main__": + unittest.main() diff --git a/frontend/src/components/ArticleReader.jsx b/frontend/src/components/ArticleReader.jsx index f8f6cdf..b38d54e 100644 --- a/frontend/src/components/ArticleReader.jsx +++ b/frontend/src/components/ArticleReader.jsx @@ -295,10 +295,11 @@ export default function ArticleReader({ {' · '}{[ref.journal, ref.year].filter(Boolean).join(', ')} )} - {ref.pmid && ( - PMID {ref.pmid} - )} + {/* The number, not a link. A reference list is a list of what + was read, and every entry that leaves the site is an + invitation to leave mid-article — the PMID is enough for + anybody who wants to look it up. */} + {ref.pmid && PMID {ref.pmid}} ))} diff --git a/frontend/src/components/ImageFigure.jsx b/frontend/src/components/ImageFigure.jsx index 48f995f..83ddf57 100644 --- a/frontend/src/components/ImageFigure.jsx +++ b/frontend/src/components/ImageFigure.jsx @@ -30,7 +30,16 @@ import './ImageFigure.css' //: CSS, which cannot see a file extension, and given a width to work from. const isVector = (src) => /\.svg(\?|#|$)/i.test(String(src || '')) -export function ImageViewer({ src, alt = '', attemptId, onClose }) { +export function ImageViewer({ src, alt = '', attemptId, onClose, + // Whether what the *library* knows about the + // image is shown beside it. In a session it is + // not: a catalogue description is written to + // find an image again, not to sit next to a + // question, and "showing abnormalities in the + // right hip joint" answers the stem it + // illustrates. What a question says about its + // own figure is set on the question. + libraryInfo = true }) { const [asset, setAsset] = useState(null) const [marked, setMarked] = useState(false) @@ -64,14 +73,14 @@ export function ImageViewer({ src, alt = '', attemptId, onClose }) { }, [onClose]) useEffect(() => { - if (asset !== null || !src) return undefined + if (!libraryInfo || asset !== null || !src) return undefined let live = true api.get('/media/by-path', { params: { path: src } }) // `false` rather than null: asked and there is nothing, so do not ask again. .then(res => { if (live) setAsset(res.data || false) }) .catch(() => { if (live) setAsset(false) }) return () => { live = false } - }, [asset, src]) + }, [asset, src, libraryInfo]) const label = (alt || '').trim() const title = (asset && asset.title) || label @@ -123,6 +132,9 @@ export function ImageViewer({ src, alt = '', attemptId, onClose }) { } export default function ImageFigure({ src, alt = '', attemptId, className = '', + // Passed straight through; see the + // viewer. + libraryInfo = true, // Whether the alt text is also printed // under the thumbnail. On a card it is // not: the card's own words are the @@ -147,7 +159,8 @@ export default function ImageFigure({ src, alt = '', attemptId, className = '', {open && ( - setOpen(false)} /> + setOpen(false)} /> )} ) diff --git a/frontend/src/components/RichText.jsx b/frontend/src/components/RichText.jsx index 5f0912b..6037c16 100644 --- a/frontend/src/components/RichText.jsx +++ b/frontend/src/components/RichText.jsx @@ -94,7 +94,10 @@ export default function RichText({ // full size on a click. A 2,000px radiograph rendered inline was a wall of // greyscale in the middle of a sentence, and four megabytes to draw it. img: ({ node, src, alt, ...props }) => ( - + // Inside an attempt the library's description of the image is not shown: + // what a question says about its figure is set on the question, and + // `attemptId` is only ever passed by the player and the review. + ), // `{{phrase|tip}}` — a teaching point that opens where the phrase is. span: ({ node, children, ...props }) => ( diff --git a/frontend/src/pages/ArticlesPage.css b/frontend/src/pages/ArticlesPage.css index 06c4924..2b1b873 100644 --- a/frontend/src/pages/ArticlesPage.css +++ b/frontend/src/pages/ArticlesPage.css @@ -376,12 +376,12 @@ .article-ref-title { color: var(--text); font-weight: 600; } .article-ref-pages { font-variant-numeric: tabular-nums; } /* The checkable part of a paper. Set apart from the citation so the eye finds - it, and a link because it goes somewhere. */ + it — plain text, because a reference list is what was read, not a set of + doors out of the article. */ .article-ref-pmid { margin-left: 8px; font-size: .78rem; font-weight: 600; - color: var(--primary); text-decoration: none; white-space: nowrap; + color: var(--text-muted); white-space: nowrap; } -.article-ref-pmid:hover { text-decoration: underline; } @media (max-width: 900px) { /* Four things in a bar this narrow is three too many; the rail and the diff --git a/frontend/src/pages/QuizPage.jsx b/frontend/src/pages/QuizPage.jsx index 95fdbae..fdb7596 100644 --- a/frontend/src/pages/QuizPage.jsx +++ b/frontend/src/pages/QuizPage.jsx @@ -1599,8 +1599,12 @@ const timerStarted = timeLeft !== null
{/* The same burger that opened it closes it: on a phone this drawer is what that button does while a session is open. */} + {/* A cross, because the menu is open. The button that opened + it kept its ☰ while the drawer covered the screen, which + reads as a second menu to open rather than the way out of + the one you are looking at. */} + onClick={() => setNavOpen(false)}>✕
diff --git a/frontend/src/pages/ResultsPage.jsx b/frontend/src/pages/ResultsPage.jsx index 2569b52..dfb5ac2 100644 --- a/frontend/src/pages/ResultsPage.jsx +++ b/frontend/src/pages/ResultsPage.jsx @@ -114,8 +114,12 @@ export default function ResultsPage() {
e.target === e.currentTarget && setNavOpen(false)}>
+ {/* A cross, because the menu is open. The button that opened + it kept its ☰ while the drawer covered the screen, which + reads as a second menu to open rather than the way out of + the one you are looking at. */} + onClick={() => setNavOpen(false)}>✕