Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/components/apClasses.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
13 changes: 12 additions & 1 deletion src/components/frq/editor/partCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -80,7 +91,7 @@ const PartCard = ({
return (
<AccordionItem
value={part.id}
data-frq-part={part.id}
id={getEditorPartAnchorId(part.id)}
className="rounded-lg border bg-background px-4 shadow-sm"
>
<AccordionTrigger
Expand Down
23 changes: 19 additions & 4 deletions src/components/frq/editorRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import FRQEditorFooter from "@/components/frq/editorFooter";
import QuestionCard from "@/components/frq/editor/questionCard";
import { getEditorPartAnchorId } from "@/components/frq/editor/partCard";
import RichPromptEditor from "@/components/frq/editor/richPromptEditor";
import { Accordion } from "@/components/ui/accordion";
import { Button } from "@/components/ui/button";
Expand All @@ -14,8 +15,9 @@ import type {
} from "@/lib/frq/editorState";
import {
buildInitialState,
buildPartLocationIndex,
buildTemplatePayload,
canMovePart,
canMovePartIndexed,
createEditorPart,
createEditorQuestion,
deletePartById,
Expand Down Expand Up @@ -120,7 +122,10 @@ const FRQEditorRenderer = ({
],
);

const hasUnsavedChanges = JSON.stringify(currentPayload) !== savedSignature;
const hasUnsavedChanges = useMemo(
() => JSON.stringify(currentPayload) !== savedSignature,
[currentPayload, savedSignature],
);

useEffect(() => {
if (!hasUnsavedChanges) {
Expand Down Expand Up @@ -226,7 +231,7 @@ const FRQEditorRenderer = ({

requestAnimationFrame(() => {
document
.querySelector(`[data-frq-part="${partId}"]`)
.getElementById(getEditorPartAnchorId(partId))
?.scrollIntoView({ behavior: "smooth", block: "start" });
});
};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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}
Expand Down
24 changes: 6 additions & 18 deletions src/components/frq/gradingRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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,
Expand Down Expand Up @@ -57,30 +58,17 @@ const FRQGradingRenderer = ({
const [overallFeedback, setOverallFeedback] = useState("");
const [isSubmitting, setIsSubmitting] = useState(false);
const [showPrompt, setShowPrompt] = useState(true);
const [pendingScrollPartId, setPendingScrollPartId] = useState<string | null>(
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.
const [grades, setGrades] = useState<Record<string, PartGrade>>(() =>
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);
Expand Down
10 changes: 4 additions & 6 deletions src/components/frq/test/downloadResponsesPdf.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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
Expand Down
24 changes: 20 additions & 4 deletions src/components/frq/test/questionPane.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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[] }) => (
<RenderContent
content={toQuestionInput(prompt, promptFiles)}
origin="question"
/>
),
);
PartPrompt.displayName = "PartPrompt";

/**
* DOM id of one part's block. The footer's part shortcuts and the review grid
Expand Down Expand Up @@ -70,10 +89,7 @@ const QuestionPane = ({
</div>

<div className="mb-4 font-sans text-sm">
<RenderContent
content={toQuestionInput(part.prompt, part.promptFiles)}
origin="question"
/>
<PartPrompt prompt={part.prompt} promptFiles={part.promptFiles} />
</div>

<div className="w-full max-w-[50rem]">
Expand Down
23 changes: 5 additions & 18 deletions src/components/frq/testRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -96,10 +97,6 @@ const FRQTestRenderer = ({
const [markedForReview, setMarkedForReview] = useState<
Record<string, boolean>
>({});
const [pendingScrollPartId, setPendingScrollPartId] = useState<string | null>(
null,
);

const [timeRemaining, setTimeRemaining] = useState(
() => (template?.timeLimitMinutes ?? DEFAULT_TIME_LIMIT_MINUTES) * 60,
);
Expand Down Expand Up @@ -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(
Expand Down
40 changes: 40 additions & 0 deletions src/components/frq/usePendingPartScroll.ts
Original file line number Diff line number Diff line change
@@ -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;
};
6 changes: 6 additions & 0 deletions src/components/landingPage/APLibrary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => {
Expand Down
54 changes: 46 additions & 8 deletions src/lib/frq/editorState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[],
Expand All @@ -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<string, { questionIndex: number; partIndex: number }> => {
const index = new Map<string, { questionIndex: number; partIndex: number }>();

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<string, { questionIndex: number; partIndex: number }>,
partId: string,
direction: -1 | 1,
): boolean => {
const location = locationIndex.get(partId);

return location ? canMoveLocatedPart(questions, location, direction) : false;
};
Loading
Loading