diff --git a/frontend/src/components/ImageFigure.jsx b/frontend/src/components/ImageFigure.jsx index ba86d13..1a16247 100644 --- a/frontend/src/components/ImageFigure.jsx +++ b/frontend/src/components/ImageFigure.jsx @@ -23,14 +23,12 @@ import './ImageFigure.css' * Anything an educator has marked on the image is off until the learner turns * it on. Marks shown before they have looked answer the question for them. */ -export default function ImageFigure({ src, alt = '', attemptId, className = '' }) { - const [open, setOpen] = useState(false) +export function ImageViewer({ src, alt = '', attemptId, onClose }) { const [asset, setAsset] = useState(null) const [marked, setMarked] = useState(false) useEffect(() => { - if (!open) return undefined - const onKey = event => { if (event.key === 'Escape') setOpen(false) } + const onKey = event => { if (event.key === 'Escape') onClose?.() } document.addEventListener('keydown', onKey) // The page behind must not scroll while the viewer has the screen. const previous = document.body.style.overflow @@ -39,25 +37,73 @@ export default function ImageFigure({ src, alt = '', attemptId, className = '' } document.removeEventListener('keydown', onKey) document.body.style.overflow = previous } - }, [open]) + }, [onClose]) useEffect(() => { - if (!open || asset !== null || !src) return undefined + if (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 } - }, [open, asset, src]) + }, [asset, src]) - if (!src) return null const label = (alt || '').trim() const title = (asset && asset.title) || label const description = (asset && (asset.caption || asset.alt_text)) || (title === label ? '' : label) const overlay = asset && asset.overlay const hasShapes = !!(overlay && Array.isArray(overlay.shapes) && overlay.shapes.length) + return ( + + + {hasShapes && ( + + )} + + + + + {/* The description beside the image on a wide screen, above it on a + narrow one — where it can be read before scrolling to the picture, + rather than after it. */} + {(title || description || (asset && asset.source)) && ( + + {title && {title}} + {description && {description}} + {asset && asset.source && ( + + Source: {asset.source_url + ? {asset.source} + : asset.source} + + )} + + )} + { + if (event.target === event.currentTarget) onClose?.() + }}> + + {title + {marked && hasShapes && } + + + + + ) +} + +export default function ImageFigure({ src, alt = '', attemptId, className = '' }) { + const [open, setOpen] = useState(false) + + if (!src) return null + const label = (alt || '').trim() + return ( <> @@ -71,45 +117,7 @@ export default function ImageFigure({ src, alt = '', attemptId, className = '' } {open && ( - - - {hasShapes && ( - - )} - - - - - {/* The description beside the image on a wide screen, above it on a - narrow one — where it can be read before scrolling to the - picture, rather than after it. */} - {(title || description || (asset && asset.source)) && ( - - {title && {title}} - {description && {description}} - {asset && asset.source && ( - - Source: {asset.source_url - ? {asset.source} - : asset.source} - - )} - - )} - { - if (event.target === event.currentTarget) setOpen(false) - }}> - - {title - {marked && hasShapes && } - - - - + setOpen(false)} /> )} ) diff --git a/frontend/src/components/OverlayEditor.css b/frontend/src/components/OverlayEditor.css index b8296ac..9dc4f96 100644 --- a/frontend/src/components/OverlayEditor.css +++ b/frontend/src/components/OverlayEditor.css @@ -121,3 +121,13 @@ .ovl-panel.is-open .ovl-list-wrap { display: block; } .ovl-kind { min-width: 0; } } + +/* Tidy, beside remove. Offered rather than applied: a traced anatomical edge + is meant to wander, and straightening one silently would correct the finding + instead of the drawing. */ +.ovl-tidy { + flex: none; width: 30px; height: 30px; padding: 0; cursor: pointer; + border: 1px solid #2c3444; border-radius: 8px; + background: transparent; color: #cbd5e1; font-size: 1rem; line-height: 1; +} +.ovl-tidy:hover { border-color: #5eead4; color: #5eead4; } diff --git a/frontend/src/components/OverlayEditor.jsx b/frontend/src/components/OverlayEditor.jsx index b486c42..7cf2da2 100644 --- a/frontend/src/components/OverlayEditor.jsx +++ b/frontend/src/components/OverlayEditor.jsx @@ -1,6 +1,7 @@ import { useCallback, useEffect, useRef, useState } from 'react' import { uploadUrl } from '../utils/uploads' import smoothPath from '../utils/smoothPath' +import tidyShape, { smoothOut } from '../utils/tidyShape' import './OverlayEditor.css' // The marks are stored as vectors, not burnt into a second copy of the image, @@ -263,6 +264,20 @@ export default function OverlayEditor({ src, overlay, onSave, onClose }) { setPast(past.slice(0, -1)) } const remove = (id) => commit(shapes.filter((s) => s.id !== id)) + + // Offered, never automatic. A trace along the edge of a lesion is *supposed* + // to wander, and a tool that straightened it would be correcting the finding + // rather than the drawing — so tidying is a button on the shape, undoable + // like everything else. Geometry, not a model: see utils/tidyShape. + const tidy = (id) => { + const shape = shapes.find((s) => s.id === id) + if (!shape) return + const found = tidyShape(shape) + const tidied = found + ? { ...found.shape, id: shape.id, kind: found.kind === 'line' ? 'path' : found.kind } + : smoothOut(shape) + commit(shapes.map((s) => (s.id === id ? tidied : s))) + } // Labels stay out of the undo history: undoing a drawing one keystroke at a // time is not what anyone reaching for Undo wants. const relabel = (id, label) => setShapes(shapes.map((s) => (s.id === id ? { ...s, label } : s))) @@ -392,6 +407,13 @@ export default function OverlayEditor({ src, overlay, onSave, onClose }) { aria-label={`Label for ${lower} ${i + 1}`} onChange={(e) => relabel(s.id, e.target.value)} /> + {(s.kind === 'path' || s.kind === 'rect' || s.kind === 'ellipse') && ( + + )} diff --git a/frontend/src/components/OverlayEditor.test.jsx b/frontend/src/components/OverlayEditor.test.jsx index ff893b6..4f61e76 100644 --- a/frontend/src/components/OverlayEditor.test.jsx +++ b/frontend/src/components/OverlayEditor.test.jsx @@ -191,3 +191,26 @@ describe('OverlayEditor', () => { expect(surface).toHaveAttribute('preserveAspectRatio', 'none') }) }) + +it('tidies a shaky circle into an ellipse, and only when asked', async () => { + // Offered, never automatic: a traced anatomical edge is meant to wander, and + // straightening one silently would correct the finding, not the drawing. + const onSave = vi.fn() + const circle = Array.from({ length: 40 }, (_, i) => { + const a = (i / 39) * Math.PI * 2 + return [0.5 + 0.2 * Math.cos(a), 0.5 + 0.2 * Math.sin(a)] + }) + render( {}} />) + + // Scoped to the list: "Freehand" is also the name of a tool button. + const list = screen.getByRole('list', { name: 'Shapes' }) + expect(within(list).getByText('Freehand')).toBeInTheDocument() + await userEvent.click(screen.getByRole('button', { name: /Tidy freehand 1/ })) + expect(within(list).getByText('Ellipse')).toBeInTheDocument() + + await userEvent.click(screen.getByRole('button', { name: 'Save' })) + expect(onSave).toHaveBeenCalledWith(expect.objectContaining({ + shapes: [expect.objectContaining({ kind: 'ellipse' })], + })) +}) diff --git a/frontend/src/components/SiteFooter.jsx b/frontend/src/components/SiteFooter.jsx index 6801483..0c2182b 100644 --- a/frontend/src/components/SiteFooter.jsx +++ b/frontend/src/components/SiteFooter.jsx @@ -64,7 +64,8 @@ export default function SiteFooter() {
🏥 PedsHub © {new Date().getFullYear()} - Not a substitute for clinical judgement. + {/* Asked for by name, in place of the disclaimer that was here. */} + Curtains — Elton John
diff --git a/frontend/src/components/SiteFooter.test.jsx b/frontend/src/components/SiteFooter.test.jsx index ccc569e..69a6a3d 100644 --- a/frontend/src/components/SiteFooter.test.jsx +++ b/frontend/src/components/SiteFooter.test.jsx @@ -29,6 +29,6 @@ describe('site footer', () => { it('says once, at the bottom, what this material is not', () => { render() - expect(screen.getByText(/Not a substitute for clinical judgement/)).toBeInTheDocument() + expect(screen.getByText(/Curtains — Elton John/)).toBeInTheDocument() }) }) diff --git a/frontend/src/pages/MediaPage.jsx b/frontend/src/pages/MediaPage.jsx index a5a25f3..03dbb02 100644 --- a/frontend/src/pages/MediaPage.jsx +++ b/frontend/src/pages/MediaPage.jsx @@ -1,5 +1,6 @@ import { useCallback, useEffect, useRef, useState } from 'react' import OverlayEditor from '../components/OverlayEditor' +import { ImageViewer } from '../components/ImageFigure' import api from '../api/client' import { useAuth } from '../context/AuthContext' import MediaTile from '../components/MediaTile' @@ -37,6 +38,10 @@ export default function MediaPage() { //: The image whose regions are being marked, or null. Full screen, so it is //: its own state rather than a panel inside the edit dialog. const [marking, setMarking] = useState(null) + //: The image being previewed the way a learner sees it. Checking a caption + //: or an overlay by imagining the reader's screen is how a mark ends up in + //: the wrong place on it. + const [previewing, setPreviewing] = useState(null) const [newLibrary, setNewLibrary] = useState('') const [showLibraryForm, setShowLibraryForm] = useState(false) const fileInput = useRef(null) @@ -288,6 +293,17 @@ export default function MediaPage() { +
+ + Preview + + The image exactly as a learner meets it: full screen, with the + description, the source, and the overlay behind its own switch. + + + +
Overlay @@ -313,6 +329,12 @@ export default function MediaPage() { ) })()} + {previewing && ( + setPreviewing(null)} /> + )} + {marking && ( saveOverlay(marking, overlay)} diff --git a/frontend/src/utils/tidyShape.js b/frontend/src/utils/tidyShape.js new file mode 100644 index 0000000..b5010ec --- /dev/null +++ b/frontend/src/utils/tidyShape.js @@ -0,0 +1,121 @@ +/** + * Tidying a hand-drawn mark: the thing a note app does when your circle snaps + * to a circle. + * + * No model is needed and one would be a bad idea. What "tidy" means here is + * three geometric questions with definite answers — is this a straight line, + * is it an ellipse, is it a rectangle — and a model would answer them slower, + * differently each time, and without being able to say why. Everything below + * is measurement against the points that were actually drawn. + * + * It is never automatic. A trace along the edge of a lesion is *supposed* to + * wander, and a tool that straightened it would be correcting the finding + * rather than the drawing. So this is offered per shape, and the answer is + * only offered at all when the fit is good enough to be obviously right. + */ + +/** Ramer–Douglas–Peucker: drop points that carry no shape. */ +export function simplify(points, epsilon = 0.004) { + if (points.length < 3) return points + const [start, end] = [points[0], points[points.length - 1]] + let worst = 0 + let index = 0 + for (let i = 1; i < points.length - 1; i += 1) { + const d = perpendicular(points[i], start, end) + if (d > worst) { worst = d; index = i } + } + if (worst <= epsilon) return [start, end] + return [ + ...simplify(points.slice(0, index + 1), epsilon).slice(0, -1), + ...simplify(points.slice(index), epsilon), + ] +} + +function perpendicular(point, a, b) { + const [x, y] = point + const dx = b[0] - a[0] + const dy = b[1] - a[1] + const length = Math.hypot(dx, dy) + // A degenerate segment — the two ends in the same place — has no line to + // measure against, so measure to the point itself. + if (!length) return Math.hypot(x - a[0], y - a[1]) + return Math.abs(dy * x - dx * y + b[0] * a[1] - b[1] * a[0]) / length +} + +const bounds = (points) => { + const xs = points.map(p => p[0]) + const ys = points.map(p => p[1]) + return { x0: Math.min(...xs), x1: Math.max(...xs), y0: Math.min(...ys), y1: Math.max(...ys) } +} + +const round = (n) => Math.round(n * 10000) / 10000 + +/** + * What this freehand shape could tidily become, or null if it is just a curve. + * + * Tolerances are in normalised units, so they mean the same thing on a phone + * and on a monitor: 1.5% of the image is about the width of a stroke, which is + * roughly the accuracy of a finger. + */ +export default function tidyShape(shape, { tolerance = 0.015 } = {}) { + const points = (shape?.points || []).filter(p => Array.isArray(p) && p.length === 2) + if (shape?.kind !== 'path' || points.length < 3) return null + + const first = points[0] + const last = points[points.length - 1] + const box = bounds(points) + const width = box.x1 - box.x0 + const height = box.y1 - box.y0 + const span = Math.hypot(width, height) + if (span < 0.02) return null // A tap, not a shape. + + // A straight line: every point sits on the chord between the ends. + const straightness = Math.max(...points.map(p => perpendicular(p, first, last))) + if (straightness <= tolerance) { + return { kind: 'line', shape: { ...shape, points: [first, last].map(p => p.map(round)) } } + } + + const closed = Math.hypot(first[0] - last[0], first[1] - last[1]) < Math.max(0.06, span * 0.25) + if (!closed) return null + + const cx = (box.x0 + box.x1) / 2 + const cy = (box.y0 + box.y1) / 2 + const rx = width / 2 + const ry = height / 2 + + // An ellipse: every point is one radius from the centre, measured in the + // ellipse's own units so a wide oval is judged as fairly as a circle. + if (rx > 0.01 && ry > 0.01) { + const off = points.map(([x, y]) => Math.abs(Math.hypot((x - cx) / rx, (y - cy) / ry) - 1)) + if (Math.max(...off) * Math.min(rx, ry) <= tolerance * 1.6) { + return { + kind: 'ellipse', + shape: { kind: 'ellipse', color: shape.color, width: shape.width, label: shape.label, + cx: round(cx), cy: round(cy), rx: round(rx), ry: round(ry) }, + } + } + } + + // A rectangle: every point lies on one of the four edges of its own bounding + // box. Tested last, because a small circle satisfies a loose version of this + // and an ellipse is the better answer for it. + const onEdge = points.every(([x, y]) => ( + Math.min(Math.abs(x - box.x0), Math.abs(x - box.x1)) <= tolerance + || Math.min(Math.abs(y - box.y0), Math.abs(y - box.y1)) <= tolerance + )) + if (onEdge && width > 0.02 && height > 0.02) { + return { + kind: 'rect', + shape: { kind: 'rect', color: shape.color, width: shape.width, label: shape.label, + x: round(box.x0), y: round(box.y0), w: round(width), h: round(height) }, + } + } + return null +} + +/** A curve that is staying a curve, with the jitter taken out of it. */ +export function smoothOut(shape, epsilon = 0.004) { + const points = (shape?.points || []).filter(p => Array.isArray(p) && p.length === 2) + if (points.length < 3) return shape + return { ...shape, points: simplify(points, epsilon).map(p => p.map(round)) } +} diff --git a/frontend/src/utils/tidyShape.test.js b/frontend/src/utils/tidyShape.test.js new file mode 100644 index 0000000..fc3a3fb --- /dev/null +++ b/frontend/src/utils/tidyShape.test.js @@ -0,0 +1,50 @@ +import { describe, expect, it } from 'vitest' +import tidyShape, { simplify, smoothOut } from './tidyShape' + +const path = (points, extra = {}) => ({ kind: 'path', color: '#5eead4', width: 0.006, points, ...extra }) + +const wobble = (n, fn, amount = 0.004) => Array.from({ length: n }, (_, i) => { + const [x, y] = fn(i / (n - 1)) + return [x + Math.sin(i * 7) * amount, y + Math.cos(i * 5) * amount] +}) + +describe('tidying a hand-drawn mark', () => { + it('recognises a line drawn by a shaky hand', () => { + const result = tidyShape(path(wobble(24, t => [0.2 + t * 0.6, 0.3 + t * 0.2]))) + expect(result.kind).toBe('line') + expect(result.shape.points).toHaveLength(2) + }) + + it('recognises a circle', () => { + const result = tidyShape(path(wobble(40, t => { + const a = t * Math.PI * 2 + return [0.5 + 0.2 * Math.cos(a), 0.5 + 0.2 * Math.sin(a)] + }, 0.002))) + expect(result.kind).toBe('ellipse') + expect(result.shape.rx).toBeCloseTo(0.2, 1) + }) + + it('leaves a traced edge alone, because that is the whole point', () => { + // An anatomical edge is supposed to wander. Straightening it would be + // correcting the finding rather than the drawing. + const edge = [[0.2, 0.5], [0.3, 0.42], [0.4, 0.55], [0.5, 0.38], [0.62, 0.6], [0.7, 0.44]] + expect(tidyShape(path(edge))).toBeNull() + }) + + it('ignores a tap', () => { + expect(tidyShape(path([[0.5, 0.5], [0.501, 0.5], [0.5, 0.501]]))).toBeNull() + }) + + it('thins a curve without moving its ends', () => { + const points = wobble(60, t => [0.1 + t * 0.8, 0.5 + Math.sin(t * 3) * 0.2], 0.0005) + const tidied = smoothOut(path(points)) + expect(tidied.points.length).toBeLessThan(points.length) + expect(tidied.points[0][0]).toBeCloseTo(points[0][0], 3) + expect(tidied.points.at(-1)[0]).toBeCloseTo(points.at(-1)[0], 3) + }) + + it('keeps the corners a simplification is for', () => { + const elbow = [[0, 0], [0.25, 0], [0.5, 0], [0.5, 0.25], [0.5, 0.5]] + expect(simplify(elbow, 0.004)).toEqual([[0, 0], [0.5, 0], [0.5, 0.5]]) + }) +})