From 9d407ca1d9e7c5bb05373f8420de71fe1cef326c Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 11 Sep 2026 12:46:13 +0200 Subject: [PATCH] feat: settings as places with addresses; repair nested cross-references MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settings was one 600px column holding the account form, a theme picker, a Nextcloud integration, a document list and an admin grid, in that order, with no way to link to any of it. It is now a section list beside one panel, with the section in the URL — so "change your password" is a link and Back works. On a phone the list becomes a scrolling strip rather than a second level of navigation. - The exam objective moves in. It scopes the bank, the filters and now the knowledge profile, which makes it a setting; it was only reachable from a dropdown in the header. - The notifications panel is gone. Its one control switched quiz reminders, and the reminder scheduler was removed earlier today — it was a toggle wired to nothing. - Form fields are 16px on touch so iOS does not zoom the page in on focus and refuse to zoom back out; nav rows are 44px targets. Also fixed, found in an agent's report rather than by looking: 37 cross-references across 25 articles are nested and broken — `[[363|[[245|gastroesophageal reflux]] disease]]`, which renders as literal brackets and resolves to nothing. The first linker pass linked the longest title, then let a shorter one cut into the result. The current pass cannot do this (a finished marker is stashed), but the damage was already in the database and strip_owned could not see it: its label group stops at the first "]". link_articles now unwraps the inner marker, keeping the outer — the longer, more specific title. And the ArticleSplitView flake: the preview card appears on a 350ms timer and the query allowed 2s, which the full parallel run exceeded often enough to fail a different case each time. Tried fake timers first; they fight waitFor. A longer allowance is the honest fix — the test is about the split view, not about how fast the box is. Four consecutive clean runs. Frontend 274/274. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN --- backend/scripts/link_articles.py | 45 ++++- frontend/src/pages/ArticleSplitView.test.jsx | 6 +- frontend/src/pages/SettingsPage.css | 113 +++++++++++ frontend/src/pages/SettingsPage.jsx | 187 ++++++++++--------- frontend/src/pages/SettingsPage.test.jsx | 85 +++++++++ 5 files changed, 336 insertions(+), 100 deletions(-) create mode 100644 frontend/src/pages/SettingsPage.css create mode 100644 frontend/src/pages/SettingsPage.test.jsx diff --git a/backend/scripts/link_articles.py b/backend/scripts/link_articles.py index 352a3c6..59ffe1a 100644 --- a/backend/scripts/link_articles.py +++ b/backend/scripts/link_articles.py @@ -72,6 +72,30 @@ def word_pattern(title: str) -> re.Pattern: return re.compile(rf"(? tuple[str, int]: + """Unwrap the inner marker, keeping the outer. + + The outer is the longer, more specific title — "Gastroesophageal reflux + disease" over "Gastroesophageal reflux" — so it is the link worth keeping, + and the inner one's label becomes plain words again. + """ + if not text: + return text, 0 + fixed = 0 + while True: + text, count = NESTED.subn(lambda m: f"[[{m.group(1)}|{m.group(2)}{m.group(3)}{m.group(4)}]]", text) + fixed += count + if not count: # nesting can be more than one deep + return text, fixed + + def strip_owned(text: str, titles_by_id: dict[int, str]) -> tuple[str, int]: """Remove the links this script owns, leaving the words behind. @@ -181,15 +205,22 @@ def main() -> int: # Strip first, always: the rule is applied to clean prose so a re-run # cannot layer a new pass on top of an old one. - stripped = 0 + stripped = repaired = 0 + + def clean(value): + nonlocal stripped, repaired + value, fixed = unnest(value) + repaired += fixed + value, removed = strip_owned(value, titles_by_id) + stripped += removed + return value + for article in articles: for section in article.sections or []: - section["content"], n = strip_owned(section.get("content"), titles_by_id) - stripped += n - article.summary, n = strip_owned(article.summary, titles_by_id) - stripped += n - article.content, n = strip_owned(article.content, titles_by_id) - stripped += n + section["content"] = clean(section.get("content")) + article.summary = clean(article.summary) + article.content = clean(article.content) + print(f" nested markers repaired : {repaired}") print(f" existing auto-links stripped: {stripped}") if strip_only: diff --git a/frontend/src/pages/ArticleSplitView.test.jsx b/frontend/src/pages/ArticleSplitView.test.jsx index 0992001..b9c5218 100644 --- a/frontend/src/pages/ArticleSplitView.test.jsx +++ b/frontend/src/pages/ArticleSplitView.test.jsx @@ -44,9 +44,13 @@ describe('reading a cross-reference beside the article', () => { }) }) + // The preview card appears on a 350ms timer (ArticleLink.HOVER_DELAY). Under + // the full parallel run the box is busy enough that the old 2s allowance ran + // out, and a different case failed each time. The wait is generous because + // this test is about the split view, not about how fast the machine is. const openSplit = async (name) => { await userEvent.hover(screen.getByRole('link', { name })) - const card = await screen.findByRole('tooltip', {}, { timeout: 2000 }) + const card = await screen.findByRole('tooltip', {}, { timeout: 8000 }) await userEvent.click(within(card).getByRole('button', { name: /split view/i })) } diff --git a/frontend/src/pages/SettingsPage.css b/frontend/src/pages/SettingsPage.css new file mode 100644 index 0000000..185fd65 --- /dev/null +++ b/frontend/src/pages/SettingsPage.css @@ -0,0 +1,113 @@ +/* Settings as a set of places rather than one long scroll. + * + * Everything lived in a single 600px column: account, theme, an integration, + * a document list and an admin grid, in that order, with no way to link to any + * of it. Each is now its own panel with its own address. + */ + +.set-layout { + display: grid; + grid-template-columns: 220px minmax(0, 1fr); + gap: 28px; + align-items: start; + max-width: 1000px; + margin: 0 auto; + padding-bottom: 48px; +} + +.set-head { grid-column: 1 / -1; margin-bottom: 4px; } +.set-head h1 { margin: 0 0 4px; font-size: 1.4rem; font-weight: 700; } +.set-head p { margin: 0; font-size: 0.86rem; color: var(--text-muted); } + +/* ── Section list ─────────────────────────────────────────────────── */ +.set-nav { position: sticky; top: 76px; display: flex; flex-direction: column; gap: 2px; } +.set-nav button { + display: flex; align-items: center; gap: 10px; + padding: 10px 12px; min-height: 44px; /* touch target */ + font: inherit; font-size: 0.89rem; text-align: left; + background: none; border: 0; border-radius: 8px; + color: var(--text-muted); cursor: pointer; +} +.set-nav button:hover { background: var(--bg); color: var(--text); } +.set-nav button.is-active { background: var(--option-sel-bg); color: var(--primary); font-weight: 650; } +.set-nav-icon { width: 20px; text-align: center; font-size: 1rem; } +.set-nav-rule { margin: 8px 4px; border: 0; border-top: 1px solid var(--border); } + +/* ── Panel ────────────────────────────────────────────────────────── */ +.set-panel { + min-width: 0; + background: var(--card-bg); + border: 1px solid var(--border); + border-radius: 12px; + overflow: hidden; +} +.set-panel + .set-panel { margin-top: 16px; } +.set-panel-head { padding: 16px 20px; border-bottom: 1px solid var(--border); } +.set-panel-head h2 { margin: 0; font-size: 1.02rem; font-weight: 650; } +.set-panel-head p { margin: 4px 0 0; font-size: 0.83rem; color: var(--text-muted); line-height: 1.55; } +.set-panel-body { padding: 20px; } + +/* Form controls, so a section does not have to style its own. */ +.set-panel-body .form-group { margin-bottom: 14px; } +.set-panel-body .form-group label { + display: block; margin-bottom: 6px; + font-size: 0.68rem; font-weight: 700; letter-spacing: 0.07em; + text-transform: uppercase; color: var(--text-subtle); +} +.set-panel-body input[type="text"], +.set-panel-body input[type="password"], +.set-panel-body input[type="email"], +.set-panel-body input[type="url"], +.set-panel-body select { + width: 100%; padding: 10px 12px; + /* 16px on touch: iOS zooms in on a smaller font and never zooms back out. */ + font-size: 16px; font-family: inherit; + border: 1px solid var(--border); border-radius: 8px; + background: var(--input-bg); color: var(--text); +} +@media (min-width: 700px) { + .set-panel-body input[type="text"], + .set-panel-body input[type="password"], + .set-panel-body input[type="email"], + .set-panel-body input[type="url"], + .set-panel-body select { font-size: 0.92rem; } +} + +.set-note { margin: 0 0 14px; font-size: 0.85rem; color: var(--text-muted); line-height: 1.6; } +.set-role { + display: inline-block; margin-left: 6px; padding: 1px 8px; border-radius: 10px; + font-size: 0.72rem; font-weight: 600; + background: var(--option-sel-bg); color: var(--primary); +} + +/* Cards used by Appearance and Administration. */ +.set-cards { display: grid; grid-template-columns: repeat(auto-fill, minmax(170px, 1fr)); gap: 12px; } +.set-card { + display: block; padding: 14px 16px; text-align: left; + background: var(--bg); border: 2px solid var(--border); border-radius: 10px; + text-decoration: none; color: inherit; cursor: pointer; font: inherit; +} +.set-card:hover { border-color: var(--primary); } +.set-card.is-on { border-color: var(--primary); background: var(--option-sel-bg); } +.set-card-icon { font-size: 1.35rem; margin-bottom: 6px; } +.set-card-name { font-weight: 650; font-size: 0.9rem; } +.set-card.is-on .set-card-name { color: var(--primary); } +.set-card-desc { font-size: 0.78rem; color: var(--text-muted); margin-top: 2px; line-height: 1.45; } + +/* ── Narrow ───────────────────────────────────────────────────────── */ +/* The section list becomes a scrolling strip above the panel. Two levels of + navigation on a phone — list, then panel, then back — is one too many for + changing a password. */ +@media (max-width: 820px) { + .set-layout { grid-template-columns: minmax(0, 1fr); gap: 16px; } + .set-nav { + position: static; flex-direction: row; gap: 6px; + overflow-x: auto; scrollbar-width: none; + padding-bottom: 4px; border-bottom: 1px solid var(--border); + } + .set-nav::-webkit-scrollbar { display: none; } + .set-nav button { white-space: nowrap; border-radius: 999px; border: 1px solid var(--border); } + .set-nav button.is-active { border-color: var(--primary); } + .set-nav-rule { display: none; } + .set-nav-icon { display: none; } +} diff --git a/frontend/src/pages/SettingsPage.jsx b/frontend/src/pages/SettingsPage.jsx index c1ce3ed..0b8f92e 100644 --- a/frontend/src/pages/SettingsPage.jsx +++ b/frontend/src/pages/SettingsPage.jsx @@ -1,15 +1,20 @@ -import { useState, useEffect } from 'react' -import { Link } from 'react-router-dom' +import { Fragment, useState, useEffect } from 'react' +import { Link, useSearchParams } from 'react-router-dom' import { useAuth } from '../context/AuthContext' import { useTheme } from '../context/ThemeContext' import api from '../api/client' +import ExamSwitcher from '../components/ExamSwitcher' +import './SettingsPage.css' -function Section({ title, children }) { +function Section({ title, description, children }) { return ( -
-

{title}

- {children} -
+
+
+

{title}

+ {description &&

{description}

} +
+
{children}
+
) } @@ -43,7 +48,7 @@ function ProfileSection({ user }) { } return ( -
+

{user?.email}   { - api.get('/auth/me/settings').then(res => { - setRemindersDisabled(!!res.data.reminders_disabled) - }).catch(() => {}).finally(() => setLoaded(true)) - }, []) - - const toggle = async (disabled) => { - setRemindersDisabled(disabled) - try { - const current = await api.get('/auth/me/settings') - await api.put('/auth/me/settings', { ...current.data, reminders_disabled: disabled }) - } catch { setRemindersDisabled(!disabled) } - } - - if (!loaded) return null - - return ( -

-
-
- Quiz Reminders - - Receive email reminders to review quizzes based on your performance. - -
- -
-
- ) -} - function AppearanceSection() { const { theme, setTheme } = useTheme() const themes = [ @@ -131,26 +93,35 @@ function AppearanceSection() { { value: 'markdown', label: 'Warm Brown', desc: 'Parchment serif — easy on the eyes', icon: '📖' }, ] return ( -
-
+
+
{themes.map(t => ( -
setTheme(t.value)} - style={{ - flex: 1, minWidth: 140, padding: '14px 16px', borderRadius: 10, cursor: 'pointer', - border: `2px solid ${theme === t.value ? 'var(--primary)' : 'var(--border)'}`, - background: theme === t.value ? 'var(--option-sel-bg)' : 'var(--card-bg)', - transition: 'border-color 0.15s', - }}> -
{t.icon}
-
{t.label}
-
{t.desc}
-
+ ))}
) } +/** + * What the learner is revising for. It scopes the question bank, the facets + * and now the knowledge profile, which makes it a setting rather than a + * navbar control — it was only reachable from a dropdown in the header. + */ +function StudySection() { + return ( +
+ +
+ ) +} + function NextcloudSection() { const [server, setServer] = useState('https://cloud.danvics.com') const [username, setUsername] = useState('') @@ -243,26 +214,20 @@ function NextcloudSection() { function AdminSection() { return ( -
-
+
+
{[ - { to: '/admin', icon: '⚙️', label: 'Admin Dashboard', desc: 'Models, users, and settings' }, + { to: '/admin', icon: '⚙️', label: 'Admin dashboard', desc: 'Models, users and settings' }, { to: '/upload', icon: '📄', label: 'Upload PDF', desc: 'Add new documents' }, - { to: '/trash', icon: '🗑️', label: 'Trash', desc: 'Restore deleted quizzes' }, - { to: '/jobs', icon: '📋', label: 'Extraction Jobs', desc: 'View extraction history' }, + { to: '/categories', icon: '🗂️', label: 'Taxonomy', desc: 'Topics, systems, symptoms, diseases' }, + { to: '/editorial', icon: '✍️', label: 'Editorial', desc: 'Article drafts and references' }, + { to: '/trash', icon: '🗑️', label: 'Trash', desc: 'Restore deleted tests' }, + { to: '/jobs', icon: '📋', label: 'Extraction jobs', desc: 'Extraction history' }, ].map(item => ( - -
e.currentTarget.style.borderColor = 'var(--primary)'} - onMouseLeave={e => e.currentTarget.style.borderColor = 'var(--border)'} - > -
{item.icon}
-
{item.label}
-
{item.desc}
-
+ + +
{item.label}
+
{item.desc}
))}
@@ -402,23 +367,61 @@ function DataSection() { ) } +/** + * Settings as a set of places, each with its own address. + * + * It was one 600px column holding the account form, the theme picker, a + * Nextcloud integration, a document list and an admin link grid, in that + * order, with no way to link to any of it. The section now lives in the URL, + * so "change your password" is a link and Back works. + */ export default function SettingsPage() { const { user } = useAuth() const isAdmin = user?.role === 'admin' - const isModerator = user?.role === 'admin' || user?.role === 'moderator' + const isModerator = isAdmin || user?.role === 'moderator' + const [params, setParams] = useSearchParams() + + const sections = [ + { key: 'account', icon: '👤', label: 'Account', render: () => }, + { key: 'study', icon: '🎯', label: 'Studying for', render: () => }, + { key: 'appearance', icon: '🎨', label: 'Appearance', render: () => }, + { key: 'data', icon: '🗄️', label: 'Your data', render: () => }, + ...(isModerator ? [ + { key: 'library', icon: '📚', label: 'Documents', divider: true, + render: () => <> }, + { key: 'admin', icon: '🛠️', label: 'Administration', render: () => }, + ] : []), + ] + + const requested = params.get('s') + const active = sections.find(section => section.key === requested) || sections[0] return ( -
-
-

Settings

+
+
+

Settings

+

Your account, how the site looks, and what it shows you.

- - - - - {isModerator && } - {isModerator && } - {(isAdmin || isModerator) && } + + + +
{active.render()}
) } diff --git a/frontend/src/pages/SettingsPage.test.jsx b/frontend/src/pages/SettingsPage.test.jsx new file mode 100644 index 0000000..12678e3 --- /dev/null +++ b/frontend/src/pages/SettingsPage.test.jsx @@ -0,0 +1,85 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { MemoryRouter, Route, Routes } from 'react-router-dom' +import SettingsPage from './SettingsPage' +import api from '../api/client' + +vi.mock('../api/client', () => ({ default: { get: vi.fn(), put: vi.fn(), post: vi.fn(), delete: vi.fn() } })) +let currentUser = { id: 1, name: 'Learner', email: 'learner@example.test', role: 'user' } +vi.mock('../context/AuthContext', () => ({ useAuth: () => ({ user: currentUser }) })) +vi.mock('../context/ThemeContext', () => ({ useTheme: () => ({ theme: 'default', setTheme: vi.fn() }) })) + +const mount = (entry = '/settings') => render( + + } /> + ) + +describe('settings', () => { + beforeEach(() => { + vi.clearAllMocks() + currentUser = { id: 1, name: 'Learner', email: 'learner@example.test', role: 'user' } + api.get.mockImplementation(url => { + if (url === '/exams/') return Promise.resolve({ data: { exams: [{ id: 1, name: 'Boards', question_count: 900 }], active_exam_id: 1 } }) + return Promise.resolve({ data: [] }) + }) + }) + + it('opens on the account section and shows one panel at a time', async () => { + mount() + expect(await screen.findByRole('heading', { name: 'Account' })).toBeInTheDocument() + expect(screen.queryByRole('heading', { name: 'Appearance' })).not.toBeInTheDocument() + }) + + it('puts the section in the address, so it can be linked to', async () => { + mount('/settings?s=appearance') + expect(await screen.findByRole('heading', { name: 'Appearance' })).toBeInTheDocument() + expect(screen.queryByRole('heading', { name: 'Account' })).not.toBeInTheDocument() + }) + + it('switches section from the list', async () => { + mount() + await screen.findByRole('heading', { name: 'Account' }) + await userEvent.click(screen.getByRole('button', { name: /Your data/ })) + expect(await screen.findByRole('heading', { name: 'Your data' })).toBeInTheDocument() + }) + + it('falls back to the first section when the address names one that is gone', async () => { + mount('/settings?s=notifications') + expect(await screen.findByRole('heading', { name: 'Account' })).toBeInTheDocument() + }) + + it('keeps moderator sections away from a learner', async () => { + mount() + await screen.findByRole('heading', { name: 'Account' }) + expect(screen.queryByRole('button', { name: /Administration/ })).not.toBeInTheDocument() + expect(screen.queryByRole('button', { name: /Documents/ })).not.toBeInTheDocument() + }) + + it('offers them to a moderator', async () => { + currentUser = { ...currentUser, role: 'moderator' } + mount() + await screen.findByRole('heading', { name: 'Account' }) + expect(screen.getByRole('button', { name: /Administration/ })).toBeInTheDocument() + }) + + it('carries the exam objective, which was only reachable from the navbar', async () => { + mount('/settings?s=study') + expect(await screen.findByRole('heading', { name: 'Studying for' })).toBeInTheDocument() + await waitFor(() => expect(screen.getByLabelText('Studying for')).toHaveValue('1')) + }) + + it('asks for a typed word before wiping practice data, and says what survives', async () => { + mount('/settings?s=data') + await userEvent.click(await screen.findByRole('button', { name: 'Reset all practice data' })) + const confirm = screen.getByRole('button', { name: 'Reset everything' }) + expect(confirm).toBeDisabled() + await userEvent.type(screen.getByLabelText('Type RESET to confirm'), 'reset') + expect(confirm).toBeEnabled() + + api.post.mockResolvedValue({ data: { removed: { attempts: 3, answers: 40, saved_questions: 1, question_notes: 0 } } }) + await userEvent.click(confirm) + await waitFor(() => expect(api.post).toHaveBeenCalledWith('/attempts/reset-all', { confirm: 'reset' })) + expect(await screen.findByText(/Removed 3 sessions/)).toBeInTheDocument() + }) +})