From 16aed6b6b0bb1386b6bdd9b5a6d729493a02cbd8 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 12 Sep 2026 06:24:23 +0200 Subject: [PATCH] feat: Cap on its own host, hints per topic, and an objective is asked for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cap moved from /cap/ under this app to cap.pedshub.com, so anything else on this machine can use the same instance. Caddy terminates it, the backend keeps verifying over the compose network rather than going out and back, and the widget endpoint is configuration rather than a path baked into the component. Verified: a challenge is issued on the subdomain, and a token that was never issued is still refused. "Correct using hints" is now a per-topic figure. The knowledge profile's accuracy bar was two-tone because /study-tools/recommendations carried only `answered` and `correct`; the hint count existed lifetime-wide but never per topic, and inferring one from the other would have been a different set of answers drawn as though it were this one. The column was already on attempt_answers, so it is a group-by, and the bar is three-tone as the reference has it. And the objective is asked for. It decides which questions exist, how relevance is weighted, and what readiness measures against — and it was possible to sit a whole board paper without ever being asked, because no objective quietly means the entire bank. That is a reasonable default and a poor thing to arrive at by accident. Five of six accounts here had never set one. It can be declined: "everything" is a real answer, and trapping somebody behind a modal because a list failed to load would be worse than the gap it closes. Declining is still a choice made, which is the point. Also in this commit, from the exam-player work: Show answer in study mode that reveals without recording an answer, review keyed on the attempt being closed rather than every question being answered — a block that timed out with nothing answered is over too — and the exam top and bottom bars. That work found something worth knowing: the exam player is *served* questions with no correct answer and no explanation, so review cannot un-hide what it never had, and the player refetches the marked version once the attempt closes. Nothing is revealed while a block is running. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN --- backend/app/routers/study_tools.py | 11 +- docker-compose.yml | 6 +- frontend/docker-entrypoint.sh | 3 +- frontend/nginx.conf | 19 +- frontend/src/App.jsx | 5 + frontend/src/components/Captcha.jsx | 14 +- frontend/src/components/Captcha.test.jsx | 14 +- frontend/src/components/ChooseObjective.css | 32 +++ frontend/src/components/ChooseObjective.jsx | 97 +++++++ .../src/components/ChooseObjective.test.jsx | 66 +++++ frontend/src/pages/AnalysisPage.css | 4 + frontend/src/pages/AnalysisPage.jsx | 15 +- frontend/src/pages/QuizPage.jsx | 261 ++++++++++++++---- frontend/src/pages/QuizPage.test.jsx | 153 +++++++++- frontend/src/pages/QuizPlayer.css | 47 +++- 15 files changed, 641 insertions(+), 106 deletions(-) create mode 100644 frontend/src/components/ChooseObjective.css create mode 100644 frontend/src/components/ChooseObjective.jsx create mode 100644 frontend/src/components/ChooseObjective.test.jsx diff --git a/backend/app/routers/study_tools.py b/backend/app/routers/study_tools.py index ea43b77..93d774b 100644 --- a/backend/app/routers/study_tools.py +++ b/backend/app/routers/study_tools.py @@ -552,6 +552,7 @@ def study_recommendations( # ── What the learner has answered ────────────────────────────── answered_rows = db.query( AttemptAnswer.question_id, AttemptAnswer.is_correct, Question.question_category_id, + AttemptAnswer.used_hint, ).join(QuizAttempt, QuizAttempt.id == AttemptAnswer.attempt_id ).join(Quiz, Quiz.id == QuizAttempt.quiz_id ).join(Question, Question.id == AttemptAnswer.question_id @@ -571,15 +572,20 @@ def study_recommendations( answered: dict[int, int] = defaultdict(int) correct: dict[int, int] = defaultdict(int) + # Right after opening a tip. Counted as correct, because it was, and kept + # apart so a topic can show how much of its score leaned on one. + hinted: dict[int, int] = defaultdict(int) seen_questions: dict[int, set[int]] = defaultdict(set) total_answers = len(answered_rows) - total_correct = sum(1 for _, is_correct, _ in answered_rows if is_correct) - for question_id, is_correct, primary in answered_rows: + total_correct = sum(1 for _, is_correct, _, _ in answered_rows if is_correct) + for question_id, is_correct, primary, used_hint in answered_rows: for key in groups_for(question_id, primary): answered[key] += 1 seen_questions[key].add(question_id) if is_correct: correct[key] += 1 + if used_hint: + hinted[key] += 1 # ── How much bank material each group holds ──────────────────── available: dict[int, int] = defaultdict(int) @@ -667,6 +673,7 @@ def study_recommendations( "key": key, "answered": n, "correct": c, + "correct_with_hints": hinted.get(key, 0), "seen_questions": seen, "available": pool, "coverage": coverage, diff --git a/docker-compose.yml b/docker-compose.yml index a1e3747..3be721d 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -80,10 +80,14 @@ services: # so a flush of one cannot clear the other's challenges. cap: image: tiago2/cap:latest + ports: + # Its own host, so other sites on this machine can use it too — which is + # the point of self-hosting it rather than pathing it under one app. + - "127.0.0.1:8093:3000" environment: ADMIN_KEY: ${CAP_ADMIN_KEY} REDIS_URL: redis://redis:6379/3 - CORS_ORIGIN: ${APP_URL:-https://pedshub.com} + CORS_ORIGIN: ${CAP_CORS_ORIGIN:-https://pedshub.com} SERVER_PORT: 3000 depends_on: - redis diff --git a/frontend/docker-entrypoint.sh b/frontend/docker-entrypoint.sh index ee6f53d..4ba48e9 100755 --- a/frontend/docker-entrypoint.sh +++ b/frontend/docker-entrypoint.sh @@ -3,7 +3,8 @@ # This avoids baking secrets/keys into the Docker image at build time. cat > /usr/share/nginx/html/config.js </challenge and answering NOT_FOUND. - rewrite ^/cap/(.*)$ /$1 break; - proxy_pass $cap; - proxy_set_header Host $host; - proxy_set_header X-Real-IP $remote_addr; - proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; - proxy_set_header X-Forwarded-Proto $scheme; - } - # API proxy to backend location /api/ { resolver 127.0.0.11 valid=10s; diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index f2ec8c7..0d38240 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -5,6 +5,7 @@ import { SessionDrawerProvider } from './context/SessionDrawer' import { ThemeProvider } from './context/ThemeContext' import Navbar from './components/Navbar' import SiteFooter from './components/SiteFooter' +import ChooseObjective from './components/ChooseObjective' import ErrorBoundary from './components/ErrorBoundary' import lazyPage from './utils/lazyPage' @@ -66,6 +67,10 @@ function AppLayout() { is a spinner, and the footer used to sit halfway up with the page background showing beneath it. */
+ {/* Asked of anybody who has never answered it. No objective quietly + means the whole bank, which is a reasonable default and a poor thing + to arrive at without being asked. */} +
diff --git a/frontend/src/components/Captcha.jsx b/frontend/src/components/Captcha.jsx index 6f02671..0580923 100644 --- a/frontend/src/components/Captcha.jsx +++ b/frontend/src/components/Captcha.jsx @@ -22,6 +22,11 @@ export function captchaSiteKey() { return window.__APP_CONFIG__?.CAP_SITE_KEY || '' } +/** Where Cap answers. Its own host, so other sites can use the same one. */ +export function captchaOrigin() { + return window.__APP_CONFIG__?.CAP_API_URL || '' +} + /** * The captcha on the forms strangers can reach — sign-up and contact. * @@ -71,12 +76,11 @@ export default function Captcha({ onVerify }) { const siteKey = captchaSiteKey() if (!siteKey) return null - // Same origin, proxied to the cap service by nginx. The custom element is - // rendered whether or not its script has arrived yet; it upgrades itself - // when the definition lands, and until then it is an empty box and the form - // stays unsubmittable, which is the honest outcome. + // The custom element is rendered whether or not its script has arrived yet; + // it upgrades itself when the definition lands, and until then it is an + // empty box and the form stays unsubmittable, which is the honest outcome. return ( + data-cap-api-endpoint={`${captchaOrigin().replace(/\/$/, '')}/${siteKey}/`} /> ) } diff --git a/frontend/src/components/Captcha.test.jsx b/frontend/src/components/Captcha.test.jsx index 6f8585e..439c4ab 100644 --- a/frontend/src/components/Captcha.test.jsx +++ b/frontend/src/components/Captcha.test.jsx @@ -6,7 +6,10 @@ const script = () => document.getElementById('cap-widget-script') const widget = () => document.querySelector('cap-widget') beforeEach(() => { - window.__APP_CONFIG__ = { CAP_SITE_KEY: 'configured-test-site-key' } + window.__APP_CONFIG__ = { + CAP_SITE_KEY: 'configured-test-site-key', + CAP_API_URL: 'https://cap.example.test', + } }) afterEach(() => { delete window.__APP_CONFIG__ @@ -30,12 +33,13 @@ it('loads the pinned widget once, however many are on the page', () => { expect(script().src).toBe('https://cdn.jsdelivr.net/npm/@cap.js/widget@0.1.56') }) -it('points the widget at this origin, not at anybody else', () => { +it('points the widget at our own Cap, not at anybody else', () => { render() - // Same-origin through nginx, which is the whole reason for self-hosting: - // nothing about the person signing up reaches a third party. + // Our host, which is the whole reason for self-hosting: nothing about the + // person signing up reaches a third party. Its own subdomain rather than a + // path under this app, so other sites can use the same one. expect(widget().getAttribute('data-cap-api-endpoint')) - .toBe('/cap/configured-test-site-key/') + .toBe('https://cap.example.test/configured-test-site-key/') }) it('hands the solved token up', () => { diff --git a/frontend/src/components/ChooseObjective.css b/frontend/src/components/ChooseObjective.css new file mode 100644 index 0000000..8fda2bd --- /dev/null +++ b/frontend/src/components/ChooseObjective.css @@ -0,0 +1,32 @@ +.co-overlay { + position: fixed; inset: 0; z-index: 200; display: grid; place-items: center; + padding: 20px; background: rgba(15, 23, 42, 0.55); +} +.co-card { + width: min(520px, 100%); max-height: 90vh; overflow-y: auto; + padding: 26px 24px; border-radius: 16px; + background: var(--card-bg); border: 1px solid var(--border); + box-shadow: 0 24px 60px rgba(15, 23, 42, 0.3); +} +.co-card h2 { margin: 0 0 8px; font-size: 1.3rem; font-weight: 700; } +.co-lead { margin: 0 0 18px; font-size: 0.88rem; line-height: 1.6; color: var(--text-muted); } +.co-error { margin: 0 0 12px; font-size: 0.85rem; color: var(--wrong-fg); } + +.co-list { list-style: none; margin: 0 0 16px; padding: 0; display: flex; flex-direction: column; gap: 8px; } +.co-list button { + display: flex; flex-direction: column; gap: 3px; width: 100%; + padding: 13px 15px; text-align: left; cursor: pointer; + background: var(--bg); color: var(--text); + border: 1px solid var(--border); border-radius: 11px; font: inherit; +} +.co-list button:hover:not(:disabled) { border-color: var(--primary); background: var(--option-sel-bg); } +.co-list button:disabled { opacity: 0.6; cursor: default; } +.co-list strong { font-size: 0.95rem; font-weight: 650; } +.co-list small { font-size: 0.78rem; color: var(--text-muted); } + +/* Quieter than the choices, because it is one — but not the one being urged. */ +.co-later { + width: 100%; padding: 10px; font: inherit; font-size: 0.84rem; cursor: pointer; + background: none; color: var(--text-muted); border: 0; +} +.co-later:hover:not(:disabled) { color: var(--primary); text-decoration: underline; } diff --git a/frontend/src/components/ChooseObjective.jsx b/frontend/src/components/ChooseObjective.jsx new file mode 100644 index 0000000..57ae83d --- /dev/null +++ b/frontend/src/components/ChooseObjective.jsx @@ -0,0 +1,97 @@ +import { useEffect, useState } from 'react' +import api from '../api/client' +import './ChooseObjective.css' + +const DEFERRED = 'pedshub.objectiveDeferred' + +/** + * Asked once, of anybody who has not answered it. + * + * The study objective decides which questions exist, how relevance is + * weighted, and what the analysis is measuring against — and it was possible + * to sit a whole board paper without ever being asked for one, because no + * objective quietly means "the entire bank". That is a reasonable default and + * a terrible thing to arrive at by accident. + * + * So it is a question rather than a setting to discover. It can be declined — + * "everything" is a real answer, and trapping somebody behind a modal because + * a list failed to load would be worse than the gap it closes — but declining + * is a choice made, which is the whole point. + */ +export default function ChooseObjective() { + const [exams, setExams] = useState(null) + const [busy, setBusy] = useState(false) + const [error, setError] = useState('') + const [dismissed, setDismissed] = useState(() => { + try { return sessionStorage.getItem(DEFERRED) === 'true' } catch { return false } + }) + + useEffect(() => { + if (dismissed) return undefined + let live = true + api.get('/exams/') + .then(res => { + if (!live) return + // Already answered: nothing to ask. + if (res.data?.active_exam_id) { setExams([]); return } + setExams(res.data?.exams || []) + }) + .catch(() => { if (live) setExams([]) }) + return () => { live = false } + }, [dismissed]) + + const choose = async (examId) => { + setBusy(true); setError('') + try { + await api.put('/exams/active', { exam_id: examId }) + // A hard reload rather than a state update: the objective scopes almost + // every query on the page, and half a screen answering the old question + // is worse than a second of waiting. + window.location.reload() + } catch { + setError('Could not save that. Try again, or pick it later from the bar at the top.') + setBusy(false) + } + } + + const later = () => { + try { sessionStorage.setItem(DEFERRED, 'true') } catch { /* private browsing */ } + setDismissed(true) + } + + if (dismissed || exams === null || exams.length === 0) return null + + return ( +
+
+

What are you studying for?

+

+ It decides which questions you are shown, how much each topic is + worth, and what your readiness is measured against. You can change it + any time from the bar at the top. +

+ + {error &&

{error}

} + +
    + {exams.map(exam => ( +
  • + +
    • + ))} +
+ + +
+
+ ) +} diff --git a/frontend/src/components/ChooseObjective.test.jsx b/frontend/src/components/ChooseObjective.test.jsx new file mode 100644 index 0000000..7902659 --- /dev/null +++ b/frontend/src/components/ChooseObjective.test.jsx @@ -0,0 +1,66 @@ +import { render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { beforeEach, expect, it, vi } from 'vitest' +import ChooseObjective from './ChooseObjective' +import api from '../api/client' + +vi.mock('../api/client', () => ({ default: { get: vi.fn(), put: vi.fn() } })) + +const EXAMS = [ + { id: 1, name: 'Pediatrics Boards', question_count: 2924 }, + { id: 2, name: 'USMLE Step 2 CK', question_count: 0 }, +] + +const answer = (activeExamId) => api.get.mockResolvedValue({ + data: { active_exam_id: activeExamId, exams: EXAMS }, +}) + +beforeEach(() => { + vi.clearAllMocks() + sessionStorage.clear() + api.put.mockResolvedValue({ data: {} }) +}) + +it('asks anybody who has never answered', async () => { + answer(null) + render() + expect(await screen.findByRole('dialog')).toBeInTheDocument() + expect(screen.getByRole('button', { name: /Pediatrics Boards/ })).toBeInTheDocument() + // An objective with nothing behind it says so rather than looking ready. + expect(screen.getByRole('button', { name: /USMLE Step 2 CK/ })) + .toHaveTextContent('No questions filed under it yet') +}) + +it('does not ask somebody who has already answered', async () => { + answer(1) + render() + await waitFor(() => expect(api.get).toHaveBeenCalled()) + expect(screen.queryByRole('dialog')).not.toBeInTheDocument() +}) + +it('saves the choice', async () => { + answer(null) + render() + await userEvent.click(await screen.findByRole('button', { name: /Pediatrics Boards/ })) + await waitFor(() => expect(api.put).toHaveBeenCalledWith('/exams/active', { exam_id: 1 })) +}) + +it('lets somebody decline, and does not ask again this session', async () => { + answer(null) + const { unmount } = render() + // "Everything" is a real answer, and trapping somebody behind a modal is + // worse than the gap it closes. + await userEvent.click(await screen.findByRole('button', { name: /Show me everything/ })) + expect(screen.queryByRole('dialog')).not.toBeInTheDocument() + + unmount() + render() + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()) +}) + +it('stays out of the way when the list cannot be fetched', async () => { + api.get.mockRejectedValue(new Error('down')) + render() + await waitFor(() => expect(api.get).toHaveBeenCalled()) + expect(screen.queryByRole('dialog')).not.toBeInTheDocument() +}) diff --git a/frontend/src/pages/AnalysisPage.css b/frontend/src/pages/AnalysisPage.css index eafd858..6f469a3 100644 --- a/frontend/src/pages/AnalysisPage.css +++ b/frontend/src/pages/AnalysisPage.css @@ -231,3 +231,7 @@ @media (max-width: 900px) { .an-readiness { grid-template-columns: minmax(0, 1fr); } } + +/* Right after a tip: the same family as right, lighter, because it is the + same answer arrived at with help. */ +.an-bar-split .is-hinted, .an-detail-legend i.is-hinted { background: var(--correct-bd); } diff --git a/frontend/src/pages/AnalysisPage.jsx b/frontend/src/pages/AnalysisPage.jsx index 595f673..388abe5 100644 --- a/frontend/src/pages/AnalysisPage.jsx +++ b/frontend/src/pages/AnalysisPage.jsx @@ -325,6 +325,9 @@ function FocusRow({ row, onPractise }) { const [open, setOpen] = useState(false) const seenShare = row.available ? Math.min(100, (row.seen_questions / row.available) * 100) : 0 const correctShare = row.answered ? (row.correct / row.answered) * 100 : 0 + // Right after a tip is part of correct, drawn as a slice of it rather than + // beside it, so the two bars still add to what was answered. + const hintedShare = row.answered ? ((row.correct_with_hints || 0) / row.answered) * 100 : 0 const accuracyLabel = row.answered ? `${row.accuracy}% (${row.correct} out of ${row.answered})` : 'Not attempted' @@ -382,21 +385,21 @@ function FocusRow({ row, onPractise }) { material into it would make a perfect score on a tenth of the questions look like a poor one. - The reference splits it three ways — right, right after a tip, - wrong. Per-topic hint counts are not in the recommendations - payload, only `answered` and `correct`, so the middle slice is - left undrawn rather than inferred from the lifetime figure, - which is about a different set of answers. */} + Three ways: right, right after opening a tip, wrong. The + middle one counts as correct — it was — and is drawn apart so + a topic can show how much of its score leaned on a nudge. */}
{row.answered ? ( <> - + + ) : }
  • Correct
    • + {hintedShare > 0 &&
  • Correct using hints
    • }
  • Incorrect
diff --git a/frontend/src/pages/QuizPage.jsx b/frontend/src/pages/QuizPage.jsx index 4195cd1..8b95c4a 100644 --- a/frontend/src/pages/QuizPage.jsx +++ b/frontend/src/pages/QuizPage.jsx @@ -215,6 +215,15 @@ const clock = (seconds) => { return `${String(Math.floor(s / 60)).padStart(2, '0')}:${String(s % 60).padStart(2, '0')}` } +// Hours as well as minutes, for the one figure a candidate looks at most. A +// block clock reading 89:00 is a different amount of time depending on how +// long you thought the block was, and it is read at a glance. +const blockClock = (seconds) => { + const s = Math.max(0, Math.round(seconds)) + return [Math.floor(s / 3600), Math.floor((s % 3600) / 60), s % 60] + .map(part => String(part).padStart(2, '0')).join(':') +} + /** * Session time, time on this question, and the running average. * @@ -450,6 +459,12 @@ export default function QuizPage() { const [timeUp, setTimeUp] = useState(false) const [afterTimeUp, setAfterTimeUp] = useState(null) const timeUpRef = useRef(false) + // The attempt is closed: it was handed in, or the clock ran out and it was + // handed in for you. This is what "finished" means. It used to be read off + // the answer count, which called a block finished only once every question + // had an answer — an exam that ran out with nothing answered is just as + // over, and that was the one case the old reading got wrong. + const [attemptClosed, setAttemptClosed] = useState(false) // Seconds spent on each question, banked when you leave it. Without this the // analysis can report a total but never a per-question time. const [questionTimes, setQuestionTimes] = useState({}) @@ -472,6 +487,10 @@ export default function QuizPage() { const [statsError, setStatsError] = useState('') const [showStats, setShowStats] = useState(() => localStorage.getItem('pedshub_show_stats') !== '0') const [showAllExplanations, setShowAllExplanations] = useState(false) + // Questions whose answer was asked for rather than given. Being shown the + // answer is not answering: these stay out of `answers`, so the rail, the + // count and what is handed in all still say the question is unanswered. + const [revealed, setRevealed] = useState(() => new Set()) // Which options have had their reasoning opened by clicking them. Separate // from the show-all toggle so one does not fight the other. const [openExplanations, setOpenExplanations] = useState(() => new Set()) @@ -873,6 +892,26 @@ const timerStarted = timeLeft !== null handleSubmit(true) }, [timeLeft]) + /** + * A closed block is served with its answers; an open one never is. + * + * The exam player is sent questions with no correct option and no + * explanation — that is the integrity rule, and it is enforced on the server + * rather than by hiding what the page already holds. So review cannot simply + * un-hide anything: once the attempt is completed the same request returns + * the marked version, and this asks for it. A study session was served the + * answers at the start, so there is nothing here for it to fetch. + */ + useEffect(() => { + if (!attemptClosed || !attemptId) return undefined + if (questions.some(question => question.correct_answer)) return undefined + let live = true + api.get(`/quizzes/${id}?attempt_id=${attemptId}`) + .then(res => { if (live && res.data) setQuiz(res.data) }) + .catch(() => { /* The unmarked questions stay on screen, which is honest */ }) + return () => { live = false } + }, [attemptClosed, attemptId, id]) + const saveProgressNow = useCallback((overrides = {}) => { if (!attemptId || !quizMode) return Promise.resolve() return api.post('/attempts/progress', { @@ -964,13 +1003,41 @@ const timerStarted = timeLeft !== null const setAnswer = (questionId, value) => setAnswers(prev => ({ ...prev, [questionId]: value })) // Reset one question rather than the whole attempt: a misclick should cost - // the answer you just gave, not the nineteen before it. + // the answer you just gave, not the nineteen before it. A question you only + // asked to see the answer to is put back the same way, because otherwise one + // press would close it for the rest of the session. const resetQuestion = (questionId) => { setAnswers(prev => { const next = { ...prev } delete next[questionId] return next }) + setRevealed(prev => { + if (!prev.has(questionId)) return prev + const next = new Set(prev) + next.delete(questionId) + return next + }) + setTyped('') + } + + /** + * Show me the answer. + * + * The other direction from resetQuestion, and the same shape: it opens one + * question's answer where that one closes it again. Until now the only way + * to read an explanation was to choose an option, so a learner who was stuck + * had to guess first — and a guess entered to unlock the explanation is a + * wrong answer in the score, in the rail and in every figure the analysis + * draws afterwards. Asking is not answering, so nothing is recorded. + */ + const revealAnswer = (questionId) => { + setRevealed(prev => { + if (prev.has(questionId)) return prev + const next = new Set(prev) + next.add(questionId) + return next + }) setTyped('') } @@ -997,9 +1064,14 @@ const timerStarted = timeLeft !== null * on every question. Clicking an option marks it: green if it was right, red * if it was not, with the explanation. Exam mode records it and moves on * when you do. + * + * A question whose answer is already on screen takes no more answers: after + * the block is closed, and after Show answer, there is nothing left to + * decide and anything recorded now would be a copy rather than a response. */ const chooseAnswer = value => { - if (!current || (isStudy && answers[current.id])) return + if (!current || attemptClosed) return + if (isStudy && (answers[current.id] || revealed.has(current.id))) return setAnswer(current.id, value) } @@ -1024,21 +1096,23 @@ const timerStarted = timeLeft !== null // not, so a typed answer is held until Enter or leaving the field. const [typed, setTyped] = useState('') const commitTyped = () => { - if (!current || !typed.trim()) return - if (isStudy && answers[current.id]) return + if (!current || !typed.trim() || attemptClosed) return + if (isStudy && (answers[current.id] || revealed.has(current.id))) return setAnswer(current.id, typed.trim()) } useEffect(() => { let active = true setResponseStats(null); setStatsError('') - if (isStudy && attemptId && current && answers[current.id]) { + // What everybody else picked is worth reading whether the answer was given + // or asked for — it is the same page of feedback either way. + if (isStudy && attemptId && current && (answers[current.id] || revealed.has(current.id))) { api.get(`/study-tools/attempts/${attemptId}/questions/${current.id}/responses`) .then(r => { if (active) setResponseStats(r.data) }) .catch(() => { if (active) setStatsError('Response statistics are unavailable.') }) } return () => { active = false } - }, [isStudy, attemptId, current?.id, answers[current?.id]]) + }, [isStudy, attemptId, current?.id, answers[current?.id], revealed.has(current?.id)]) const clearCurrentHighlights = () => { if (!current || !manualHighlights[current.id]) return @@ -1107,6 +1181,10 @@ const timerStarted = timeLeft !== null } const res = await api.post(`/attempts/${attemptId}/submit`, submission) clearInterval(timerRef.current) + // Handed in, so the block is closed and the player may show its answers. + // Set here rather than when the clock hit zero because the server only + // reveals a completed attempt, and it is not completed until this returns. + setAttemptClosed(true) api.delete(`/attempts/progress/${attemptId}`).catch(() => {}) const target = returnTo // A course quiz reports back to its course, and has no analysis of its @@ -1238,9 +1316,20 @@ const timerStarted = timeLeft !== null const answeredCount = Object.keys(answers).length const totalCount = questions.length const isLast = currentIdx === totalCount - 1 - // Whether the answer is in. Category and difficulty are hints, so they wait - // for this; in exam mode nothing is revealed until the whole test is over. - const answerRevealed = isStudy && !!answers[current?.id] + // Reading the block back rather than sitting it. There is nothing left to + // protect once it is closed, so a finished exam reads like study mode: the + // rail says what each question was, and the answers are on the page. + const reviewing = attemptClosed + // Whether this question's answer is on screen — given, asked for with Show + // answer, or open to everybody because the block is over. Category and + // difficulty are hints, so they wait for it too; while an exam is still + // being sat this is false for every question, which is the whole rule. + const answerRevealed = + reviewing || (isStudy && (!!answers[current?.id] || revealed.has(current?.id))) + // The block chrome — item and block counters, the countdown, Pause and End + // Block — belongs to a block being sat. Study mode never had any of it, and + // a closed block has nothing left to pause or hand in. + const examChrome = !isStudy && !reviewing const quizNavigation = (position = 'bottom') => (
@@ -1305,9 +1396,6 @@ const timerStarted = timeLeft !== null } } - // Handed in: the block is over, so the rail may say what each question was. - const reviewing = answeredCount >= totalCount && totalCount > 0 - const QuestionRailItem = ({ q, i }) => { const isActive = i === currentIdx const isDone = !!answers[q.id] @@ -1323,7 +1411,7 @@ const timerStarted = timeLeft !== null const excerpt = seen ? questionStem(q).replace(/\s+/g, ' ').trim() : '' return (
- {timeLeft !== null && } + {/* One clock. While a block is being sat the countdown lives in + the bar at the foot of the screen, where a paper puts it; this + is what is left for anything else that runs to a limit. */} + {timeLeft !== null && !examChrome && } {/* Suspend, Restart and Edit were three buttons above a question nobody was looking away from to press them. Exit is in the bar at the bottom, where the session's own controls are; restarting @@ -1535,11 +1630,9 @@ const timerStarted = timeLeft !== null {drawerTab === 'questions' ? ( <>
- {/* Everything answered means there is nothing left to sit; + {/* The block is closed, so there is nothing left to sit; what you are doing now is reading it back. */} - {answeredCount >= totalCount && totalCount > 0 && ( - Review - )} + {reviewing && Review} {isStudy ? 'Study mode' : 'Exam mode'}: {quiz.title} {answeredCount}/{totalCount}
@@ -1960,20 +2092,33 @@ const timerStarted = timeLeft !== null {/* The session's own bar, outside the scrolling columns so it is always on screen — the player is a fixed-height shell and the question scrolls inside it, rather than the whole page scrolling. */} -
- - {quizNavigation('bottom')} - {/* Reviewing a block before handing it in is an exam idea. A study - session has nothing to hand in: it keeps going until every - question is answered, and at that point it is the review. This - button sat in both because the study player inherited the exam - player's bar. */} - {!isStudy && answeredCount > 0 && ( - +
+ {examChrome ? ( + <> +
+ + {/* The block's own clock, and the only one: a second countdown + elsewhere on the screen is a second chance to misread it. An + untimed block says so rather than showing an empty space + where the figure everybody looks for should be. */} + + Block Time Remaining: {timeLeft === null ? 'Untimed' : blockClock(timeLeft)} + +
+ {/* Pause is the exam's own pause — the same flag the clocks read, + and the same Exam Paused dialog that covers the questions. */} + + + + ) : ( + <> + + {quizNavigation('bottom')} + )}
diff --git a/frontend/src/pages/QuizPage.test.jsx b/frontend/src/pages/QuizPage.test.jsx index fa3f790..73c1ca8 100644 --- a/frontend/src/pages/QuizPage.test.jsx +++ b/frontend/src/pages/QuizPage.test.jsx @@ -64,6 +64,10 @@ const findStem = async (text) => { return waitFor(() => inCard().getByText(text)) } +/** The one control that ends a block: the End Block in the block's own bar, + * not the one in the dialog it opens. */ +const endBlock = () => within(document.querySelector('.quiz-footbar')).getByRole('button', { name: 'End Block' }) + async function begin(study = true) { quizModeVar = study ? 'learning' : 'timed' mount() @@ -288,6 +292,60 @@ describe('quiz player', () => { expect(screen.queryByRole('button', { name: /Rule out option/ })).toBeNull() }) + it('shows the answer to a learner who asks, without answering for them', async () => { + await begin() + expect(screen.queryByText(/Full explanation/)).not.toBeInTheDocument() + await userEvent.click(screen.getByRole('button', { name: 'Show answer' })) + + // The correct option and the explanation, the same as they would be after + // a right answer — being stuck is a reason to read them, not to guess. + expect(await screen.findByText(/Full explanation, preserved without shortening/)).toBeInTheDocument() + expect(inCard().getByText('First answer').closest('.option')).toHaveClass('correct') + // Once it is showing there is nothing left to offer. + expect(screen.queryByRole('button', { name: 'Show answer' })).toBeNull() + + // Nothing was answered by it: not in the rail, not in the count, and not + // in what is handed in. + expect(document.querySelectorAll('.quiz-rail-item')[0].className).not.toMatch(/is-done/) + expect(within(document.querySelector('.quiz-header-card')).getByText('0 answered')).toBeInTheDocument() + // Nor can an option be picked afterwards — that would be copying, not + // answering, and it would count. + await userEvent.click(inCard().getByText('Second answer').closest('.option')) + expect(document.querySelector('.option.selected')).toBeNull() + + await userEvent.click(screen.getAllByRole('button', { name: /Next/ })[0]) + await findStem('Full second clinical question.') + await userEvent.click(screen.getByRole('button', { name: 'Finish session' })) + await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'End Block' })) + // Handing in nothing is announced before it happens, so this waits out + // that message rather than racing it. + expect(await screen.findByText('Submitted results', {}, { timeout: 3000 })).toBeInTheDocument() + expect(api.post).toHaveBeenCalledWith('/attempts/50/submit', expect.objectContaining({ answers: [] })) + }) + + it('puts a question shown back the way it puts an answered one back', async () => { + await begin() + await userEvent.click(screen.getByRole('button', { name: 'Show answer' })) + await screen.findByText(/Full explanation, preserved without shortening/) + // Reset is the other direction from Show answer, so one press must not + // close a question for the rest of the session. + await userEvent.click(screen.getByRole('button', { name: /Reset question/ })) + expect(screen.queryByText(/Full explanation/)).not.toBeInTheDocument() + expect(screen.getByRole('button', { name: 'Show answer' })).toBeInTheDocument() + fireEvent.keyDown(window, { key: '1' }) + expect(await screen.findByText(/Full explanation, preserved without shortening/)).toBeInTheDocument() + }) + + it('offers no way to see the answer while an exam is being sat', async () => { + await begin(false) + // The exam is the one place where being shown the answer is not on offer, + // and the server does not send it either. + expect(screen.queryByRole('button', { name: 'Show answer' })).toBeNull() + fireEvent.keyDown(window, { key: '1' }) + expect(screen.queryByRole('button', { name: 'Show answer' })).toBeNull() + expect(screen.queryByText(/Full explanation/)).not.toBeInTheDocument() + }) + it('marks a study answer the moment it is chosen, and shows genuine response data', async () => { await begin() // Choosing is answering. It used to hold the choice and wait for "Submit @@ -465,7 +523,7 @@ describe('quiz player', () => { fireEvent.keyDown(window, { key: '1' }) expect(screen.queryByText(/Full explanation/)).not.toBeInTheDocument() expect(api.get.mock.calls.some(([url]) => url.startsWith('/study-tools/attempts/'))).toBe(false) - await userEvent.click(screen.getAllByRole('button', { name: 'Review & Complete' })[0]) + await userEvent.click(endBlock()) const review = screen.getByRole('dialog', { name: /This block is incomplete/ }) // The sentence names what is missing, not what is done — that is the // decision being confirmed. @@ -476,7 +534,7 @@ describe('quiz player', () => { await userEvent.click(screen.getAllByRole('button', { name: /Next/ })[0]) await findStem('Full second clinical question.') expect(api.post.mock.calls.some(([url]) => url === '/attempts/50/submit')).toBe(false) - await userEvent.click(screen.getAllByRole('button', { name: 'Review & Complete' })[0]) + await userEvent.click(endBlock()) await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'End Block' })) expect(await screen.findByText('Submitted results')).toBeInTheDocument() expect(api.post).toHaveBeenCalledWith('/attempts/50/submit', expect.objectContaining({ @@ -491,7 +549,7 @@ describe('quiz player', () => { await userEvent.click(await screen.findByRole('button', { name: 'stridor' })) expect(screen.getByRole('note')).toHaveTextContent('extrathoracic') fireEvent.keyDown(window, { key: '1' }) - await userEvent.click(screen.getAllByRole('button', { name: 'Review & Complete' })[0]) + await userEvent.click(endBlock()) await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'End Block' })) expect(await screen.findByText('Submitted results')).toBeInTheDocument() expect(api.post).toHaveBeenCalledWith('/attempts/50/submit', expect.objectContaining({ @@ -508,7 +566,7 @@ describe('quiz player', () => { await beginWithTip() fireEvent.keyDown(window, { key: '1' }) await userEvent.click(await screen.findByRole('button', { name: 'stridor' })) - await userEvent.click(screen.getAllByRole('button', { name: 'Review & Complete' })[0]) + await userEvent.click(endBlock()) await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'End Block' })) expect(await screen.findByText('Submitted results')).toBeInTheDocument() expect(api.post).toHaveBeenCalledWith('/attempts/50/submit', expect.objectContaining({ @@ -554,7 +612,12 @@ describe('quiz player', () => { it('an exam still offers it, because a block is handed in', async () => { await begin(false) fireEvent.keyDown(window, { key: '1' }) - expect(screen.getByRole('button', { name: /^Review \(1\/2\)/ })).toBeInTheDocument() + // One control, at the right-hand end of the block's own bar. It used to be + // three — the top bar, the rail and this bar — under two names for the + // same dialog. + expect(endBlock()).toBeInTheDocument() + expect(screen.queryByRole('button', { name: 'Review & Complete' })).toBeNull() + expect(screen.queryByRole('button', { name: /^Review \(/ })).toBeNull() }) it('a finished study session submits without asking twice', async () => { @@ -568,6 +631,84 @@ describe('quiz player', () => { expect(await screen.findByText('Submitted results')).toBeInTheDocument() }) + it('reviews a block that ran out of time, with nothing answered', async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }) + try { + timeLimit = 1 + await begin(false) + // Sitting it: a column of numbers, and no answer anywhere on the page. + expect(document.querySelectorAll('.quiz-rail-item')[0].className).toMatch(/is-numbers/) + expect(screen.queryByText('Review')).not.toBeInTheDocument() + + // The server hands back a completed attempt with its answers, which is + // what switching the fixture to the marked questions stands in for. + mode = 'study' + await act(async () => { vi.advanceTimersByTime(61_000) }) + expect(await screen.findByText(/You have run out of time/)).toBeInTheDocument() + // Nothing was answered and the block is still finished — the case + // "everything is answered" got wrong. + expect(api.post).toHaveBeenCalledWith('/attempts/50/submit', expect.objectContaining({ answers: [] })) + + expect(await screen.findByText('Review')).toBeInTheDocument() + const item = document.querySelectorAll('.quiz-rail-item')[0] + expect(item.className).not.toMatch(/is-numbers/) + expect(within(item).getByText('Full first clinical question.')).toBeInTheDocument() + expect(await screen.findByText(/Full explanation, preserved without shortening/)).toBeInTheDocument() + expect(inCard().getByText('First answer').closest('.option')).toHaveClass('correct') + // Nothing left to pause or hand in. + expect(screen.queryByRole('button', { name: 'End Block' })).toBeNull() + expect(screen.queryByRole('button', { name: 'Pause' })).toBeNull() + } finally { + vi.useRealTimers() + timeLimit = null + } + }) + + it('sits an exam between an item counter and a block bar', async () => { + timeLimit = 60 + try { + await begin(false) + const meta = document.querySelector('.quiz-block-meta') + expect(meta.textContent).toContain('Item: 1 of 2') + expect(meta.textContent).toContain('Block: 1 of 1') + + // Previous / n of m / Next in the middle, not tucked in beside the tools. + const nav = document.querySelector('.quiz-item-nav') + expect(within(nav).getByRole('button', { name: /Previous/ })).toBeDisabled() + expect(within(nav).getByText('1 / 2')).toBeInTheDocument() + await userEvent.click(within(nav).getByRole('button', { name: /Next/ })) + await findStem('Full second clinical question.') + expect(document.querySelector('.quiz-block-meta').textContent).toContain('Item: 2 of 2') + expect(within(document.querySelector('.quiz-item-nav')).getByRole('button', { name: /Next/ })).toBeDisabled() + // The tools kept their end of the bar. + expect(within(document.querySelector('.quiz-top-actions')).getByRole('button', { name: 'Lab values' })).toBeInTheDocument() + + // One clock, at the foot of the screen where a paper puts it. + const bar = document.querySelector('.quiz-footbar') + expect(within(bar).getByText(/Block Time Remaining/).textContent).toMatch(/\d\d:\d\d:\d\d/) + expect(document.querySelectorAll('.quiz-block-time')).toHaveLength(1) + + // Pause is the exam's own pause, and End Block its one confirmation. + await userEvent.click(within(bar).getByRole('button', { name: 'Pause' })) + expect(screen.getByRole('dialog', { name: 'Exam Paused' })).toBeInTheDocument() + await userEvent.click(screen.getByRole('button', { name: 'Return to exam' })) + await userEvent.click(endBlock()) + expect(screen.getByRole('dialog', { name: /This block is incomplete/ })).toBeInTheDocument() + } finally { + timeLimit = null + } + }) + + it('leaves the study bar alone — the block chrome is the exam\'s', async () => { + await begin() + expect(document.querySelector('.quiz-block-meta')).toBeNull() + expect(document.querySelector('.quiz-block-time')).toBeNull() + expect(screen.queryByRole('button', { name: 'Pause' })).toBeNull() + const bar = document.querySelector('.quiz-footbar') + expect(within(bar).getByRole('button', { name: 'Exit session' })).toBeInTheDocument() + expect(within(bar).getByRole('button', { name: /Next/ })).toBeInTheDocument() + }) + it('says so when five minutes are left, once', async () => { vi.useFakeTimers({ shouldAdvanceTime: true }) try { @@ -604,7 +745,7 @@ describe('quiz player', () => { if (url === '/attempts/50/submit' && !failed) { failed = true; return Promise.reject({ response: { data: { detail: 'Try again safely' } } }) } return originalPost(url, ...args) }) - await userEvent.click(screen.getAllByRole('button', { name: 'Review & Complete' })[0]) + await userEvent.click(endBlock()) await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'End Block' })) expect(await screen.findByRole('alert')).toHaveTextContent('Try again safely') expect(api.delete).not.toHaveBeenCalled() diff --git a/frontend/src/pages/QuizPlayer.css b/frontend/src/pages/QuizPlayer.css index fec220b..f739954 100644 --- a/frontend/src/pages/QuizPlayer.css +++ b/frontend/src/pages/QuizPlayer.css @@ -60,7 +60,15 @@ .quiz-top-actions { display: flex; gap: 8px; align-items: center; flex-wrap: wrap; justify-content: flex-end; } .quiz-top-actions button { font-size: .85rem; min-height: 42px; } .quiz-top-actions button[title] { background: transparent; } -.quiz-top-actions .quiz-review-button { background: #496fa5; color: white; font-weight: 650; text-transform: uppercase; padding: 12px 18px; margin-left: 12px; } +/* ── The block bars ─────────────────────────────────────────────────── + A paper says where you are before it says anything else, so the counters + come first, the arrows sit in the middle where the hand already is, and the + tools keep the right-hand end they have always had. */ +.quiz-block-meta { display: flex; flex-direction: column; gap: 2px; margin: 0; font-size: .82rem; color: #737982; white-space: nowrap; } +.quiz-block-meta strong { color: #30343a; font-weight: 650; } +.quiz-item-nav { display: flex; align-items: center; gap: 6px; } +.quiz-topbar .quiz-item-nav button { min-width: 92px; font-size: .85rem; min-height: 42px; } +.quiz-item-count { font-size: .9rem; font-weight: 650; color: #30343a; font-variant-numeric: tabular-nums; min-width: 58px; text-align: center; } .quiz-player button:focus-visible, .quiz-player a:focus-visible, .quiz-results a:focus-visible { outline: 3px solid #779ad1; outline-offset: 3px; } .quiz-player .question-card { border: 0; padding: 0; border-radius: 0; box-shadow: none; margin-bottom: 22px; } /* One compact meta strip — the category trail used to be a 78px block that @@ -116,6 +124,24 @@ .quiz-response-track { display: block; width: min(36vw, 340px); height: 8px; background: #edeef4; flex-shrink: 1; } .quiz-response-track > span { display: block; height: 100%; background: #444; } .option.correct .quiz-response-track > span { background: #71b298; } +/* ── Show answer ────────────────────────────────────────────────────── + Under the options, quiet enough that choosing one is still the obvious + thing to do, and beside a line saying what it costs — which is nothing. */ +.quiz-reveal { display: flex; align-items: center; flex-wrap: wrap; gap: 10px; padding: 14px 0 2px; } +.quiz-reveal-button { + background: none; border: 1px solid var(--primary); border-radius: 2px; + padding: 9px 18px; font: inherit; font-size: .85rem; font-weight: 650; + color: var(--primary); cursor: pointer; +} +.quiz-reveal-button:hover { background: var(--primary); color: #fff; } +.quiz-reveal span { font-size: .78rem; color: var(--text-muted); } + +/* Read back rather than sat, said in the header where the mode is. */ +.quiz-review-badge { + padding: 1px 8px; border-radius: 12px; font-weight: 700; letter-spacing: .06em; + text-transform: uppercase; font-size: .68rem; + background: #fdf6e8; color: #8a6417; border: 1px solid #eddfbe; +} .quiz-review-tabs { display: flex; flex-wrap: wrap; gap: 8px; align-items: center; border-bottom: 1px solid var(--border); padding: 10px 0; margin-top: 18px; } .quiz-review-tabs > span:first-child { background: #333; color: white; padding: 11px 24px; font-size: .85rem; font-weight: 600; } .quiz-review-tabs .quiz-source-page { color: #6f7886; padding: 10px; font-size: .84rem; } @@ -147,7 +173,8 @@ .quiz-top-actions button[title] span { display: none; } .quiz-top-actions button[aria-label="Next question"] { font-size: 0; } .quiz-top-actions button[aria-label="Next question"]::after { content: '›'; font-size: 1.25rem; } - .quiz-top-actions .quiz-review-button { padding: 9px; margin-left: 0; font-size: .73rem; } + .quiz-block-meta { font-size: .74rem; } + .quiz-topbar .quiz-item-nav button { min-width: 0; padding: 8px 10px; } .quiz-qmeta { padding: 10px 0 8px; } .quiz-breadcrumbs { font-size: .76rem; } .quiz-actionbar button span, .quiz-actionbar .manual-highlight-toolbar button span { display: none; } @@ -174,7 +201,6 @@ .quiz-player .quiz-header-card > div { gap: 10px; } .quiz-topbar { gap: 10px; } .quiz-nav-controls { gap: 8px; } -.quiz-review-button { margin-left: auto; } @media (max-width: 640px) { .quiz-topbar { flex-wrap: wrap; } .quiz-header-card { padding-bottom: 12px; } @@ -316,7 +342,20 @@ } .quiz-footbar .quiz-nav-controls { flex: 1; justify-content: center; margin: 0; } .quiz-exit { flex-shrink: 0; } -.quiz-review-link { flex-shrink: 0; } + +/* The block's bar: the clock at the left where it is read from, Pause in the + middle, and the one control that ends the block at the far right, away from + the hand that is moving between items. */ +.quiz-footbar.is-exam { justify-content: space-between; } +.quiz-footbar-left { display: flex; align-items: center; gap: 14px; min-width: 0; } +.quiz-block-time { font-size: .85rem; color: #737982; white-space: nowrap; } +.quiz-block-time strong { color: #30343a; font-weight: 700; font-variant-numeric: tabular-nums; font-size: .95rem; } +.quiz-block-pause { flex-shrink: 0; min-width: 96px; } +.quiz-block-end { + flex-shrink: 0; background: #496fa5; color: #fff; border: 1px solid #496fa5; + font-weight: 650; text-transform: uppercase; letter-spacing: .03em; padding: 10px 18px; +} +.quiz-block-end:hover:not(:disabled) { background: #365b8d; } /* Nothing else on the page while a session is being sat. */ body:has(.quiz-player.is-boxed) .site-footer { display: none; }