diff --git a/scripts/frq-template.test.ts b/scripts/frq-template.test.ts index d381a40e..75986224 100644 --- a/scripts/frq-template.test.ts +++ b/scripts/frq-template.test.ts @@ -10,6 +10,7 @@ import { hasResponseText, makeId, normalizeFrqTemplate, + stripResponseHtml, } from "../src/lib/frq/template.ts"; test("part labels do not walk off the alphabet", () => { @@ -25,6 +26,17 @@ test("hasResponseText ignores markup-only responses", () => { assert.equal(hasResponseText(undefined), false); }); +test("stripResponseHtml decodes HTML entities, not just strips tags", () => { + assert.equal( + stripResponseHtml("

Supply & demand shift the curve.

"), + "Supply & demand shift the curve.", + ); + assert.equal(stripResponseHtml("

5 < 10 > 2

"), "5 < 10 > 2"); + assert.equal(stripResponseHtml("

"quoted"

"), '"quoted"'); + assert.equal(stripResponseHtml("

café

"), "café"); + assert.equal(stripResponseHtml("

 padded 

"), "padded"); +}); + test("a malformed document degrades instead of throwing", () => { const out = normalizeFrqTemplate(null, { id: "t1", diff --git a/src/components/apClasses.json b/src/components/apClasses.json index 152a6c79..30e72eaf 100644 --- a/src/components/apClasses.json +++ b/src/components/apClasses.json @@ -4,12 +4,14 @@ "AP 3-D Art and Design", "AP Art History", "AP Biology", + "AP Business", "AP Calculus AB", "AP Calculus BC", "AP Chemistry", "AP Chinese", "AP Comparative Government", "AP Computer Science A", + "AP Cybersecurity", "AP Computer Science Principles", "AP Drawing", "AP English Language", diff --git a/src/components/frq/editor/partCard.tsx b/src/components/frq/editor/partCard.tsx index 4f3251a3..03dd21fb 100644 --- a/src/components/frq/editor/partCard.tsx +++ b/src/components/frq/editor/partCard.tsx @@ -26,6 +26,17 @@ import { formatPoints, getEditorPartPoints } from "@/lib/frq/editorState"; import { makeId } from "@/lib/frq/template"; import { ChevronDown, ChevronUp, Info, Plus, Trash2 } from "lucide-react"; +/** + * DOM id of one part's card. The footer's jump-to-part shortcut scrolls to + * this. + * + * Read back with `getElementById` rather than `querySelector`, because a part + * id comes from Firestore and may contain characters that need escaping in a + * CSS selector but are fine in an id lookup. + */ +export const getEditorPartAnchorId = (partId: string) => + `frq-editor-part-${partId}`; + interface PartCardProps { part: EditorPart; /** Display label within its question, restarting at A for each question. */ @@ -80,7 +91,7 @@ const PartCard = ({ return ( JSON.stringify(currentPayload) !== savedSignature, + [currentPayload, savedSignature], + ); useEffect(() => { if (!hasUnsavedChanges) { @@ -226,7 +231,7 @@ const FRQEditorRenderer = ({ requestAnimationFrame(() => { document - .querySelector(`[data-frq-part="${partId}"]`) + .getElementById(getEditorPartAnchorId(partId)) ?.scrollIntoView({ behavior: "smooth", block: "start" }); }); }; @@ -269,6 +274,11 @@ const FRQEditorRenderer = ({ } }, [currentPayload, frqTemplate]); + const partLocationIndex = useMemo( + () => buildPartLocationIndex(questions), + [questions], + ); + const totalPoints = getEditorTotalPoints(questions); const totalParts = questions.reduce( (total, question) => total + question.parts.length, @@ -461,7 +471,12 @@ const FRQEditorRenderer = ({ onDeletePart={deletePart} onMovePart={movePartBy} canMovePart={(partId, direction) => - canMovePart(questions, partId, direction) + canMovePartIndexed( + questions, + partLocationIndex, + partId, + direction, + ) } canDelete={questions.length > 1} openParts={openParts} diff --git a/src/components/frq/gradingRenderer.tsx b/src/components/frq/gradingRenderer.tsx index ed5d9ee4..eb7d8c9d 100644 --- a/src/components/frq/gradingRenderer.tsx +++ b/src/components/frq/gradingRenderer.tsx @@ -2,7 +2,7 @@ import { RenderContent } from "@/components/article-creator/custom_questions/RenderAdvancedTextbox"; import { db } from "@/lib/firebase"; -import { useEffect, useMemo, useState } from "react"; +import { useMemo, useState } from "react"; import Link from "next/link"; import { LogOut } from "lucide-react"; import { runTransaction, serverTimestamp } from "firebase/firestore"; @@ -14,6 +14,7 @@ import GradingFooter from "@/components/frq/grading/gradingFooter"; import GradingPartCard, { getGradingPartAnchorId, } from "@/components/frq/grading/partCard"; +import { usePendingPartScroll } from "@/components/frq/usePendingPartScroll"; import { buildGradingQuestions, buildStoredGrades, @@ -57,9 +58,6 @@ const FRQGradingRenderer = ({ const [overallFeedback, setOverallFeedback] = useState(""); const [isSubmitting, setIsSubmitting] = useState(false); const [showPrompt, setShowPrompt] = useState(true); - const [pendingScrollPartId, setPendingScrollPartId] = useState( - null, - ); // Keyed by part id, which is what the stored response map and every existing // grade are keyed by. Grouping parts under questions changes navigation and // nothing about how a grade resolves. @@ -67,20 +65,10 @@ const FRQGradingRenderer = ({ createEmptyGrades(parts), ); - // Runs after the target question has rendered, so the part being scrolled to - // is in the DOM. Jumping to a part on another question sets the index and - // this id together, and React commits both before the effect fires. - useEffect(() => { - if (!pendingScrollPartId) { - return; - } - - document - .getElementById(getGradingPartAnchorId(pendingScrollPartId)) - ?.scrollIntoView({ behavior: "smooth", block: "start" }); - - setPendingScrollPartId(null); - }, [pendingScrollPartId, currentQuestionIndex]); + const setPendingScrollPartId = usePendingPartScroll( + getGradingPartAnchorId, + currentQuestionIndex, + ); const possiblePoints = getTemplatePoints(parts); const earnedPoints = getEarnedPoints(parts, grades); diff --git a/src/components/frq/test/downloadResponsesPdf.ts b/src/components/frq/test/downloadResponsesPdf.ts index 9278341d..80727df1 100644 --- a/src/components/frq/test/downloadResponsesPdf.ts +++ b/src/components/frq/test/downloadResponsesPdf.ts @@ -1,5 +1,6 @@ import type { StudentQuestion } from "@/lib/frq/studentView"; import { getPartHeading } from "@/lib/frq/studentView"; +import { stripResponseHtml } from "@/lib/frq/template"; const PRINT_STYLES = ` body { @@ -33,13 +34,10 @@ const PRINT_STYLES = ` /** * Responses are stored as sanitized HTML, but the printed copy is plain text: * the print window has none of the app's styles, so markup would show up as - * literal tags. + * literal tags. Shares its stripping logic with `hasResponseText` rather than + * parsing each response through its own `DOMParser` instance. */ -const getPlainText = (html: string) => { - const parsedDocument = new DOMParser().parseFromString(html, "text/html"); - - return parsedDocument.body.textContent?.trim() || "No response"; -}; +const getPlainText = (html: string) => stripResponseHtml(html) || "No response"; /** * Open a print window holding the student's own answers. Headings name both diff --git a/src/components/frq/test/questionPane.tsx b/src/components/frq/test/questionPane.tsx index 07584161..e9058951 100644 --- a/src/components/frq/test/questionPane.tsx +++ b/src/components/frq/test/questionPane.tsx @@ -4,7 +4,26 @@ import { RenderContent } from "@/components/article-creator/custom_questions/Ren import FRQResponseEditor from "@/components/frq/responseEditor"; import type { StudentQuestion } from "@/lib/frq/studentView"; import { toQuestionInput } from "@/lib/frq/template"; +import type { QuestionFile } from "@/types/questions"; import { Bookmark } from "lucide-react"; +import { memo } from "react"; + +/** + * A page now stacks every part of a question, so an unmemoized prompt would + * re-run KaTeX rendering and rich-text sanitization for every other part on + * the page each time a keystroke in any one part's response box re-renders + * `QuestionPane`. Memoized on the prompt's own text and files so it only + * re-renders when its own content actually changes. + */ +const PartPrompt = memo( + ({ prompt, promptFiles }: { prompt?: string; promptFiles?: QuestionFile[] }) => ( + + ), +); +PartPrompt.displayName = "PartPrompt"; /** * DOM id of one part's block. The footer's part shortcuts and the review grid @@ -70,10 +89,7 @@ const QuestionPane = ({
- +
diff --git a/src/components/frq/testRenderer.tsx b/src/components/frq/testRenderer.tsx index 93a2a9c0..be187e49 100644 --- a/src/components/frq/testRenderer.tsx +++ b/src/components/frq/testRenderer.tsx @@ -9,6 +9,7 @@ import QuestionPane, { } from "@/components/frq/test/questionPane"; import ReviewPage from "@/components/frq/test/reviewPage"; import { SubmissionModal, TimeUpModal } from "@/components/frq/test/testModals"; +import { usePendingPartScroll } from "@/components/frq/usePendingPartScroll"; import { getUngradedFrqsCollectionRef } from "@/lib/firestore/frqRefs"; import { buildStudentQuestions, @@ -96,10 +97,6 @@ const FRQTestRenderer = ({ const [markedForReview, setMarkedForReview] = useState< Record >({}); - const [pendingScrollPartId, setPendingScrollPartId] = useState( - null, - ); - const [timeRemaining, setTimeRemaining] = useState( () => (template?.timeLimitMinutes ?? DEFAULT_TIME_LIMIT_MINUTES) * 60, ); @@ -142,20 +139,10 @@ const FRQTestRenderer = ({ } }, [draftKey, responses, hasSubmitted]); - // Runs after the target question has rendered, so the part being scrolled to - // is in the DOM. Jumping to a part on another question sets the index and - // this id together, and React commits both before the effect fires. - useEffect(() => { - if (!pendingScrollPartId) { - return; - } - - document - .getElementById(getPartAnchorId(pendingScrollPartId)) - ?.scrollIntoView({ behavior: "smooth", block: "start" }); - - setPendingScrollPartId(null); - }, [pendingScrollPartId, currentQuestionIndex]); + const setPendingScrollPartId = usePendingPartScroll( + getPartAnchorId, + currentQuestionIndex, + ); useEffect(() => { setTimeRemaining( diff --git a/src/components/frq/usePendingPartScroll.ts b/src/components/frq/usePendingPartScroll.ts new file mode 100644 index 00000000..9fc75486 --- /dev/null +++ b/src/components/frq/usePendingPartScroll.ts @@ -0,0 +1,40 @@ +import { useEffect, useState } from "react"; + +/** + * Scroll to a part once the question holding it has rendered. Shared by the + * student test page and the grading page: both page by question and jump to + * an individual part via a footer shortcut, so the part being scrolled to has + * to exist in the DOM before `scrollIntoView` runs. Jumping to a part on + * another question sets the question index and the pending part id together, + * and React commits both before the effect below fires. + * + * `anchorId` converts a part id to the DOM id its page renders it under; the + * two pages use different prefixes for that id, which is the only thing that + * differs between them. + */ +export const usePendingPartScroll = ( + anchorId: (partId: string) => string, + currentQuestionIndex: number, +) => { + const [pendingScrollPartId, setPendingScrollPartId] = useState< + string | null + >(null); + + useEffect(() => { + if (!pendingScrollPartId) { + return; + } + + document + .getElementById(anchorId(pendingScrollPartId)) + ?.scrollIntoView({ behavior: "smooth", block: "start" }); + + setPendingScrollPartId(null); + // `anchorId` is a stable module-level function at every call site, so it + // is intentionally left out of the dependency list rather than forcing + // callers to memoize it. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [pendingScrollPartId, currentQuestionIndex]); + + return setPendingScrollPartId; +}; diff --git a/src/components/landingPage/APLibrary.tsx b/src/components/landingPage/APLibrary.tsx index 760652ca..86a71100 100644 --- a/src/components/landingPage/APLibrary.tsx +++ b/src/components/landingPage/APLibrary.tsx @@ -91,6 +91,12 @@ const sectionData = [ borderColor: "#0891b2", courses: ["AP Research", "AP Seminar"], }, + { + title: "AP Career Kickstart", + numofCol: "lg:col-span-1", + borderColor: "#65a30d", + courses: ["AP Business", "AP Cybersecurity"], + }, ]; const APLibrary = () => { diff --git a/src/lib/frq/editorState.ts b/src/lib/frq/editorState.ts index b665df39..93419c25 100644 --- a/src/lib/frq/editorState.ts +++ b/src/lib/frq/editorState.ts @@ -315,6 +315,20 @@ export const movePart = ( }); }; +/** Whether a part at an already-located position has anywhere to go. */ +const canMoveLocatedPart = ( + questions: EditorQuestion[], + location: { questionIndex: number; partIndex: number }, + direction: -1 | 1, +): boolean => { + const { questionIndex, partIndex } = location; + + return direction === -1 + ? questionIndex > 0 || partIndex > 0 + : questionIndex < questions.length - 1 || + partIndex < (questions[questionIndex]?.parts.length ?? 0) - 1; +}; + /** Whether a part has anywhere to go, used to disable the move buttons. */ export const canMovePart = ( questions: EditorQuestion[], @@ -323,14 +337,38 @@ export const canMovePart = ( ): boolean => { const location = locatePart(questions, partId); - if (!location) { - return false; - } + return location ? canMoveLocatedPart(questions, location, direction) : false; +}; - const { questionIndex, partIndex } = location; +/** + * Every part's location, computed once instead of scanning all questions per + * part. The editor calls `canMovePart` twice for every part on every render, + * which made `locatePart`'s linear scan effectively O(n^2) over the whole + * document; building this map once per `questions` change keeps each lookup + * O(1). + */ +export const buildPartLocationIndex = ( + questions: EditorQuestion[], +): Map => { + const index = new Map(); - return direction === -1 - ? questionIndex > 0 || partIndex > 0 - : questionIndex < questions.length - 1 || - partIndex < (questions[questionIndex]?.parts.length ?? 0) - 1; + questions.forEach((question, questionIndex) => { + question.parts.forEach((part, partIndex) => { + index.set(part.id, { questionIndex, partIndex }); + }); + }); + + return index; +}; + +/** Same as `canMovePart`, but reading from a precomputed location index. */ +export const canMovePartIndexed = ( + questions: EditorQuestion[], + locationIndex: Map, + partId: string, + direction: -1 | 1, +): boolean => { + const location = locationIndex.get(partId); + + return location ? canMoveLocatedPart(questions, location, direction) : false; }; diff --git a/src/lib/frq/feedbackDocument.ts b/src/lib/frq/feedbackDocument.ts index 22c26bdf..8398b7c9 100644 --- a/src/lib/frq/feedbackDocument.ts +++ b/src/lib/frq/feedbackDocument.ts @@ -2,7 +2,7 @@ import type { FRQFeedbackDocument } from "@/components/frq/feedback/types"; import type { FRQTemplate, GradedFRQSubmission } from "@/types/frq"; import type { Timestamp } from "firebase/firestore"; import { buildGradingQuestions } from "./gradingView.ts"; -import { getAllParts, toQuestionInput } from "./template.ts"; +import { getAllParts, isMultiQuestion, toQuestionInput } from "./template.ts"; const formatTimestamp = (value: Timestamp | undefined) => { if (!value || typeof value.toDate !== "function") { @@ -27,7 +27,10 @@ const getQuestionName = ( templateTitle: string, questionCount: number, questionIndex: number, -) => (questionCount > 1 ? `Question ${questionIndex + 1}` : templateTitle); +) => + isMultiQuestion(questionCount) + ? `Question ${questionIndex + 1}` + : templateTitle; /** * Rebuild the rubric-shaped document the feedback UI renders from the two diff --git a/src/lib/frq/gradingView.ts b/src/lib/frq/gradingView.ts index 6e5a17f5..c50e42ce 100644 --- a/src/lib/frq/gradingView.ts +++ b/src/lib/frq/gradingView.ts @@ -4,7 +4,7 @@ import type { FRQTemplatePart, } from "@/types/frq"; import type { QuestionFile } from "@/types/questions"; -import { getPartLabel } from "./template.ts"; +import { getPartLabel, isMultiQuestion } from "./template.ts"; /** * The model the grading page pages through, kept apart from the component so @@ -97,7 +97,7 @@ export const getQuestionLabel = ( questionCount: number, questionIndex: number, ): string | null => - questionCount > 1 ? `Question ${questionIndex + 1}` : null; + isMultiQuestion(questionCount) ? `Question ${questionIndex + 1}` : null; /** * A blank grade for every part, keyed by part id. diff --git a/src/lib/frq/studentView.ts b/src/lib/frq/studentView.ts index ad12aa6b..62c8fa4c 100644 --- a/src/lib/frq/studentView.ts +++ b/src/lib/frq/studentView.ts @@ -1,6 +1,10 @@ import type { FRQTemplate, FRQTemplatePart } from "@/types/frq"; import type { QuestionFile } from "@/types/questions"; -import { getPartLabel, getStudentFacingQuestions } from "./template.ts"; +import { + getPartLabel, + getStudentFacingQuestions, + isMultiQuestion, +} from "./template.ts"; /** * The model the test renderer pages through, kept apart from the component so @@ -82,7 +86,7 @@ export const getPartHeading = ( questionIndex: number, label: string, ) => - questionCount > 1 + isMultiQuestion(questionCount) ? `Question ${questionIndex + 1}, Part ${label}` : `Part ${label}`; diff --git a/src/lib/frq/template.ts b/src/lib/frq/template.ts index 4d1dfa49..e46a71f3 100644 --- a/src/lib/frq/template.ts +++ b/src/lib/frq/template.ts @@ -270,6 +270,14 @@ export const getStudentFacingQuestions = ( })) .filter((question) => question.parts.length > 0); +/** + * Whether a document's questions should be numbered at all. Every legacy + * document normalizes into exactly one question, and those pages never + * carried a question number, so a single-question exam stays unnumbered + * rather than reading "Question 1" for an exam nobody split up. + */ +export const isMultiQuestion = (questionCount: number) => questionCount > 1; + export const getPartPoints = (part: FRQTemplatePart) => (part.criteria ?? []).reduce( (total, criterion) => total + criterion.points, @@ -295,12 +303,48 @@ export const getPartLabel = (index: number) => { return label; }; -/** Strips markup so "did the student write anything" is not fooled by `

`. */ +/** + * The named entities the response toolbar can actually produce or that show + * up in ordinary typed text once `&` is escaped. Not an exhaustive HTML5 + * entity table on purpose: this only has to undo what the sanitizer/editor + * puts in, not parse arbitrary markup. + */ +const NAMED_ENTITIES: Record = { + amp: "&", + lt: "<", + gt: ">", + quot: '"', + apos: "'", + nbsp: " ", +}; + +const decodeHtmlEntities = (value: string) => + value.replace(/&(#x[0-9a-f]+|#\d+|[a-z]+);/gi, (match, entity: string) => { + if (entity.startsWith("#")) { + const codePoint = entity.toLowerCase().startsWith("#x") + ? parseInt(entity.slice(2), 16) + : parseInt(entity.slice(1), 10); + + return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match; + } + + return NAMED_ENTITIES[entity.toLowerCase()] ?? match; + }); + +/** + * Strip a response's stored HTML down to plain text. Regex-based rather than + * `richTextToPlainText`'s DOM-parsing so it behaves the same whether this + * runs in the browser or under `node:test`, and so `src/lib/frq` does not + * reach into `article-creator` for something this small. Entities are decoded + * after tags are stripped, so `<p>` typed as literal text renders as + * `

` rather than vanishing as if it were markup. + */ +export const stripResponseHtml = (response: string | undefined) => + decodeHtmlEntities((response ?? "").replace(/<[^>]*>/g, "")).trim(); + +/** Whether a response has anything in it, not fooled by markup-only `

`. */ export const hasResponseText = (response: string | undefined) => - (response ?? "") - .replace(/<[^>]*>/g, "") - .replace(/ /g, " ") - .trim().length > 0; + stripResponseHtml(response).length > 0; /** * Unique, immutable ID built from the current time plus a short random suffix. diff --git a/src/types/frq.ts b/src/types/frq.ts index 181e1027..883a2e4c 100644 --- a/src/types/frq.ts +++ b/src/types/frq.ts @@ -83,7 +83,7 @@ export interface GradableFRQSubmission { subject: string; unitId: string; studentId: string; - /** Responses are keyed by a stable FRQTemplateQuestion.id. */ + /** Responses are keyed by a stable FRQTemplatePart.id. */ responses: Record; submittedAt: Timestamp; }