From e6df507f51b7cf5a75c4b7d2df7532a14c3bb833 Mon Sep 17 00:00:00 2001 From: Tamara Date: Fri, 6 Feb 2026 12:06:21 -0500 Subject: [PATCH 01/12] [tb/claude-progressive-disclosure] Add prompt library docs and update CLAUDE.md --- .claude/architecture-research.md | 612 ++++++++++++++++++++ .claude/prompts/README.md | 33 ++ .claude/prompts/codebase-map.md | 95 +++ .claude/prompts/component-best-practices.md | 324 +++++++++++ .claude/prompts/file-organization.md | 271 +++++++++ .claude/prompts/iteration-and-feedback.md | 421 ++++++++++++++ .claude/prompts/math-and-content.md | 270 +++++++++ .claude/prompts/testing-best-practices.md | 181 ++++++ .claude/prompts/widget-development.md | 286 +++++++++ .claude/widget-patterns-research.md | 551 ++++++++++++++++++ CLAUDE.md | 240 +++----- 11 files changed, 3120 insertions(+), 164 deletions(-) create mode 100644 .claude/architecture-research.md create mode 100644 .claude/prompts/README.md create mode 100644 .claude/prompts/codebase-map.md create mode 100644 .claude/prompts/component-best-practices.md create mode 100644 .claude/prompts/file-organization.md create mode 100644 .claude/prompts/iteration-and-feedback.md create mode 100644 .claude/prompts/math-and-content.md create mode 100644 .claude/prompts/testing-best-practices.md create mode 100644 .claude/prompts/widget-development.md create mode 100644 .claude/widget-patterns-research.md diff --git a/.claude/architecture-research.md b/.claude/architecture-research.md new file mode 100644 index 00000000000..5a89eb08279 --- /dev/null +++ b/.claude/architecture-research.md @@ -0,0 +1,612 @@ +# Perseus Architecture Research + +**Date**: February 5, 2026 +**Researcher**: Claude (AI Assistant) +**Focus**: Current widget state management, data flow, onChange patterns, and package structure + +## Executive Summary + +Perseus is a well-structured React monorepo with a clear separation of concerns across multiple packages. The current architecture uses a props-driven approach where widgets receive their state through `handleUserInput` callbacks and `userInput` props, rather than the deprecated `onChange` pattern. State management has been intentionally refactored to move away from the old widget-centric model toward a more centralized approach managed by the `UserInputManager` and `Renderer`. + +--- + +## 1. Widget State Management + +### Current Pattern: Props-Driven with Callbacks + +Modern widgets do NOT use the old `onChange` handler. Instead, they: + +1. **Receive state through props**: `userInput: TUserInput` (typed per widget) +2. **Call `handleUserInput` callback** when state changes: `handleUserInput(newUserInput, callback?, silent?)` +3. **Call `trackInteraction` callback** to notify about interactions + +#### Radio Widget Example (Multiple Choice) + +From `/packages/perseus/src/widgets/radio/multiple-choice-widget.tsx`: + +```typescript +type Props = WidgetProps; + +const handleChoiceChange = (choiceId: string, newCheckedState: boolean): void => { + // Build list of checked choice IDs + const checkedChoiceIds: string[] = []; + // ... logic to determine new selection state ... + + // Create new choice states + const newChoiceStates: ChoiceState[] = choiceStates + ? choiceStates.map((state) => ({...state})) + : choices.map(() => ({ + selected: false, + // ... other state fields ... + })); + + // Update states based on selection + newChoiceStates.forEach((choiceState: ChoiceState, i) => { + const choiceId = choices[i].id; + choiceState.selected = checkedChoiceIds.includes(choiceId); + }); + + // Notify parent of change + onChange({choiceStates: newChoiceStates}); // ← Still has onChange! + trackInteraction(); + announceChoiceChange(newChoiceStates); +}; +``` + +**Note**: The Radio widget still uses `onChange`, but this is marked for deprecation (LEMS-3542, LEMS-3245). This is a transitional state during the Radio Revitalization Project. + +#### NumericInput Widget Example + +From `/packages/perseus/src/widgets/numeric-input/numeric-input.tsx`: + +```typescript +const handleChange = (newValue: string): void => { + // Call handleUserInput with new state + props.handleUserInput({currentValue: newValue}); + props.trackInteraction(); +}; + +const handleFocus = (): void => { + props.onFocus([]); + setIsFocused(true); +}; + +const handleBlur = (): void => { + props.onBlur([]); + setIsFocused(false); +}; +``` + +**Key differences from Radio**: +- Uses `handleUserInput` callback (modern pattern) +- No local component state for user input +- Props-driven: receives `props.userInput.currentValue` and sends updates via callback + +### State Storage Location + +**User Input State**: Managed by `UserInputManager` component +- Location: `/packages/perseus/src/user-input-manager.tsx` +- Stored in a central `userInput` object: `UserInputMap` (type from `@khanacademy/perseus-core`) +- Updated via `handleUserInput(widgetId, newUserInput, widgetsEmpty)` callback + +**Widget-Specific State**: +- Some widgets maintain internal component state (see Radio's `choiceStates`) +- This is being transitioned to centralized state management +- Focus management state: `onFocus`, `onBlur` callbacks update parent renderer + +--- + +## 2. Data Flow Architecture + +### High-Level Flow + +``` +ServerItemRenderer + ↓ +UserInputManager (manages userInput state) + ↓ (provides userInput, handleUserInput, initializeUserInput) + ↓ +Renderer (question rendering) + ├─ renderWidget() → WidgetContainer + │ ├─ Passes WidgetProps (from getWidgetProps()) + │ └─ WidgetContainer → Widget Component + │ ├─ Receives userInput prop + │ ├─ Calls handleUserInput on change + │ └─ Calls trackInteraction on user action + │ + └─ Returns UserInputMap to parent +``` + +### Detailed Props Flow + +**Where**: `/packages/perseus/src/renderer.tsx` lines 511-575 + +```typescript +getWidgetProps(widgetId: string): WidgetProps { + const apiOptions = this.getApiOptions(); + const widgetProps = this.props.widgets[widgetId].options; // ← Config from question + const widgetInfo = this.state.widgetInfo[widgetId]; + + return { + ...widgetProps, // ← Widget-specific options + userInput: this.props.userInput?.[widgetId], // ← Current user input + widgetId: widgetId, + widgetIndex: this._getWidgetIndexById(widgetId), + alignment: widgetInfo && widgetInfo.alignment, + static: widgetInfo?.static, + problemNum: this.props.problemNum, + apiOptions: this.getApiOptions(), // ← API configuration + keypadElement: this.props.keypadElement, + showSolutions: this.props.showSolutions, + onFocus: _.partial(this._onWidgetFocus, widgetId), + onBlur: _.partial(this._onWidgetBlur, widgetId), + findWidgets: this.findWidgets, // ← Inter-widget communication + reviewMode: this.props.reviewMode, + + // ← Core state management callback + handleUserInput: (newUserInput: UserInput) => { + const updatedUserInput = { + ...this.props.userInput, + [widgetId]: newUserInput, + }; + const emptyWidgetIds = emptyWidgetsFunctional( + this.state.widgetInfo, + this.widgetIds, + updatedUserInput, + this.context.locale, + ); + const widgetsEmpty = emptyWidgetIds.length > 0; + this.props.handleUserInput?.( + widgetId, + newUserInput, + widgetsEmpty, + ); + this.props.apiOptions?.interactionCallback?.(updatedUserInput); + }, + + trackInteraction: interactionTracker.track, + }; +} +``` + +### User Input Manager Flow + +**Where**: `/packages/perseus/src/user-input-manager.tsx` + +```typescript +export function sharedInitializeUserInput( + widgetOptions: PerseusWidgetsMap | undefined, + problemNum: number, +): UserInputMap { + const startUserInput: UserInputMap = {}; + + // For each widget, call widget's initialization function + Object.entries(widgetOptions).forEach(([id, widgetInfo]) => { + const widgetExports = Widgets.getWidgetExport(widgetInfo.type); + + // Static widgets use correct answer + if (widgetInfo.static && widgetExports?.getCorrectUserInput) { + startUserInput[id] = widgetExports.getCorrectUserInput( + widgetInfo.options, + ); + } + // Other widgets use start input + else if (widgetExports?.getStartUserInput) { + startUserInput[id] = widgetExports.getStartUserInput( + widgetInfo.options, + problemNum ?? 0, + ); + } + }); + + return startUserInput; +} + +// Then in component: +function handleUserInput( + id: string, + nextUserInput: UserInputMap[keyof UserInputMap], + widgetsEmpty: boolean, +) { + const next = { + ...userInput, + [id]: nextUserInput, // ← Update only this widget's input + }; + setUserInput(next); + props.handleUserInput?.(next, widgetsEmpty); +} +``` + +--- + +## 3. onChange Pattern Status + +### Current Status: DEPRECATION IN PROGRESS + +**The `onChange` pattern is being removed but still exists in legacy code.** + +#### Where it's still used: + +1. **Radio Widget** (partially during migration) + - File: `/packages/perseus/src/widgets/radio/radio.ff.tsx` + - Type definition: `/packages/perseus/src/types.ts` line 117 + - Status: TODO marked for removal (LEMS-3542, LEMS-3245) + +2. **Types Still Define It** + - File: `/packages/perseus/src/types.ts` + ```typescript + /** + * TODO(LEMS-3245) remove ChangeHandler + * @deprecated + */ + export type ChangeHandler = ( + arg1: { + hints?: ReadonlyArray; + replace?: boolean; + content?: string; + widgets?: PerseusWidgetsMap; + images?: ImageDict; + choiceStates?: ReadonlyArray; + // ... more fields ... + }, + callback?: () => void, + silent?: boolean, + ) => unknown; + ``` + +#### Where it's NOT used: + +- **NumericInput**: Uses `handleUserInput` directly +- **Most modern widgets**: Use `handleUserInput` callback +- **All new widget code**: Should use the props-driven approach + +#### Migration Path + +The Radio widget wrapper (`radio.ff.tsx`) bridges old and new: + +```typescript +class Radio extends React.Component implements Widget { + _handleChange(arg: {choiceStates?: ReadonlyArray}) { + const newChoiceStates = arg.choiceStates; + // ... converts ChoiceState to UserInput ... + const props = this._mergePropsAndState(); + // Use getUserInputFromSerializedState to get proper UserInput format + const userInput = getUserInputFromSerializedState(mergedProps, ...); + this.props.handleUserInput(userInput); // ← Calls new pattern + } +} +``` + +--- + +## 4. Widget Interface and Lifecycle + +### Widget Interface Definition + +**File**: `/packages/perseus/src/types.ts` lines 60-93 + +```typescript +export interface Widget { + /** + * don't use isWidget; it's just a dummy property to help TypeScript's + * weak typing to recognize non-interactive widgets as Widgets + * @deprecated + */ + isWidget?: true; + + focus?: () => { + id: string; + path: FocusPath; + } | boolean; + + getDOMNodeForPath?: (path: FocusPath) => Element | Text | null; + + /** + * @deprecated - do not use in new code. + */ + getSerializedState?: () => SerializedState; + + blurInputPath?: (path: FocusPath) => void; + focusInputPath?: (path: FocusPath) => void; + getInputPaths?: () => ReadonlyArray; + + getPromptJSON?: () => WidgetPromptJSON; +} +``` + +### WidgetExports Pattern + +**File**: `/packages/perseus/src/types.ts` lines 468-512 + +```typescript +export type WidgetExports< + T extends React.ComponentType & Widget = React.ComponentType, + TUserInput = Empty, +> = Readonly<{ + name: string; + displayName: string; + + widget: T; // The component itself + + version?: Version; + isLintable?: boolean; + tracking?: Tracking; + + // Static widget support + getOneCorrectAnswerFromRubric?: (rubric: WidgetOptions) => string | null; + + // ← Initialization functions + getCorrectUserInput?: (widgetOptions: WidgetOptions) => TUserInput; + getStartUserInput?: ( + widgetOptions: WidgetOptions, + problemNum: number, + ) => TUserInput; + + // ← Deprecation functions + getUserInputFromSerializedState?: ( + serializedState: unknown, + widgetOptions?: WidgetOptions, + ) => TUserInput; +}>; +``` + +--- + +## 5. Package Structure and Dependencies + +### Package Organization + +``` +packages/ +├── perseus/ # Main renderer package (74.0.2) +│ ├── src/ +│ │ ├── renderer.tsx # Core renderer component +│ │ ├── server-item-renderer.tsx # Exercise renderer +│ │ ├── widget-container.tsx # Widget wrapper +│ │ ├── user-input-manager.tsx # State management +│ │ ├── widgets/ # Widget implementations +│ │ │ ├── radio/ +│ │ │ ├── numeric-input/ +│ │ │ ├── interactive-graphs/ +│ │ │ └── ... (40+ widget types) +│ │ ├── types.ts # Core type definitions +│ │ ├── widgets.ts # Widget registry +│ │ └── index.ts # Main exports +│ └── package.json +│ +├── perseus-core/ # Shared types & utilities (23.0.0) +│ ├── src/ +│ │ ├── data-schema.ts # Data type definitions +│ │ ├── scoring-functions/ # Widget-specific scoring +│ │ └── utils/generators # Test data generators +│ └── package.json +│ +├── perseus-score/ # Server-side scoring +│ ├── src/widgets/[type]/score-[type].ts +│ └── widgets/widget-registry.ts +│ +├── perseus-linter/ # Content validation +├── perseus-editor/ # Editor components +├── math-input/ # Math keypad components +└── ... (other packages) +``` + +### Dependencies + +**Perseus Package** depends on: +- `@khanacademy/perseus-core` - Type definitions +- `@khanacademy/perseus-score` - Scoring logic +- `@khanacademy/perseus-linter` - Content validation +- `@khanacademy/math-input` - Math input components +- Wonder Blocks components (UI library) +- React/ReactDOM + +**Perseus-Core** dependencies: +- `@khanacademy/kas` - CAS system +- `@khanacademy/perseus-utils` - Shared utilities +- `@khanacademy/pure-markdown` - Markdown parser +- `tiny-invariant` - Assertions + +### Import Boundaries + +**Rules** (from CLAUDE.md): +- Use package aliases: `@khanacademy/perseus`, `@khanacademy/perseus-core` +- NO file extensions in imports (TypeScript file) +- NO cross-package relative imports +- Import order: builtin > external > internal > relative > types + +**Example correct imports**: +```typescript +import React from "react"; // builtin +import {ApiOptions} from "@khanacademy/perseus"; // internal package +import {WidgetContainer} from "../widget-container"; // relative + +import type {WidgetProps} from "@khanacademy/perseus-core"; +``` + +--- + +## 6. Module Boundaries and Inter-Widget Communication + +### WidgetContainer Boundary + +**File**: `/packages/perseus/src/widget-container.tsx` + +```typescript +type Props = { + shouldHighlight: boolean; + type: string; // ← Widget type + id: string; // ← Widget ID + widgetProps: WidgetProps; // ← All props + linterContext: LinterContextProps; +}; + +class WidgetContainer extends React.Component { + render() { + const WidgetType = Widgets.getWidget(this.props.type); + if (WidgetType == null) { + console.warn(`Widget type '${this.props.type}' not found!`); + return
; + } + + return ( + + + + ); + } +} +``` + +**Responsibilities**: +- Looks up widget component from registry +- Handles error boundaries +- Measures container size on mobile +- Passes props through without modification + +### Inter-Widget Communication + +**File**: `/packages/perseus/src/renderer.tsx` lines 626-683 + +```typescript +findInternalWidgets = (filterCriterion: FilterCriterion) => { + // Returns widgets matching filter + // Filters can be: + // - Widget ID: "interactive-graph 3" + // - Widget type: "interactive-graph" + // - Function: (id, widgetInfo, widget) => boolean +}; + +findWidgets = (filterCriterion: FilterCriterion) => { + // Combines internal and external widgets + return [ + ...this.findInternalWidgets(filterCriterion), + ...this.props.findExternalWidgets(filterCriterion), + ]; +}; +``` + +**Usage**: Passed to all widgets as `findWidgets` prop, enabling: +- Graded group widgets to find their children +- Custom widgets to communicate with other widgets +- Cross-renderer communication through parent's `findExternalWidgets` + +--- + +## 7. Key Files and Their Responsibilities + +| File | Responsibility | +|------|-----------------| +| `server-item-renderer.tsx` | Top-level exercise renderer; manages hints | +| `renderer.tsx` | Core question renderer; widget lifecycle | +| `widget-container.tsx` | Widget wrapper; error boundaries | +| `user-input-manager.tsx` | Central state management for user input | +| `types.ts` | Core type definitions (Widget, WidgetProps, etc.) | +| `widgets.ts` | Widget registry and lookup | +| `widgets/[type]/index.ts` | Widget export entry point | +| `widgets/[type]/[type].ts` | Widget registration (WidgetExports) | +| `widgets/[type]/[component].tsx` | Widget component implementation | + +--- + +## 8. Recent Architecture Changes + +### Recent PRs (from git log): +1. **Upgrade to Jest v30** (#3230) - Testing infrastructure +2. **Fix parsing of answerless Label Image widgets** (#3231) - Bug fix +3. **Remove unused `isItemRenderableByVersion`** (#3229) - Code cleanup +4. **Delete TODOs we're never going to fix** (#3197) - Debt reduction +5. **Simplify applyDefaultsToWidgets** (#3217) - Refactoring + +### Known Deprecations + +- `getSerializedState()` - Use `getStartUserInput()` instead (LEMS-3185) +- `onChange` handler - Use `handleUserInput()` instead (LEMS-3245, LEMS-3542) +- `ChangeHandler` type - Being phased out +- Old Radio widget files - Being replaced (LEMS-2994) + +--- + +## 9. Testing Architecture + +### Test Patterns + +**Location**: Widgets typically have `[widget].test.ts` or `.test.tsx` files + +**Key testing helpers** (from `packages/perseus-core/src/utils/generators`): +- Widget generators for creating test data +- testdata files for example questions + +**Example structure** (from CLAUDE.md): +```typescript +import {render, screen} from "@testing-library/react"; +import {userEvent} from "@testing-library/user-event"; +import {question1} from "../__testdata__/widget.testdata"; +import WidgetComponent from "../widget-component"; + +describe("WidgetComponent", () => { + it("renders correctly", () => { + render(); + expect(screen.getByRole("button")).toBeInTheDocument(); + }); + + it("handles user interaction", async () => { + const user = userEvent.setup(); + const onChange = jest.fn(); + render(); + await user.click(screen.getByRole("button")); + expect(onChange).toHaveBeenCalled(); + }); +}); +``` + +--- + +## 10. Key Insights and Findings + +### Ground Truth: Modern Widget Pattern + +1. **No onChange**: Modern widgets use `handleUserInput(newState)` callback +2. **Props-driven**: Widget state is passed via props, not stored internally +3. **Centralized state**: `UserInputManager` holds all widget states in one place +4. **Lazy initialization**: Widgets use `getStartUserInput()` to initialize state + +### Current Transition State + +- **Radio widget**: Partially migrated (still uses `onChange` internally) +- **NumericInput widget**: Fully migrated to modern pattern +- **Legacy code**: Still contains `onChange` and `ChangeHandler` types +- **Feature flags**: Used to conditionally render new Radio widget (radio.ff.tsx) + +### Architecture Strengths + +1. **Clear separation**: Renderer, WidgetContainer, Widgets have distinct roles +2. **Type safety**: Extensive use of TypeScript generics for widget props +3. **Extensibility**: Widget registry system allows plugins +4. **Testability**: Props-driven design makes widgets easy to test +5. **Error handling**: Error boundaries at widget level + +### Areas of Complexity + +1. **State shape duality**: Some widgets still have both `ChoiceState` and `UserInput` +2. **Serialization deprecation**: Old serialized state format still supported for backwards compatibility +3. **Focus management**: FocusPath system is complex but necessary for accessibility +4. **Feature flags**: Multiple codepaths for old vs. new Radio implementation + +--- + +## Conclusion + +Perseus has a well-designed, modern React architecture with clear data flow. The system is transitioning from widget-centric state management (onChange) to centralized state management (UserInputManager + handleUserInput callbacks). This research shows the actual current state of the codebase, which differs from some of the documentation in that modern widgets are fully prop-driven with no onChange pattern at all. + +The key takeaway: **When building or modifying widgets, use `handleUserInput()` callbacks, NOT `onChange` handlers**. The latter is deprecated and being actively removed. + +--- + +**Co-authored by**: Claude (AI Assistant) +**Research Depth**: Thorough - examined 15+ source files and traced data flow through the entire system diff --git a/.claude/prompts/README.md b/.claude/prompts/README.md new file mode 100644 index 00000000000..e7644b8c47f --- /dev/null +++ b/.claude/prompts/README.md @@ -0,0 +1,33 @@ +# Perseus Prompt Library + +This directory contains modular documentation for AI assistants working on the Perseus codebase. Each document focuses on a specific aspect of development. + +## Available Documents + +### Core References +- **[codebase-map.md](./codebase-map.md)** - Package structure, data flow, and module boundaries +- **[widget-development.md](./widget-development.md)** - Complete guide for creating and maintaining widgets +- **[testing-best-practices.md](./testing-best-practices.md)** - Testing patterns, utilities, and guidelines + +### Development Guidelines +- **[component-best-practices.md](./component-best-practices.md)** - React components, Wonder Blocks, accessibility +- **[file-organization.md](./file-organization.md)** - Directory structure, naming conventions, imports +- **[iteration-and-feedback.md](./iteration-and-feedback.md)** - When to ask for help vs. iterate autonomously + +## How to Use + +These documents are referenced from the main `CLAUDE.md` file. When working on specific tasks, the relevant documents will be loaded into context. + +## Maintenance + +When updating these documents: +1. Verify information against actual code (not just documentation) +2. Use concrete examples from the codebase +3. Include file paths and line numbers where helpful +4. Mark deprecated patterns clearly +5. Date any time-sensitive information + +## Recent Updates + +- **2024-02-05**: Initial prompt library created based on current codebase analysis +- **2024-02-05**: Corrected widget patterns after discovering Rubric is still used (but only for scoring) \ No newline at end of file diff --git a/.claude/prompts/codebase-map.md b/.claude/prompts/codebase-map.md new file mode 100644 index 00000000000..14ffc21d23e --- /dev/null +++ b/.claude/prompts/codebase-map.md @@ -0,0 +1,95 @@ +# Perseus Codebase Map + +## Package Structure Overview + +``` +packages/ +├── perseus/ (v74.0.2) # Core rendering engine +│ ├── src/ +│ │ ├── widgets/ # Interactive components +│ │ ├── components/ # Reusable UI components +│ │ ├── renderers/ # Content renderers +│ │ ├── util/ # Utility functions +│ │ └── __docs__/ # Storybook documentation +│ └── __testdata__/ # Test fixtures +│ +├── perseus-editor/ # Content authoring tools +│ ├── src/ +│ │ ├── components/ # Editor UI components +│ │ └── widgets/ # Widget editors +│ +├── perseus-core/ (v23.0.0) # Shared types and utilities +│ ├── src/ +│ │ ├── data-schema.ts # Core type definitions +│ │ ├── utils/ +│ │ │ └── generators/ # Test data generators +│ │ └── types/ # Type exports +│ +├── perseus-score/ # Server-side scoring +│ ├── src/ +│ │ └── widgets/ # Widget-specific scoring +│ +├── math-input/ # Math keypad and input +│ ├── src/ +│ │ ├── components/ # Math UI components +│ │ └── services/ # Math processing +│ +└── perseus-linter/ # Content validation + └── src/ + └── rules/ # Linting rules +``` + +## Key Entry Points + +### Renderers & State Management +- `packages/perseus/src/server-item-renderer.tsx` - Main exercise renderer +- `packages/perseus/src/user-input-manager.ts` - Centralized state management +- `packages/perseus/src/renderer.tsx` - Base renderer component +- `packages/perseus/src/widget-container.tsx` - Widget boundary layer + +### Widget System +- `packages/perseus/src/widgets.ts` - Widget registry +- Individual widgets in `packages/perseus/src/widgets/[name]/` +- Modern widgets use `handleUserInput()` callback pattern +- Legacy widgets being migrated from `onChange` pattern + +### Type System +- `packages/perseus-core/src/data-schema.ts` - Core types +- `packages/perseus-core/src/types/` - Type exports +- Widget-specific types in each widget directory + +## Modern Data Flow (Post-Migration) + +1. **State Initialization**: + - ServerItemRenderer → UserInputManager.getStartUserInput() + +2. **User Interaction**: + - Widget → handleUserInput() → UserInputManager → Re-render + +3. **Props Distribution**: + - UserInputManager.getWidgetProps() → WidgetContainer → Widget + +4. **Scoring**: + - UserInputManager state → Perseus Score → Validation result + +## Important Migration Notes + +- **onChange is DEPRECATED** (LEMS-3245, LEMS-3542) +- New widgets must use `handleUserInput()` pattern +- Radio widget currently bridges old/new patterns during migration +- NumericInput is fully modernized example + +## Module Boundaries + +- **perseus**: Public rendering API (React-based) +- **perseus-editor**: Editor-only functionality +- **perseus-core**: Shared types (no React dependencies) +- **perseus-score**: Server-side scoring (no React) +- **math-input**: Standalone math components + +## Import Rules + +- Use package aliases: `@khanacademy/perseus`, etc. +- NO file extensions in imports +- NO cross-package relative imports +- Import order: builtin > external > internal > relative > types \ No newline at end of file diff --git a/.claude/prompts/component-best-practices.md b/.claude/prompts/component-best-practices.md new file mode 100644 index 00000000000..cf4cfcd69d2 --- /dev/null +++ b/.claude/prompts/component-best-practices.md @@ -0,0 +1,324 @@ +# Component Development Best Practices + +## Component Structure + +### Prefer Functional Components +```typescript +// ✅ Good - Named function +function ComponentName(props: Props) { + return
{props.content}
; +} +ComponentName.displayName = "ComponentName"; // Explicit for debugging + +// ❌ Avoid - Arrow function requires displayName +const ComponentName = (props: Props) => { + return
{props.content}
; +}; +ComponentName.displayName = "ComponentName"; // Extra step +``` + +### Type Definitions +```typescript +// Place types at the top of the file +type Props = { + content: string; + onAction: () => void; + optional?: boolean; +}; + +// For complex props, use intersection types +type Props = BaseProps & { + specificProp: string; +}; +``` + +## Wonder Blocks Integration + +Perseus uses Khan Academy's Wonder Blocks design system: + +```typescript +import {View} from "@khanacademy/wonder-blocks-core"; +import {Button} from "@khanacademy/wonder-blocks-button"; +import {TextField} from "@khanacademy/wonder-blocks-form"; +import {LabelMedium} from "@khanacademy/wonder-blocks-typography"; + +function MyComponent() { + return ( + + Label text + + + + ); +} +``` + +## State Management Patterns + +### Component State (UI only) +```typescript +// Local state for UI concerns only +function Component() { + const [isExpanded, setIsExpanded] = React.useState(false); + const [hoveredIndex, setHoveredIndex] = React.useState(null); + + // User answer state goes through handleUserInput, not local state! +} +``` + +### Derived State +```typescript +// Calculate values instead of storing them +function Component({items, filter}: Props) { + // ✅ Good - Derived from props + const filteredItems = React.useMemo( + () => items.filter(item => item.matches(filter)), + [items, filter] + ); + + // ❌ Bad - Duplicating prop data in state + const [filteredItems, setFilteredItems] = React.useState([]); + React.useEffect(() => { + setFilteredItems(items.filter(...)); + }, [items, filter]); +} +``` + +## Performance Optimization + +### Memoization +```typescript +// Memoize expensive calculations +const expensiveValue = React.useMemo(() => { + return computeExpensiveValue(data); +}, [data]); + +// Memoize callbacks passed to children +const handleClick = React.useCallback(() => { + doSomething(value); +}, [value]); + +// Memoize component when props comparison is cheaper than re-render +const MemoizedChild = React.memo(ChildComponent); +``` + +### Avoid Inline Objects/Functions +```typescript +// ❌ Bad - Creates new object every render + + +// ✅ Good - Stable reference +const style = {margin: 10}; + + +// Or use Wonder Blocks styling + +``` + +## Accessibility + +### ARIA Labels +```typescript + +``` + +### Focus Management +```typescript +function Component() { + const inputRef = React.useRef(null); + + // Auto-focus on mount when appropriate + React.useEffect(() => { + if (shouldAutoFocus) { + inputRef.current?.focus(); + } + }, [shouldAutoFocus]); + + return ; +} +``` + +### Keyboard Navigation +```typescript +function handleKeyDown(e: React.KeyboardEvent) { + switch (e.key) { + case "Enter": + submitAnswer(); + break; + case "Escape": + clearInput(); + break; + case "ArrowUp": + selectPrevious(); + e.preventDefault(); // Prevent scroll + break; + } +} +``` + +## Error Handling + +### Error Boundaries +```typescript +// Components are wrapped by error boundaries +// Provide meaningful error states +function Component({data}: Props) { + if (!data) { + return No data available; + } + + try { + return ; + } catch (error) { + return Failed to render content; + } +} +``` + +## Mobile Support + +### Touch-Friendly Interfaces +```typescript +// Use Wonder Blocks components (mobile-optimized) +import {Button} from "@khanacademy/wonder-blocks-button"; + +// Ensure touch targets are at least 44x44px + +``` + +### Responsive Layout +```typescript +import {useIsMobile} from "../hooks/use-is-mobile"; + +function Component() { + const isMobile = useIsMobile(); + + return ( + + {/* Content adapts to screen size */} + + ); +} +``` + +## Component Organization + +### File Structure +```typescript +// component-name.tsx + +// 1. Imports +import React from "react"; +import {View} from "@khanacademy/wonder-blocks-core"; + +// 2. Type definitions +type Props = { + // ... +}; + +// 3. Constants +const DEFAULT_VALUE = ""; + +// 4. Main component +function ComponentName(props: Props) { + // ... +} + +// 5. Display name +ComponentName.displayName = "ComponentName"; + +// 6. Sub-components (if needed) +function SubComponent() { + // ... +} + +// 7. Export +export default ComponentName; +``` + +### Props Patterns +```typescript +// Destructure common props +function Component({ + userInput, + handleUserInput, + trackInteraction, + ...restProps +}: Props) { + // Use restProps for pass-through + return ; +} +``` + +## Testing Components + +### Render Testing +```typescript +import {render, screen} from "@testing-library/react"; +import {userEvent} from "@testing-library/user-event"; + +it("handles interaction", async () => { + const user = userEvent.setup(); + const onClick = jest.fn(); + + render(); + await user.click(screen.getByRole("button")); + + expect(onClick).toHaveBeenCalled(); +}); +``` + +### Snapshot Testing (use sparingly) +```typescript +it("renders complex structure", () => { + const {container} = render(); + expect(container).toMatchSnapshot(); +}); +``` + +## Common Patterns + +### Conditional Rendering +```typescript +// Simple conditions +{showHint && } + +// Multiple conditions +{(() => { + if (loading) return ; + if (error) return ; + return ; +})()} +``` + +### Lists and Keys +```typescript +{items.map((item, index) => ( + handleSelect(item.id)} + /> +))} +``` + +## Anti-Patterns to Avoid + +1. **Don't mutate props or state directly** +2. **Don't use array index as key when list can reorder** +3. **Don't put business logic in components** - Keep in utilities +4. **Don't use inline styles** - Use Wonder Blocks or CSS modules +5. **Don't forget React.memo for expensive child components** +6. **Don't use useEffect for derived state** - Use useMemo instead \ No newline at end of file diff --git a/.claude/prompts/file-organization.md b/.claude/prompts/file-organization.md new file mode 100644 index 00000000000..164c6b47064 --- /dev/null +++ b/.claude/prompts/file-organization.md @@ -0,0 +1,271 @@ +# File and Folder Organization + +## Directory Naming Conventions + +### Use Kebab-Case for Directories +``` +✅ Good: +packages/perseus/src/widgets/numeric-input/ +packages/perseus/src/widgets/interactive-graph/ + +❌ Bad: +packages/perseus/src/widgets/NumericInput/ +packages/perseus/src/widgets/interactive_graph/ +``` + +## Widget File Structure + +Each widget should follow this structure: +``` +widgets/widget-name/ +├── index.ts # Public exports only +├── widget-name.tsx # Main component +├── widget-name.test.ts # Component tests +├── widget-name.testdata.ts # Test data using generators +├── types.ts # Widget-specific types (if complex) +├── utils.ts # Widget-specific utilities (if needed) +└── __docs__/ + ├── widget-name.stories.tsx # Storybook stories + ├── a11y.mdx # Accessibility documentation + └── examples/ # Example JSON fixtures (if needed) +``` + +## Component Organization + +### Simple Components (single file) +``` +components/ +├── simple-component.tsx +├── simple-component.test.tsx +└── simple-component.stories.tsx +``` + +### Complex Components (folder structure) +``` +components/complex-component/ +├── index.ts # Public exports +├── complex-component.tsx # Main component +├── complex-component.test.tsx # Tests +├── sub-component.tsx # Private sub-components +├── utils.ts # Component-specific utils +└── types.ts # Component-specific types +``` + +## File Naming Rules + +### TypeScript/JavaScript Files +```typescript +// Components - PascalCase file, named export +widget-name.tsx → export function WidgetName() {} + +// Utilities - kebab-case file, named exports +math-utils.ts → export function calculateSum() {} + +// Types - kebab-case file with .types.ts extension +validation.types.ts → export type ValidationResult = {} + +// Test files - match source with .test.ts +widget-name.tsx → widget-name.test.ts + +// Test data - match source with .testdata.ts +widget-name.tsx → widget-name.testdata.ts +``` + +### Index Files +```typescript +// index.ts should only re-export, no logic +export {default} from "./widget-name"; +export type {WidgetNameProps} from "./types"; + +// Don't put implementation in index files +``` + +## Import Organization + +### Import Order (enforced by ESLint) +```typescript +// 1. Node built-ins +import fs from "fs"; +import path from "path"; + +// 2. External packages +import React from "react"; +import {render} from "@testing-library/react"; + +// 3. Internal packages (Khan Academy) +import {View} from "@khanacademy/wonder-blocks-core"; +import {PerseusScore} from "@khanacademy/perseus-core"; + +// 4. Relative imports +import {WidgetContainer} from "../widget-container"; +import {calculateAnswer} from "./utils"; + +// 5. Type imports (always last) +import type {WidgetProps} from "@khanacademy/perseus-core"; +import type {LocalType} from "./types"; +``` + +## Test File Organization + +### Test Data Files +```typescript +// widget-name.testdata.ts +import {widgetNameGenerator} from "@khanacademy/perseus-core"; + +// Group related test cases +export const basicQuestions = { + simple: widgetNameGenerator.build({...}), + withHint: widgetNameGenerator.build({...}), + multipleChoice: widgetNameGenerator.build({...}), +}; + +export const edgeCases = { + empty: widgetNameGenerator.build({...}), + veryLong: widgetNameGenerator.build({...}), +}; +``` + +### Test Structure +```typescript +// widget-name.test.ts +describe("WidgetName", () => { + describe("rendering", () => { + it("renders with default props", () => {}); + it("renders with custom props", () => {}); + }); + + describe("user interaction", () => { + it("handles input changes", () => {}); + it("validates input", () => {}); + }); + + describe("scoring", () => { + it("scores correct answer", () => {}); + it("scores incorrect answer", () => {}); + }); +}); +``` + +## Package Exports + +### Main Package Exports (packages/perseus/src/index.ts) +```typescript +// Group exports by category +// Renderers +export {default as ServerItemRenderer} from "./server-item-renderer"; +export {default as ArticleRenderer} from "./article-renderer"; + +// Widgets +export {widgets} from "./widgets"; + +// Types +export type {WidgetProps} from "./types"; +``` + +### Package.json Exports +```json +{ + "exports": { + ".": "./dist/index.js", + "./styles": "./dist/styles.css" + } +} +``` + +## Documentation Files + +### Required Documentation +``` +widget-name/ +└── __docs__/ + ├── widget-name.stories.tsx # Interactive examples + ├── a11y.mdx # Accessibility guide + └── README.mdx # Usage documentation (optional) +``` + +### Storybook Organization +```typescript +// __docs__/widget-name.stories.tsx +export default { + title: "Perseus/Widgets/WidgetName", // Hierarchical organization + component: WidgetName, +}; + +// Stories in logical order +export const Default = {}; +export const WithHint = {}; +export const Mobile = {}; +export const ErrorState = {}; +``` + +## Asset Organization + +### Static Assets +``` +packages/perseus/src/ +├── assets/ +│ └── images/ # Shared images +├── widgets/ +│ └── widget-name/ +│ └── assets/ # Widget-specific assets +``` + +### Styles +``` +packages/perseus/src/ +├── styles/ +│ ├── global.less # Global styles +│ └── variables.less # Shared variables +├── widgets/ +│ └── widget-name/ +│ └── widget-name.less # Widget-specific styles +``` + +## Build Artifacts + +### Git Ignored +``` +# Never commit these +node_modules/ +dist/ +build/ +*.tsbuildinfo +coverage/ +.turbo/ +``` + +### Generated Files +```typescript +// Mark generated files clearly +// Generated by: script-name.js +// DO NOT EDIT MANUALLY +``` + +## Migration Patterns + +### Gradual Migration +``` +widgets/old-widget/ # Legacy structure +├── old-widget.jsx # To be migrated +├── old-widget-new.tsx # New implementation +└── __migration__/ # Migration utilities +``` + +### Deprecation +```typescript +// Mark deprecated code clearly +/** + * @deprecated Use NewWidget instead (LEMS-1234) + * @removeBefore 2024-12-01 + */ +export const OldWidget = () => {}; +``` + +## Best Practices + +1. **Keep files focused** - Single responsibility per file +2. **Co-locate related code** - Tests, stories, and component together +3. **Use index.ts for public API** - Hide internal implementation +4. **Consistent naming** - Follow conventions strictly +5. **Clean imports** - No circular dependencies +6. **Document complex structures** - Add README.md when needed \ No newline at end of file diff --git a/.claude/prompts/iteration-and-feedback.md b/.claude/prompts/iteration-and-feedback.md new file mode 100644 index 00000000000..bcf293a0b2b --- /dev/null +++ b/.claude/prompts/iteration-and-feedback.md @@ -0,0 +1,421 @@ +# Iteration and Feedback Guidelines + +## When to Seek Human Feedback + +### Always Ask When: +1. **Architectural decisions** - Choosing between different design patterns +2. **Breaking changes** - Modifications that affect external APIs +3. **Performance trade-offs** - Optimizing for speed vs. memory vs. readability +4. **Business logic ambiguity** - Multiple valid interpretations of requirements +5. **User experience changes** - Altering how users interact with features +6. **Data model changes** - Modifying core types or data structures + +### Example Decision Points: +```typescript +// ASK: Which error handling approach? +// Option A: Show inline error +// Option B: Toast notification +// Option C: Modal dialog + +// ASK: State management strategy? +// Option A: Local component state +// Option B: Centralized in UserInputManager +// Option C: Context provider +``` + +## Autonomous Iteration Patterns + +### Can Iterate Without Asking: +1. **Fixing clear bugs** - Null pointer, type errors, off-by-one errors +2. **Following established patterns** - Using existing widget patterns +3. **Code formatting** - Prettier, ESLint fixes +4. **Adding missing tests** - Improving coverage for existing code +5. **Refactoring for clarity** - Extracting functions, renaming variables +6. **Performance optimizations** - When metrics clearly show improvement + +### Iteration Workflow: +```typescript +// 1. Initial implementation +function calculateScore(answer: string): number { + return answer === "42" ? 1 : 0; +} + +// 2. Identify issue (too simplistic) +// Can iterate autonomously because pattern is clear + +// 3. Improved implementation +function calculateScore( + userInput: UserInput, + rubric: Rubric, +): PerseusScore { + const isCorrect = userInput.value === rubric.correctAnswer; + return { + type: "points", + earned: isCorrect ? 1 : 0, + total: 1, + message: null, + }; +} +``` + +## Testing Feedback Loop + +### Run Tests → Analyze → Fix → Repeat +```bash +# Autonomous iteration process +pnpm test widget-name + +# If test fails with clear error: +# ✅ Fix autonomously + +# If test fails with ambiguous requirement: +# ❌ Ask for clarification +``` + +### Test Failure Decision Tree: +``` +Test Failure +├── Type Error → Fix autonomously +├── Assertion Failure +│ ├── Clear expectation → Fix autonomously +│ └── Business logic unclear → Ask human +├── Timeout +│ ├── Missing async/await → Fix autonomously +│ └── Performance issue → Discuss approach +└── Snapshot Mismatch + ├── Intentional change → Update snapshot + └── Unexpected change → Investigate cause +``` + +## Code Review Self-Checklist + +### Before Presenting Code: +- [ ] Tests pass (`pnpm test`) +- [ ] Types check (`pnpm tsc`) +- [ ] Linted (`pnpm lint`) +- [ ] Formatted (`pnpm prettier . --write`) +- [ ] Accessibility documented +- [ ] Mobile tested (if applicable) +- [ ] Performance impact considered + +### Autonomous Fixes: +```typescript +// These issues should be fixed without asking: + +// ❌ Unused import (ESLint) +import {unused} from "./utils"; + +// ❌ Console statement (ESLint) +console.log("debug"); + +// ❌ Missing type (TypeScript) +function process(data) { // Should be: data: DataType + +// ❌ Formatting (Prettier) +const x={a:1,b:2}; // Should be: const x = {a: 1, b: 2}; +``` + +## Incremental Development + +### Build → Test → Refine Cycle +```typescript +// Iteration 1: Basic functionality +function Widget() { + return ; +} + +// Iteration 2: Add user input handling (autonomous) +function Widget({userInput, handleUserInput}) { + return ( + handleUserInput({value: e.target.value})} + /> + ); +} + +// Iteration 3: Add validation (check pattern first) +// If validation rules are clear → implement +// If business rules unclear → ask +``` + +## Performance Optimization Decisions + +### Autonomous Optimizations: +```typescript +// Clear performance win - optimize without asking +// Before: O(n²) +const hasDuplicate = items.some( + (item, i) => items.slice(i + 1).includes(item) +); + +// After: O(n) +const hasDuplicate = items.length !== new Set(items).size; +``` + +### Needs Discussion: +```typescript +// Performance vs. Readability trade-off +// Option A: Readable but slower +const result = items + .filter(item => item.active) + .map(item => item.value) + .reduce((sum, val) => sum + val, 0); + +// Option B: Faster but less readable +let result = 0; +for (let i = 0; i < items.length; i++) { + if (items[i].active) result += items[i].value; +} +// ASK: Which approach aligns with team preferences? +``` + +## Error Handling Patterns + +### Autonomous Error Handling: +```typescript +// Null checks - add without asking +function process(data?: DataType) { + if (!data) { + return defaultValue; // Clear fallback + } + // ... process data +} + +// Try-catch for parsing - add without asking +try { + const parsed = JSON.parse(userInput); + return parsed; +} catch { + return null; // Safe fallback +} +``` + +### Needs Human Input: +```typescript +// User-facing error messages +function validateInput(value: string) { + if (!value) { + // ASK: What message helps users best? + // Option A: "This field is required" + // Option B: "Please enter your answer" + // Option C: "Answer cannot be empty" + } +} +``` + +## Documentation Decisions + +### Auto-document: +- Public APIs (JSDoc for complex functions) +- Accessibility requirements +- Component props +- Test scenarios + +### Ask Before Documenting: +- Architecture decisions (ADRs) +- Migration guides +- Public-facing documentation +- API breaking changes + +## Useful Feedback Prompts + +When uncertain, use these prompts: + +1. **"I see two approaches here..."** - Present options with trade-offs +2. **"The tests suggest X but the code does Y..."** - Highlight inconsistencies +3. **"This would be cleaner if..."** - Propose refactoring +4. **"I'm assuming X because..."** - Make assumptions explicit +5. **"Would you prefer..."** - Get style/pattern preferences + +## Continuous Improvement + +### Track Patterns: +```typescript +// When you make the same fix repeatedly, suggest: +"I notice we're frequently fixing X. Should we add a linting rule?" +"This pattern appears often. Should we create a utility function?" +"Multiple widgets need this. Should we move it to shared code?" +``` + +### Learn from Feedback: +- Note preference corrections +- Update approach based on feedback +- Suggest codifying patterns in documentation + +## Debugging Practices + +### Console Debugging +```typescript +// Temporary debugging (remove before commit) +console.log("Widget state:", state); +console.log("Props received:", props); +console.log("Calculated value:", result); + +// Better: Use descriptive messages +console.log("[WidgetName] Handling user input:", userInput); +console.log("[WidgetName] Validation result:", isValid); +``` + +### Remember to Clean Up +- Remove ALL console statements before commit +- ESLint will catch these in pre-commit +- Use debugger statements sparingly +- Clean up TODO comments + +### Error Boundaries +Perseus widgets are wrapped in error boundaries. When debugging: + +1. **Check browser console** for widget-specific errors +2. **Look for error boundary messages** in the UI +3. **Implement graceful fallbacks**: +```typescript +function WidgetComponent({data}: Props) { + if (!data || !data.required) { + // Graceful fallback + return Unable to load widget content; + } + + try { + return ; + } catch (error) { + // Log for debugging but show user-friendly message + console.error("[WidgetName] Render error:", error); + return This content is temporarily unavailable; + } +} +``` + +### Storybook Development & Debugging +Use Storybook for isolated development: + +1. **Test different prop combinations** - Create stories for edge cases +2. **Verify accessibility** - Use the a11y addon to catch issues +3. **Check mobile layouts** - Use device frame addon +4. **Debug interactions** - Use actions addon to log callbacks +5. **Visual debugging** - Compare component states side-by-side + +```typescript +// Useful Storybook patterns for debugging +export const DebugStory = { + args: { + // Test with extreme values + value: "Very long text that might break layout...", + options: Array(100).fill("Option"), + }, + play: async ({canvasElement}) => { + // Automated interaction testing + const canvas = within(canvasElement); + const input = canvas.getByRole("textbox"); + await userEvent.type(input, "test"); + }, +}; +``` + +## Deployment & Pre-Submit Checklist + +### Before Submitting PR + +#### Required Checks +```bash +# 1. Run full test suite +pnpm test + +# 2. Type checking +pnpm tsc +# or +pnpm build:types + +# 3. Linting (with auto-fix) +pnpm lint --fix + +# 4. Format code +pnpm prettier . --write + +# 5. Build packages to ensure no build errors +pnpm build + +# 6. Test in Storybook +pnpm storybook +``` + +#### Manual Verification +- [ ] Accessibility documented (new widgets need a11y.mdx) +- [ ] Mobile tested (touch interactions work) +- [ ] Performance impact assessed (no unnecessary re-renders) +- [ ] Error cases handled gracefully +- [ ] Console clear of debug statements + +### Common Pre-commit Failures & Fixes + +#### ESLint Failures +```typescript +// ❌ Unused imports +import {unused} from "./utils"; // Remove + +// ❌ Console statements +console.log("debug"); // Remove or use proper logging + +// ❌ Missing dependencies in hooks +useEffect(() => { + doSomething(value); +}, []); // Add 'value' to dependency array + +// ❌ Prefer const +let unchangedVar = 5; // Change to const +``` + +#### Prettier Failures +```typescript +// Auto-fix with: pnpm prettier . --write + +// Common issues: +// - Inconsistent quotes (use double quotes) +// - Missing semicolons +// - Incorrect indentation (4 spaces) +// - Line length > 80 characters +``` + +#### TypeScript Failures +```typescript +// ❌ Implicit any +function process(data) {} // Add type: data: DataType + +// ❌ Missing return type +function calculate(x: number) { // Add : number + return x * 2; +} + +// ❌ Type mismatch +const value: string = 42; // Fix type or value +``` + +#### Test Failures +```bash +# Run specific test to debug +pnpm test path/to/test.test.ts + +# Update snapshots if changes are intentional +pnpm test -u + +# Run with coverage +pnpm test --coverage +``` + +### Post-Submit Monitoring + +After PR is merged: +1. Monitor for any reverted changes +2. Check for related bug reports +3. Watch performance metrics +4. Respond to code review feedback + +### Rollback Preparation + +Be prepared to revert if issues arise: +```bash +# Create revert PR if needed +git revert +git push origin revert-branch +# Create PR with explanation of issue +``` \ No newline at end of file diff --git a/.claude/prompts/math-and-content.md b/.claude/prompts/math-and-content.md new file mode 100644 index 00000000000..b3185d85292 --- /dev/null +++ b/.claude/prompts/math-and-content.md @@ -0,0 +1,270 @@ +# Math and Content Rendering + +## Math Rendering (TeX) + +### Basic Syntax +- **Inline math**: Use `$...$` for math within text + - Example: `The answer is $x = 42$` +- **Display math**: Use `$$...$$` for centered, standalone equations + - Example: `$$\int_0^\infty e^{-x^2} dx = \frac{\sqrt{\pi}}{2}$$` + +### Complex Expressions + +#### Fractions +```latex +// ✅ Good - Use \dfrac for display-style fractions +$$\dfrac{a+b}{c+d}$$ + +// ❌ Avoid - \frac can be too small in complex expressions +$$\frac{a+b}{c+d}$$ + +// For inline, \frac is acceptable +The fraction $\frac{1}{2}$ is equivalent to 0.5 +``` + +#### Common Patterns +```latex +// Aligned equations +\begin{align} +x + y &= 10 \\ +2x - y &= 5 +\end{align} + +// Matrices +\begin{bmatrix} +a & b \\ +c & d +\end{bmatrix} + +// Cases +f(x) = \begin{cases} +x^2 & \text{if } x > 0 \\ +0 & \text{otherwise} +\end{cases} +``` + +### Testing Math Rendering + +Test math in different contexts: +1. **Articles** - Both inline and display math +2. **Exercise questions** - Main problem statement +3. **Hints** - Step-by-step solutions +4. **Answer choices** - In radio/multiple choice widgets +5. **Feedback messages** - Correct/incorrect explanations + +### MathJax Configuration + +Perseus uses MathJax for rendering. Key points: +- Automatic equation numbering disabled +- Custom macros available (check `math-input` package) +- Accessibility features enabled (screen reader support) + +### Common Math Issues + +#### Issue: Math not rendering +```typescript +// ❌ Problem: Unescaped backslashes +const tex = "The answer is $\frac{1}{2}$"; + +// ✅ Solution: Escape or use raw strings +const tex = "The answer is $\\frac{1}{2}$"; +const tex = String.raw`The answer is $\frac{1}{2}$`; +``` + +#### Issue: Spacing problems +```latex +// Add manual spacing when needed +$x\,\text{cm}$ // Thin space +$x\ \text{units}$ // Normal space +$x\quad\text{m}$ // Quad space +``` + +## Content Structure + +### Perseus Content Format +```json +{ + "question": { + "content": "What is $2 + 2$?", + "widgets": { + "numeric-input 1": { + "type": "numeric-input", + "options": { + "answers": [{ + "value": 4, + "status": "correct" + }] + } + } + } + }, + "hints": [ + {"content": "Think about counting on your fingers."}, + {"content": "The answer is $4$."} + ] +} +``` + +### Widget References + +Widgets are referenced in content using special syntax: +```markdown +Enter your answer: [[☃ numeric-input 1]] + +Choose the correct option: [[☃ radio 1]] +``` + +The snowman character (☃) is used as a unique delimiter. + +### Markdown Extensions + +Perseus extends standard Markdown: + +#### Columns +```markdown +::: columns +::: column +Left column content +::: +::: column +Right column content +::: +::: +``` + +#### Callout Boxes +```markdown +::: callout +**Important:** This is a callout box. +::: +``` + +#### Images with Alignment +```markdown +![Description](url.png){: .align-right} +``` + +## Content Validation + +### Linting Rules + +The `perseus-linter` package enforces: +- Valid widget references +- Proper math syntax +- Accessible image alt text +- No broken links +- Consistent formatting + +### Running the Linter +```bash +pnpm --filter perseus-linter lint-content content.json +``` + +## Accessibility in Content + +### Math Accessibility +- All math automatically gets ARIA labels +- MathJax provides screen reader support +- Test with screen readers for complex expressions + +### Image Guidelines +```markdown +// ✅ Good - Descriptive alt text +![Graph showing linear growth from 0 to 100 over 10 years](graph.png) + +// ❌ Bad - Generic or missing alt text +![Graph](graph.png) +![](graph.png) +``` + +### Widget Labels +```typescript +// Ensure widgets have proper labels + +``` + +## Performance Considerations + +### Math Rendering Performance +- Initial render can be slow for many expressions +- Use `React.memo` for components with static math +- Consider pagination for content with 50+ expressions + +### Content Loading +- Large exercises should lazy-load hints +- Images should have width/height to prevent reflow +- Preload critical assets + +## Testing Content Rendering + +### Unit Tests +```typescript +it("renders math expressions correctly", () => { + const {container} = render( + + ); + + // MathJax adds specific classes + expect(container.querySelector(".katex")).toBeInTheDocument(); +}); +``` + +### Visual Regression Tests +- Use Storybook for visual testing +- Snapshot complex math layouts +- Test responsive behavior + +### Manual Testing Checklist +- [ ] Math renders correctly +- [ ] Widgets display properly +- [ ] Images load and align correctly +- [ ] Content is responsive on mobile +- [ ] Screen reader announces math properly +- [ ] Print view works correctly + +## Common Content Patterns + +### Exercise with Multiple Parts +```json +{ + "question": { + "content": "Solve the system of equations:\n\n$$\\begin{align}x + y &= 10\\\\2x - y &= 5\\end{align}$$\n\nWhat is $x$? [[☃ numeric-input 1]]\n\nWhat is $y$? [[☃ numeric-input 2]]" + } +} +``` + +### Article with Interactive Elements +```markdown +# Understanding Quadratics + +The standard form of a quadratic equation is $ax^2 + bx + c = 0$. + +Try adjusting the parameters below: + +[[☃ interactive-graph 1]] + +Notice how changing $a$ affects the parabola's shape. +``` + +## Debugging Tips + +### Math Not Rendering +1. Check browser console for MathJax errors +2. Verify TeX syntax is valid +3. Ensure proper escaping in strings +4. Check for conflicting CSS + +### Widget Not Appearing +1. Verify widget ID matches reference +2. Check widget is registered +3. Ensure widget data is valid +4. Look for error boundaries in console + +### Content Layout Issues +1. Inspect element for CSS conflicts +2. Check responsive breakpoints +3. Verify image dimensions +4. Test with different viewport sizes \ No newline at end of file diff --git a/.claude/prompts/testing-best-practices.md b/.claude/prompts/testing-best-practices.md new file mode 100644 index 00000000000..8ae66a4ddf9 --- /dev/null +++ b/.claude/prompts/testing-best-practices.md @@ -0,0 +1,181 @@ +# Perseus Testing Best Practices + +## Test Structure + +### Follow the AAA Pattern +```typescript +describe("ComponentName", () => { + it("does something specific", () => { + // Arrange + const props = {...}; + + // Act + const result = doSomething(props); + + // Assert + expect(result).toBe(expected); + }); + + it("handles user interaction", async () => { + // Arrange, Act (when single action) + const {container} = render(); + + // Assert + expect(container).toMatchSnapshot(); + }); +}); +``` + +## Use Test Data Generators + +Located in `packages/perseus-core/src/utils/generators/`: + +```typescript +import {radioGenerator} from "@khanacademy/perseus-core"; + +// Generate complete widget props +const radioProps = radioGenerator.build({ + choices: ["Option A", "Option B"], + hasNoneOfTheAbove: false, +}); + +// Generate multiple variants +const testCases = radioGenerator.buildMany(3); +``` + +## Widget Testing Patterns + +### Modern Widget Pattern (with handleUserInput) +```typescript +import {renderWidget} from "../__tests__/test-utils"; +import {userEvent} from "@testing-library/user-event"; + +it("handles user input correctly", async () => { + // Arrange + const handleUserInput = jest.fn(); + const user = userEvent.setup(); + + renderWidget({ + widgetType: "numeric-input", + widgetProps: numericInputGenerator.build(), + handleUserInput, + }); + + // Act + await user.type(screen.getByRole("textbox"), "42"); + + // Assert + expect(handleUserInput).toHaveBeenCalledWith("42"); +}); +``` + +### Legacy Widget Pattern (with onChange - avoid for new tests) +```typescript +// Only for widgets not yet migrated +const onChange = jest.fn(); +render(); +``` + +## Testing Priorities + +1. **User interactions** - How users actually use the widget +2. **Accessibility** - Keyboard navigation, screen readers +3. **Edge cases** - Empty states, invalid input, boundaries +4. **Scoring logic** - Correct/incorrect answers +5. **Mobile behavior** - Touch interactions + +## Scoring Tests + +Located in `packages/perseus-score/src/widgets/[widget]/`: + +```typescript +import {score} from "../score-radio"; + +it("scores correct answer", () => { + const result = score(userInput, scoringData); + expect(result).toHaveBeenAnsweredCorrectly(); +}); + +it("validates input", () => { + const result = validate(userInput); + expect(result).toHaveInvalidInput("Choose an option"); +}); +``` + +## Snapshot Testing + +Use sparingly, only for: +- Complex rendered output structure +- Markdown/TeX rendering +- Error messages + +```typescript +it("renders complex math correctly", () => { + const {container} = render(); + expect(container).toMatchSnapshot(); +}); +``` + +## Test Utilities + +### Custom Matchers +```typescript +// In test files +expect(result).toHaveBeenAnsweredCorrectly(); +expect(result).toHaveInvalidInput("message"); +``` + +### Testing Library Preferences +- Use `screen` queries over container queries +- Prefer `getByRole` over `getByTestId` +- Use `userEvent` over `fireEvent` +- Wait for async operations with `waitFor` + +## Common Testing Patterns + +### Testing Focus Management +```typescript +it("manages focus correctly", async () => { + const user = userEvent.setup(); + render(); + + await user.tab(); + expect(screen.getByRole("button")).toHaveFocus(); +}); +``` + +### Testing Accessibility +```typescript +it("has accessible labels", () => { + render(); + expect(screen.getByLabelText("Answer")).toBeInTheDocument(); +}); +``` + +### Testing Mobile Interactions +```typescript +it("handles touch events", async () => { + const user = userEvent.setup(); + render(); + + await user.pointer({keys: "[TouchA>]", target: element}); + // Assert touch-specific behavior +}); +``` + +## What NOT to Test + +- Implementation details (internal state, private methods) +- Third-party library behavior +- Static types (TypeScript handles this) +- Styles and CSS (unless functionally important) + +## File Organization + +``` +widget-name/ +├── widget-name.tsx +├── widget-name.test.ts # Component tests +├── widget-name.testdata.ts # Test data using generators +└── score-widget-name.test.ts # Scoring tests (if applicable) +``` \ No newline at end of file diff --git a/.claude/prompts/widget-development.md b/.claude/prompts/widget-development.md new file mode 100644 index 00000000000..4557afdadfe --- /dev/null +++ b/.claude/prompts/widget-development.md @@ -0,0 +1,286 @@ +# Widget Development Guide + +## Three-Layer Type Architecture + +Perseus widgets use a three-layer type system: + +1. **WidgetOptions** - Widget configuration/display settings (in data-schema.ts) +2. **ValidationData** - Client-side validation data, no sensitive info (in validation.types.ts) +3. **Rubric** - Full scoring data: `ValidationData & WidgetOptions` (in validation.types.ts) + +## Creating a New Widget + +### 1. Directory Structure +``` +packages/perseus/src/widgets/[widget-name]/ +├── index.ts # Exports +├── [widget-name].tsx # Main component +├── [widget-name].test.ts # Tests +├── [widget-name].testdata.ts # Test data using generators +└── __docs__/ + ├── [widget-name].stories.tsx # Storybook stories + └── a11y.mdx # Accessibility docs +``` + +### 2. Modern Widget Implementation + +```typescript +// [widget-name].tsx +import React from "react"; +import type {PerseusWidgetNameUserInput} from "@khanacademy/perseus-core"; + +// Props are just the widget options spread directly, plus system props +type Props = { + // Widget-specific options (from WidgetOptions) + correctAnswer?: string; + placeholder?: string; + // System props + userInput: PerseusWidgetNameUserInput; + handleUserInput: (input: PerseusWidgetNameUserInput) => void; + trackInteraction: () => void; + onFocus: () => void; + onBlur: () => void; +}; + +function WidgetName(props: Props) { + const {userInput, handleUserInput} = props; + + const handleChange = (newValue: string) => { + handleUserInput({value: newValue}); + props.trackInteraction(); + }; + + return ( +
+ handleChange(e.target.value)} + onFocus={props.onFocus} + onBlur={props.onBlur} + placeholder={props.placeholder} + /> +
+ ); +} + +// IMPORTANT: Set displayName for debugging +WidgetName.displayName = "WidgetName"; + +// Export with WidgetExports interface +export default { + name: "widget-name" as const, + displayName: "Widget Display Name", + widget: WidgetName, + isLintable: true, + + // Optional scoring helpers (Rubric only used here, not in props!) + getOneCorrectAnswerFromRubric: (rubric: PerseusWidgetNameRubric) => { + return rubric.correctAnswer || null; + }, +} as WidgetExports; +``` + +### 3. Define Types in perseus-core + +```typescript +// packages/perseus-core/src/data-schema.ts + +// Widget configuration (what content creators set) +export type PerseusWidgetNameOptions = { + correctAnswer?: string; + placeholder?: string; + static?: boolean; // Common option for most widgets +}; + +// User's input +export type PerseusWidgetNameUserInput = { + value: string; +}; + +// packages/perseus-core/src/validation.types.ts + +// Client-side validation data (subset of options, no answers) +export type PerseusWidgetNameValidationData = { + placeholder?: string; +}; + +// Full scoring data (intersection type) +export type PerseusWidgetNameRubric = + PerseusWidgetNameValidationData & + PerseusWidgetNameOptions; +``` + +### 4. Register the Widget + +```typescript +// packages/perseus/src/widgets.ts +import widgetNameWidget from "./widgets/widget-name"; + +export const widgets = { + // ... existing widgets + "widget-name": widgetNameWidget, +}; +``` + +### 5. Add Scoring Function (if scorable) + +```typescript +// packages/perseus-score/src/widgets/widget-name/score-widget-name.ts +import type { + PerseusWidgetNameUserInput, + PerseusWidgetNameRubric, +} from "@khanacademy/perseus-core"; + +export function scoreWidgetName( + userInput: PerseusWidgetNameUserInput, + rubric: PerseusWidgetNameRubric, +): PerseusScore { + if (userInput.value === rubric.correctAnswer) { + return { + type: "points", + earned: 1, + total: 1, + message: null, + }; + } + return { + type: "points", + earned: 0, + total: 1, + message: null, + }; +} + +// Register in widget-registry.ts +import {scoreWidgetName} from "./widget-name/score-widget-name"; + +widgets["widget-name"] = { + score: scoreWidgetName, +}; +``` + +## Important: Props Pattern + +**What widgets receive as props:** +- Widget options spread directly (NOT wrapped in `options` or `rubric`) +- `userInput` - Current user state +- `handleUserInput` - State update callback +- System props (`trackInteraction`, `onFocus`, `onBlur`, etc.) + +**What widgets DON'T receive:** +- `rubric` prop (only used in scoring functions) +- `scoringData` or `validationData` props +- `onChange` prop (deprecated) + +## Widget State Management + +### Modern Pattern (Required for new widgets) +```typescript +// Receive state via userInput +const currentValue = props.userInput?.value || ""; + +// Update state via handleUserInput +const handleChange = (newValue: string) => { + props.handleUserInput({value: newValue}); + props.trackInteraction(); // Track user interaction +}; +``` + +### NO Local State for Answers +- User answers must be stored in UserInputManager +- Local React state only for UI concerns (hover, focus, etc.) + +## Testing Your Widget + +### Use Test Generators +```typescript +// [widget-name].testdata.ts +import {widgetNameGenerator} from "@khanacademy/perseus-core"; + +export const question1 = widgetNameGenerator.build({ + correctAnswer: "42", + placeholder: "Enter your answer", +}); +``` + +### Test Pattern +```typescript +import {renderQuestion} from "../__tests__/renderQuestion"; +import {question1} from "./widget-name.testdata"; + +describe("WidgetName", () => { + it("accepts user input", async () => { + // Arrange + const {renderer} = renderQuestion(question1); + + // Act + await renderer.setInputValue({ + widgetId: "widget-name 1", + value: "42", + }); + + // Assert + const score = renderer.score(); + expect(score).toHaveBeenAnsweredCorrectly(); + }); +}); +``` + +## Common Widget Patterns + +### Multiple Inputs +```typescript +type UserInput = { + numerator: string; + denominator: string; +}; + +const handleNumeratorChange = (value: string) => { + handleUserInput({ + ...userInput, + numerator: value, + }); +}; +``` + +### Validation (Optional) +```typescript +// packages/perseus-score/src/widgets/widget-name/validate-widget-name.ts +export function validateWidgetName( + userInput: PerseusWidgetNameUserInput, +): ValidationResult { + if (!userInput.value) { + return { + type: "invalid", + message: "Please enter an answer", + }; + } + return {type: "valid"}; +} +``` + +### Focus Management +```typescript +const inputRef = React.useRef(null); + +React.useImperativeHandle(props.widgetRef, () => ({ + focus: () => inputRef.current?.focus(), + blur: () => inputRef.current?.blur(), +})); +``` + +## Migration Notes + +- Radio widget is transitioning (TODO(LEMS-2994)) +- Numeric-input is fully modernized (good reference) +- Expression uses class component pattern (older style) +- New widgets should follow functional component pattern + +## Common Pitfalls + +1. **Don't expect rubric in props** - It's only for scoring +2. **Don't use onChange** - Use handleUserInput +3. **Don't forget displayName** - Required for debugging +4. **Don't store answers in local state** - Use UserInputManager +5. **Don't skip trackInteraction** - Needed for analytics +6. **Don't use cross-package relative imports** - Use @khanacademy/* aliases \ No newline at end of file diff --git a/.claude/widget-patterns-research.md b/.claude/widget-patterns-research.md new file mode 100644 index 00000000000..755029ce76c --- /dev/null +++ b/.claude/widget-patterns-research.md @@ -0,0 +1,551 @@ +# Perseus Widget Development Patterns Research + +**Research Date:** February 5, 2026 +**Conducted by:** Claude Code +**Co-Author:** Tamara Bozich + +## Executive Summary + +This document details the current widget development patterns in Perseus based on examination of actual codebase implementations. The key finding is that **Rubric is still the primary pattern**, not replaced by `scoringData` or `validationData` props. Widget types are split between widget configuration (in `PerseusWidgetOptions` from data-schema) and validation/scoring data (in validation.types.ts). + +--- + +## 1. Widget Type Definitions Architecture + +### 1.1 The Type System (Three-Layer Pattern) + +Perseus uses a three-layer type system for widget data, defined in `packages/perseus-core/src/validation.types.ts`: + +1. **WidgetOptions** (in `data-schema.ts`): The configuration/definition of the widget + - What a content creator sees in the editor + - Immutable across a session + - Example: `PerseusRadioWidgetOptions`, `PerseusNumericInputWidgetOptions` + +2. **ValidationData** (in `validation.types.ts`): The data needed for client-side validation + - Does NOT contain sensitive scoring information + - Can be checked before submission + - Used for accessibility, input validation + - Often overlaps with WidgetOptions + +3. **Rubric** (in `validation.types.ts`): The data needed for scoring + - Contains the "correct" answers and scoring logic parameters + - Always intersected with `ValidationData` via `&` operator + - Passed to scoring functions + - Example: `PerseusNumericInputRubric = { answers, coefficient } & PerseusNumericInputValidationData` + +### 1.2 Key Finding: Rubric is NOT Being Replaced + +**Status:** Rubric pattern is active and unchanged +- `PerseusNumericInputRubric` (line 223 in validation.types.ts): `{ answers, coefficient }` +- `PerseusRadioRubric` (line 261 in validation.types.ts): `{ choices, countChoices? }` +- Both are used as-is in widget implementations and scoring functions + +**No evidence of transition to:** +- `scoringData` prop +- `validationData` prop +- Alternative naming schemes + +--- + +## 2. Widget Implementation Patterns + +### 2.1 Current Widget Export Pattern + +All widgets follow the `WidgetExports` pattern (defined in `packages/perseus/src/types.ts` lines 468-512): + +```typescript +export type WidgetExports< + T extends React.ComponentType & Widget = React.ComponentType, + TUserInput = Empty, +> = Readonly<{ + name: string; // Widget identifier + displayName: string; // Display name in editor + widget: T; // The component + version?: Version; // Widget schema version + isLintable?: boolean; // Can be linted + tracking?: Tracking; // Tracking behavior + + // Scoring/Validation functions (OPTIONAL) + getOneCorrectAnswerFromRubric?: (rubric: WidgetOptions) => string | null; + getCorrectUserInput?: (widgetOptions: WidgetOptions) => TUserInput; + getStartUserInput?: (widgetOptions: WidgetOptions, problemNum: number) => TUserInput; + getUserInputFromSerializedState?: (serializedState: unknown, widgetOptions?: WidgetOptions) => TUserInput; +}>; +``` + +### 2.2 Numeric Input Widget (Modern Pattern) + +**File:** `packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx` + +```typescript +// Props type - combines widget options with universal props +type ExternalProps = WidgetProps< + PerseusNumericInputWidgetOptions, + PerseusNumericInputUserInput +>; + +// Widget component definition +export class NumericInput extends React.Component implements Widget { + // Widget interface methods + focus: () => boolean + focusInputPath: () => void + blurInputPath: () => void + getInputPaths: () => ReadonlyArray> + getPromptJSON(): NumericInputPromptJSON + // ... +} + +// Export follows WidgetExports pattern +export default { + name: "numeric-input", + displayName: "Numeric input", + widget: NumericInput, + isLintable: true, + getCorrectUserInput, // Function + getOneCorrectAnswerFromRubric(rubric: PerseusNumericInputRubric) { /* ... */ }, + getStartUserInput, // Function + getUserInputFromSerializedState, // Function +} satisfies WidgetExports; +``` + +**Actual Prop Names Used:** +- `answers: PerseusNumericInputAnswer[]` - from widget options +- `coefficient: boolean` - from widget options +- `userInput: { currentValue: string }` - user input (typed separately) +- `handleUserInput` - callback to update user input +- `trackInteraction` - analytics callback +- No `rubric`, `scoringData`, or `validationData` props passed directly to component + +### 2.3 Radio Widget (In Transition) + +**File:** `packages/perseus/src/widgets/radio/radio.ff.tsx` + `radio.ts` + +```typescript +type Props = WidgetProps; + +export default { + name: "radio", + displayName: "Radio / Multiple choice", + widget: Radio, + getStartUserInput, + version: radioLogic.version, + isLintable: true, + getUserInputFromSerializedState: (serializedState: unknown) => { + return getUserInputFromSerializedState(serializedState); + }, +} satisfies WidgetExports; +``` + +**Status:** +- Still using class component wrapper (line 52: `class Radio extends React.Component implements Widget`) +- Transitioning toward functional component (comment on line 50: "TODO(LEMS-2994): Clean up this file") +- Using old API with `ChoiceState` that combines UI state and user input + +### 2.4 Expression Widget (Functional) + +**File:** `packages/perseus/src/widgets/expression/expression.tsx` + +```typescript +type ExternalProps = WidgetProps< + PerseusExpressionWidgetOptions, + PerseusExpressionUserInput +>; + +type Props = ExternalProps & { + buttonSets: NonNullable; + functions: NonNullable; + times: NonNullable; +}; + +export class Expression extends React.Component implements Widget { + // Widget methods + focus: () => boolean + focusInputPath: () => void + // ... +} + +export default { + name: "expression", + displayName: "Expression / Equation", + widget: Expression, + version: expressionLogic.version, + isLintable: true, + getOneCorrectAnswerFromRubric, + getStartUserInput, + getCorrectUserInput, + getUserInputFromSerializedState, +} satisfies WidgetExports; +``` + +--- + +## 3. Props Flow Architecture + +### 3.1 How Props Are Passed to Widgets + +From `packages/perseus/src/renderer.tsx` (lines 511-575): + +```typescript +getWidgetProps(widgetId: string): WidgetProps { + const widgetProps = this.props.widgets[widgetId].options; // WidgetOptions + const widgetInfo = this.state.widgetInfo[widgetId]; // Metadata + + return { + ...widgetProps, // Spread widget options directly + userInput: this.props.userInput?.[widgetId], + widgetId: widgetId, + widgetIndex: this._getWidgetIndexById(widgetId), + alignment: widgetInfo && widgetInfo.alignment, + static: widgetInfo?.static, + apiOptions: this.getApiOptions(), // API options + onFocus: _.partial(this._onWidgetFocus, widgetId), + onBlur: _.partial(this._onWidgetBlur, widgetId), + handleUserInput: (newUserInput) => { /* ... */ }, + trackInteraction: interactionTracker.track, + }; +} +``` + +**Key Pattern:** +- Widget options are spread directly: `...widgetProps` +- UserInput is passed separately: `userInput` prop +- No `rubric` prop passed to widget component +- Rubric is only used during scoring (server-side or via scoring functions) + +### 3.2 UserInput vs WidgetOptions + +**UserInput Examples:** +```typescript +// Numeric Input +PerseusNumericInputUserInput = { currentValue: string } + +// Radio +PerseusRadioUserInput = { selectedChoiceIds: string[] } + +// Expression +PerseusExpressionUserInput = string + +// Label Image +PerseusLabelImageUserInput = { + markers: Array<{ selected?: string[], label: string }> +} +``` + +**WidgetOptions Examples:** +```typescript +// Numeric Input +PerseusNumericInputWidgetOptions = { + answers: PerseusNumericInputAnswer[], + labelText?: string, + size: string, + coefficient: boolean, + rightAlign?: boolean, + static: boolean, +} + +// Radio +PerseusRadioWidgetOptions = { + choices: PerseusRadioChoice[], + hasNoneOfTheAbove?: boolean, + countChoices?: boolean, + numCorrect?: number, + randomize?: boolean, + multipleSelect?: boolean, + deselectEnabled?: boolean, +} +``` + +--- + +## 4. Rubric Pattern in Scoring + +### 4.1 Rubric Types (validation.types.ts) + +Rubric types are ONLY used in: +1. Scoring functions (registered in perseus-score) +2. `getOneCorrectAnswerFromRubric` export in WidgetExports +3. Server-side validation/scoring + +**Examples:** + +```typescript +// Line 223-228 of validation.types.ts +export type PerseusNumericInputRubric = { + answers: PerseusNumericInputAnswer[]; + coefficient: boolean; +}; + +// Line 261-266 +export type PerseusRadioRubric = { + choices: PerseusRadioChoice[]; + countChoices?: boolean; +}; + +// Line 114-117 +export type PerseusExpressionRubric = { + answerForms: Array; + functions: string[]; +}; +``` + +### 4.2 Scoring Function Pattern + +From numeric-input implementation: + +```typescript +function getCorrectUserInput( + options: PerseusNumericInputWidgetOptions, +): PerseusNumericInputUserInput { + for (const answer of options.answers) { + if (answer.status === "correct" && answer.value != null) { + // Logic to find correct answer + if (answer.answerForms?.includes("decimal")) { + return {currentValue: answer.value.toString()}; + } + // ... more logic + } + } + return {currentValue: ""}; +} + +// Usage in WidgetExports: +getOneCorrectAnswerFromRubric( + rubric: PerseusNumericInputRubric, +): string | null | undefined { + const correctAnswers = rubric.answers.filter( + (answer) => answer.status === "correct" + ); + // ... logic to format for display +} +``` + +--- + +## 5. Type Relationships + +### 5.1 Data-Schema Hierarchy (PerseusWidgetTypes) + +``` +PerseusWidgetTypes (Interface in data-schema.ts) +├── categorizer: CategorizerWidget +├── dropdown: DropdownWidget +├── numeric-input: NumericInputWidget +├── radio: RadioWidget +├── expression: ExpressionWidget +└── ... (33+ widgets) + +Each entry like: +NumericInputWidget = WidgetOptions<'numeric-input', PerseusNumericInputWidgetOptions> +``` + +### 5.2 Validation Type Hierarchy (validation.types.ts) + +``` +RubricRegistry (Interface) +├── categorizer: PerseusCategorizerRubric +├── radio: PerseusRadioRubric +├── numeric-input: PerseusNumericInputRubric +└── ... (corresponds to PerseusWidgetTypes) + +UserInputRegistry (Interface) +├── categorizer: PerseusCategorizerUserInput +├── radio: PerseusRadioUserInput +└── ... +``` + +### 5.3 Type Parameter Naming in Widgets + +```typescript +// Standard pattern +type ExternalProps = WidgetProps< + PerseusNumericInputWidgetOptions, // TWidgetOptions + PerseusNumericInputUserInput // TUserInput +>; + +// WidgetProps definition (types.ts line 527-533): +export type WidgetProps< + TWidgetOptions, + TUserInput = Empty, + TrackingExtraArgs = Empty, +> = TWidgetOptions & UniversalWidgetProps; +``` + +--- + +## 6. Recent Widget Examples & Practices + +### 6.1 Numeric Input Widget - Fully Modernized ✓ + +**Status:** Class component with functional component integration (hybrid) +- Component: `NumericInputComponent` (functional, lines 28-132) +- Wrapper: `NumericInput` class (implements Widget interface) +- Props: Proper typing with `ExternalProps` and `NumericInputProps` +- Features: Focus management, accessibility (labelText, ariaLabel) + +### 6.2 Radio Widget - In Transition + +**Status:** Class wrapper around multiple-choice-widget +- Uses old `ChoiceState` API (combines UI state + user input) +- Comment: "TODO(LEMS-2994): Clean up this file" +- Issue: Hard to migrate due to component inheritance + +### 6.3 Expression Widget - Modern Pattern + +**Status:** Class component implementing Widget interface +- Proper prop typing +- Focus management via refs +- Comprehensive widget interface implementation + +--- + +## 7. Named Conventions Summary + +### 7.1 Type Naming + +| Pattern | Location | Purpose | +|---------|----------|---------| +| `PerseusWidgetOptions` | data-schema.ts | Widget configuration | +| `PerseusRubric` | validation.types.ts | Scoring data | +| `PerseusUserInput` | validation.types.ts | User's answer | +| `PerseusValidationData` | validation.types.ts | Client-side validation data | + +### 7.2 Prop Naming (Actual Usage) + +**What IS passed to widget components:** +- Widget option fields (spread from `widgetProps`) +- `userInput: TUserInput` +- `handleUserInput: (newUserInput: TUserInput) => void` +- `widgetId: string` +- `apiOptions: APIOptionsWithDefaults` +- `trackInteraction: () => void` +- `onFocus`, `onBlur`, `linterContext`, etc. + +**What IS NOT passed:** +- No `rubric` prop (used only in scoring) +- No `scoringData` prop +- No `validationData` prop +- No separate validation/scoring objects + +### 7.3 Focus Management Requirements + +Every widget must implement: +```typescript +focus?: () => { id: string; path: FocusPath } | boolean; +focusInputPath?: (path: FocusPath) => void; +blurInputPath?: (path: FocusPath) => void; +getInputPaths?: () => ReadonlyArray; +``` + +--- + +## 8. Widget Registration & Discovery + +### 8.1 Registration System (widgets.ts) + +```typescript +const widgets = new Registry("Perseus widget registry"); + +export const registerWidget = (type: string, widget: WidgetExports) => { + widgets.set(type, widget); +}; + +// All widgets register via: +// registerWidgets([numericInput, radio, expression, /* ... */]) +``` + +### 8.2 PerseusWidgetTypes Interface + +```typescript +export interface PerseusWidgetTypes { + categorizer: CategorizerWidget; + dropdown: DropdownWidget; + radio: RadioWidget; + "numeric-input": NumericInputWidget; + // ... 30+ more widgets +} +``` + +This interface: +- Is open for extension (can be module-augmented) +- Used by `MakeWidgetMap` to create `PerseusWidgetsMap` +- Provides strong typing for widget data throughout Perseus + +--- + +## 9. Current State Assessment + +### What Works Well +1. **Type Safety:** Three-layer system (WidgetOptions → ValidationData → Rubric) is robust +2. **Props Architecture:** Clear separation between widget config and user input +3. **Widget Interface:** Consistent interface for all widgets via `Widget` type +4. **Registration:** Flexible registry system supports custom widgets + +### Transition Areas +1. **Radio Widget:** Still transitioning from old `ChoiceState` API +2. **Class Components:** Some widgets still using class components (being converted to functional) +3. **Serialized State:** Deprecated API still present but marked `@deprecated` + +### No Evidence Of +1. Planned move to `scoringData` or `validationData` props +2. Changes to Rubric pattern +3. Alternative type naming schemes + +--- + +## 10. Key Findings for Implementation + +### When Creating a New Widget: + +1. **Define types in data-schema.ts:** + ```typescript + type MyNewWidget = WidgetOptions<'my-new-widget', MyNewWidgetOptions>; + // Add to PerseusWidgetTypes interface + ``` + +2. **Define scoring types in validation.types.ts:** + ```typescript + type PerseuMyNewWidgetRubric = { /* scoring data */ } & PerseuMyNewWidgetValidationData; + type PerseuMyNewWidgetUserInput = { /* user data */ }; + ``` + +3. **Create widget component with WidgetProps typing:** + ```typescript + type Props = WidgetProps; + ``` + +4. **Export via WidgetExports pattern:** + ```typescript + export default { + name: "my-new-widget", + displayName: "My New Widget", + widget: MyNewWidget, + isLintable: true, + getStartUserInput, + getCorrectUserInput, + // ... scoring functions + } satisfies WidgetExports; + ``` + +5. **Implement Widget interface methods:** + - `focus(): boolean` + - `focusInputPath(path: FocusPath): void` + - `blurInputPath(path: FocusPath): void` + - `getInputPaths(): ReadonlyArray` + - `getPromptJSON?(): WidgetPromptJSON` + +--- + +## Files Examined + +- `/packages/perseus-core/src/data-schema.ts` - Widget type definitions +- `/packages/perseus-core/src/validation.types.ts` - Scoring/validation types +- `/packages/perseus/src/types.ts` - Widget interface definitions +- `/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx` - Example widget +- `/packages/perseus/src/widgets/radio/radio.ff.tsx` - Example widget (in transition) +- `/packages/perseus/src/widgets/expression/expression.tsx` - Example widget +- `/packages/perseus/src/renderer.tsx` - Props passing mechanism +- `/packages/perseus/src/widgets.ts` - Widget registration + +--- + +**End of Research Document** diff --git a/CLAUDE.md b/CLAUDE.md index f6c5da9c7e9..132d578894b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,24 +1,42 @@ # Perseus Development Guide for AI Assistants -This document provides essential information for AI assistants working on the Perseus codebase. +This document provides essential context for AI assistants working on the Perseus codebase. Detailed documentation for specific topics is available in `.claude/prompts/`. ## Project Overview -Perseus is Khan Academy's educational content rendering system that powers all exercises and articles. It's a TypeScript -monorepo that extends Markdown with interactive widgets and beautiful math rendering. +Perseus is Khan Academy's educational content rendering system that powers all exercises and articles. It's a TypeScript monorepo that extends Markdown with interactive widgets and beautiful math rendering. -**Core Architecture:** +**Core Components:** - **Renderers**: Display content (ServerItemRenderer for exercises, ArticleRenderer for articles) - **Widgets**: Interactive components (radio, numeric-input, interactive-graph, etc.) -- **Editors**: Authoring interfaces for content creators +- **State Management**: Centralized via UserInputManager with `handleUserInput` pattern - **Math**: TeX expressions rendered via MathJax +## Prompt Library Reference + +Comprehensive documentation is organized by topic in `.claude/prompts/`: + +### Architecture & Organization +- **[codebase-map.md](.claude/prompts/codebase-map.md)** - Package structure, data flow, module boundaries +- **[file-organization.md](.claude/prompts/file-organization.md)** - Directory structure, naming conventions, imports + +### Development Guides +- **[widget-development.md](.claude/prompts/widget-development.md)** - Creating and maintaining widgets +- **[component-best-practices.md](.claude/prompts/component-best-practices.md)** - React components, Wonder Blocks, accessibility +- **[testing-best-practices.md](.claude/prompts/testing-best-practices.md)** - Testing patterns and utilities +- **[math-and-content.md](.claude/prompts/math-and-content.md)** - Math rendering, content structure, MathJax + +### Workflow & Process +- **[iteration-and-feedback.md](.claude/prompts/iteration-and-feedback.md)** - When to ask for help, debugging, deployment checklist + ## Quick Start Commands ### Development ```bash pnpm storybook # Launch Storybook documentation pnpm test # Run tests +pnpm build:types # Build TypeScript types +pnpm build # Build all packages ``` ### Code Quality @@ -30,176 +48,68 @@ pnpm prettier . --write # Auto-format code pnpm tsc # Type-check all packages ``` -### Testing Specific Packages +### Testing ```bash pnpm --filter perseus test # Test main perseus package pnpm --filter perseus-editor test # Test editor package pnpm test packages/perseus/src/widgets/radio # Test specific widget +pnpm test -u # Update snapshots +pnpm test --coverage # Run with coverage ``` -## Key Directories - -``` -packages/ -├── perseus/ # Main package (renderers, widgets, components) -│ ├── src/__docs__/ # Main Storybook stories -│ ├── src/widgets/ # Widget implementations -│ └── src/components/ # Reusable components -├── perseus-editor/ # Editor UI components -├── math-input/ # Math keypad and input components -├── perseus-core/ # Shared types and utilities -├── perseus-linter/ # Content validation tools -└── perseus-score/ # Server-side scoring functions -``` - -## Common Development Patterns - -### Creating a New Widget -1. **Create widget directory**: `packages/perseus/src/widgets/[widget-name]/` -2. **Implement widget files**: - - `[widget-name].tsx` - Main component - - `[widget-name].test.ts` - Tests - - `index.ts` - Exports - - `__docs__/[widget-name].stories.tsx` - Storybook story - - `__docs__/a11y.mdx` - Accessibility documentation -3. **Register widget** in `packages/perseus/src/widgets.ts` -4. **If scorable, add scoring functions** in `packages/perseus-score/src/widgets/[widget-name]/`: - - `score-[widget-name].ts` - Scoring logic - - `score-[widget-name].test.ts` - Scoring tests - - `validate-[widget-name].ts` - Input validation (optional) - - `validate-[widget-name].test.ts` - Validation tests (optional) -5. **Register scoring** in `packages/perseus-score/src/widgets/widget-registry.ts` -6. **Add types** to `packages/perseus-core/src/data-schema.ts` - -### Widget Implementation Pattern -```typescript -// Export interface following WidgetExports pattern -export default { - name: "widget-name", - displayName: "Widget Display Name", - widget: WidgetComponent, - isLintable: true, // For use by the editor -} as WidgetExports; -``` +## Key Architectural Decisions -### Focus Management -All widgets must implement proper focus management for accessibility. +### State Management Evolution +- **Legacy**: Widgets used `onChange` callbacks with local state +- **Current**: Transitioning to `handleUserInput` with centralized UserInputManager +- **Migration Status**: Radio widget bridging old/new patterns (LEMS-2994) +- **Example**: NumericInput fully modernized, use as reference -## Package Dependencies +### Type System +- **Three-layer architecture**: + 1. `WidgetOptions` - Configuration from content creators + 2. `ValidationData` - Client-side validation (no answers) + 3. `Rubric` - Full scoring data (`ValidationData & WidgetOptions`) +- **Important**: Rubric never passed as prop to widgets, only used in scoring -### Import Guidelines -- Use package aliases: `@khanacademy/perseus`, `@khanacademy/perseus-editor` -- NO file extensions in imports (`.ts`, `.tsx` banned by ESLint) +### Import Rules +- Use package aliases: `@khanacademy/perseus`, etc. +- NO file extensions in imports - NO cross-package relative imports - Import order: builtin > external > internal > relative > types -### Example Correct Imports -```typescript -import React from "react"; // external -import {ApiOptions} from "@khanacademy/perseus"; // internal package -import {WidgetContainer} from "../widget-container"; // relative - -import type {WidgetProps} from "@khanacademy/perseus-core"; -``` - -## Testing Guidelines - -### Test Structure -1. Follow the AAA pattern: Arrange, Act, Assert -1.1 If Arrange and Act are one action, combine them to `//Arrange, Act` -2. Use widget generators to build test data and test data options. - You can find generators for all widgets in packages/perseus-core/src/utils/generators. - An example usage can be seen here: packages/perseus/src/widgets/expression/expression.testdata.ts. -3. Follow the test structure below: -```typescript -import {render, screen} from "@testing-library/react"; -import {userEvent} from "@testing-library/user-event"; - -import {question1} from "../__testdata__/widget.testdata"; -import WidgetComponent from "../widget-component"; - -describe("WidgetComponent", () => { - it("renders correctly", () => { - render(); - expect(screen.getByRole("button")).toBeInTheDocument(); - }); - - it("handles user interaction", async () => { - const user = userEvent.setup(); - const onChange = jest.fn(); - - render(); - await user.click(screen.getByRole("button")); - - expect(onChange).toHaveBeenCalled(); - }); -}); -``` - -### Writing Tests -- use `it` for individual test cases and not `test` -- use `describe` to group related tests - -## Common Issues & Solutions - -### Math Rendering (TeX) -- Use `$...$` for inline math, `$$...$$` for display math -- For complex expressions, use `\dfrac` instead of `\frac` -- Test math rendering in different contexts (articles, exercises, hints) - -### Widget State Management -- Use `useState` for local component state -- Props flow down from parent renderer -- Call `onChange` to notify parent of state changes -- Implement proper serialization for persistent state - -### Mobile Considerations -- All widgets must work on mobile devices -- Support touch interactions -- Consider on-screen keypad for math inputs -- Test with different screen sizes using Storybook - -### Performance Optimization -- Use `React.memo()` for expensive components -- Implement `useMemo()` for complex calculations -- Avoid unnecessary re-renders in widget hierarchies -- Profile performance with React DevTools - -## Debugging Tips - -### Storybook Development -- Use Storybook for isolated component development -- Test different props combinations -- Verify accessibility with Storybook a11y addon -- Check mobile layouts with device frame addon - -### Console Debugging -```typescript -// Temporary debugging (remove before commit) -console.log("Widget state:", this.state); -console.log("Props received:", this.props); -``` - -### Error Boundaries -- Widgets are wrapped in error boundaries -- Check browser console for widget-specific errors -- Implement graceful fallbacks for failed widgets - -## Deployment Notes - -### Before Submitting PR -1. Run full test suite: `pnpm test` -2. Check types: `pnpm build:types` -3. Lint and format: `pnpm lint --fix && pnpm prettier . --write` -4. Test in Storybook: `pnpm storybook` -5. Verify accessibility compliance - -### Common Pre-commit Failures -- ESLint errors (unused imports, console statements) -- Prettier formatting (spacing, quotes, semicolons) -- TypeScript type errors -- Missing accessibility documentation -- Test failures +## Before Submitting Code + +### Pre-commit Checklist +1. `pnpm test` - All tests pass +2. `pnpm tsc` - No type errors +3. `pnpm lint --fix` - No linting issues +4. `pnpm prettier . --write` - Code formatted +5. `pnpm build` - Build succeeds +6. `pnpm storybook` - Components render correctly +7. Accessibility documented (for new widgets) +8. Mobile tested (if applicable) +9. Console clear of debug statements + +### Common Issues to Check +- Console statements left in code +- Unused imports +- Missing displayName on components +- onChange used instead of handleUserInput +- Local state for user answers (should use UserInputManager) +- Unescaped backslashes in TeX strings + +## Migration & Deprecation + +### Deprecated Patterns +- `onChange` prop - Use `handleUserInput` instead (LEMS-3245, LEMS-3542) +- Local widget state for answers - Use UserInputManager +- Class components for new widgets - Use functional components + +### In Transition +- Radio widget - Bridging old/new patterns +- Some widgets still using legacy patterns +- Gradual migration to centralized state ## Additional Resources @@ -208,7 +118,9 @@ console.log("Props received:", this.props); - **Widget Gallery**: Browse existing widgets in Storybook for patterns - **Accessibility Guidelines**: Each widget should have `a11y.mdx` documentation - **Khan Academy Design System**: Wonder Blocks components for consistent UI +- **Test Generators**: `@khanacademy/perseus-core/utils/generators` +- **Type Definitions**: `packages/perseus-core/src/data-schema.ts` --- -*This document is maintained for AI assistants. For human developers, see the main README.md files in each package.* +*This document provides an overview and quick reference. For detailed information on any topic, refer to the appropriate document in `.claude/prompts/`. For human developers, see the main README.md files in each package.* \ No newline at end of file From ab8a5339c5f5196aa27a144ff1dc33bd9c520e7c Mon Sep 17 00:00:00 2001 From: Tamara Date: Fri, 6 Feb 2026 17:01:38 -0500 Subject: [PATCH 02/12] [tb/claude-progressive-disclosure] docs(changeset): --- .changeset/wicked-eggs-cheer.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changeset/wicked-eggs-cheer.md diff --git a/.changeset/wicked-eggs-cheer.md b/.changeset/wicked-eggs-cheer.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/wicked-eggs-cheer.md @@ -0,0 +1,2 @@ +--- +--- From 13275c874eb45e2d8db8790b12803845fa0ffade Mon Sep 17 00:00:00 2001 From: Tamara Date: Tue, 10 Feb 2026 14:23:22 -0500 Subject: [PATCH 03/12] [tb/LEMS-3710/toggle-for-shuffle] Claude plans and research --- .claude/implementation-plan-LEMS-3710.md | 101 ++++++++++ .claude/research-LEMS-3710.md | 134 +++++++++++++ ...earch-editor-preview-dataflow-LEMS-3710.md | 181 ++++++++++++++++++ .claude/ticket-summary-LEMS-3710.md | 25 +++ 4 files changed, 441 insertions(+) create mode 100644 .claude/implementation-plan-LEMS-3710.md create mode 100644 .claude/research-LEMS-3710.md create mode 100644 .claude/research-editor-preview-dataflow-LEMS-3710.md create mode 100644 .claude/ticket-summary-LEMS-3710.md diff --git a/.claude/implementation-plan-LEMS-3710.md b/.claude/implementation-plan-LEMS-3710.md new file mode 100644 index 00000000000..46ab6385e74 --- /dev/null +++ b/.claude/implementation-plan-LEMS-3710.md @@ -0,0 +1,101 @@ +# LEMS-3710: Add Toggle for Showing Preview Unshuffled + +## Context + +Content creators using the Radio widget find the Edit tab sidebar preview unhelpful when choices are shuffled — it's hard to match shuffled preview choices back to the editor fields. They want the sidebar preview to default to unshuffled, with an option to see the shuffled view when needed. The Preview tab should always respect the `randomize` setting. + +## Approach + +### Core Idea + +Add an ephemeral "Shuffle preview" toggle to `RadioEditor` that controls whether the Edit tab sidebar preview shuffles choices. The toggle state is communicated to the preview via a marker field in `serialize()` output, which `EditorPage.updateRenderer()` processes before sending data to the iframe. + +### Why This Approach + +- `serialize()` is the only channel between `RadioEditor` and the preview iframe +- `serialize()` is also used for saving — so we use a marker field that gets stripped on the save path +- No changes to core shuffle logic (`choiceTransform`, `radio.ff.tsx`, etc.) +- Fully editor-only change + +## Implementation Steps + +### Step 1: Add toggle UI and state to RadioEditor + +**File:** `packages/perseus-editor/src/widgets/radio/editor.tsx` + +- Add component state: `state = { showShuffledPreview: false }` +- Add a "Shuffle preview" `LabeledSwitch` immediately after the "Randomize order" switch + - Disabled when `!this.props.randomize` or `isEditingDisabled` + - Checked: `this.state.showShuffledPreview` + - When toggled off or when randomize is turned off, reset to `false` +- Add `InfoTip` next to "Randomize order" explaining: "The editor preview shows choices unshuffled by default. Use 'Shuffle preview' to see the randomized order. The Preview tab always shows the randomized order when enabled." +- Follow the `LabeledSwitch + InfoTip` pattern from `decorative-toggle.tsx` (flex row with gap) + +### Step 2: Add marker field to serialize() + +**File:** `packages/perseus-editor/src/widgets/radio/editor.tsx` + +- In `serialize()`, when `this.props.randomize && this.state.showShuffledPreview`, include `_showShuffledPreview: true` in the return value +- The real `randomize` field always reflects the true prop value (unchanged) +- Widen the return type to allow the extra field + +### Step 3: Process marker in EditorPage.updateRenderer() + +**File:** `packages/perseus-editor/src/editor-page.tsx` + +- After `this.serialize()` on line 225, post-process the item: + - Walk `item.question.widgets` entries + - For any widget where `type === "radio"`: + - If `options._showShuffledPreview` is truthy → leave `randomize` as-is, delete the marker + - Otherwise → set `options.randomize = false` (default: unshuffled in editor preview) +- This ensures the Edit tab preview defaults to unshuffled unless the creator explicitly enables shuffle preview + +### Step 4: Strip marker from save path + +**File:** `packages/perseus-editor/src/editor-page.tsx` + +- In `serialize()` (line 254), after serializing, walk widgets and remove `_showShuffledPreview` from any radio widget options +- This prevents the marker from ever being persisted to content data + +### Step 5: Tests + +**Files:** + +- `packages/perseus-editor/src/widgets/__tests__/radio-editor.test.tsx` — Toggle UI behavior +- `packages/perseus-editor/src/__tests__/editor-page.test.tsx` (or similar) — Preview/save data processing + +Test cases: + +- Toggle renders and is disabled when randomize is off +- Toggle is enabled when randomize is on +- Toggle resets to false when randomize is turned off +- `serialize()` includes `_showShuffledPreview: true` only when both randomize and toggle are on +- `serialize()` does NOT include marker when toggle is off +- `updateRenderer()` sets `randomize: false` for radio widgets without the marker +- `updateRenderer()` leaves `randomize: true` for radio widgets with the marker +- Save path strips the marker field + +## Key Files + +| File | Change | +| ----------------------------------------------------------------------------------- | ---------------------------------------------------- | +| `packages/perseus-editor/src/widgets/radio/editor.tsx` | Add toggle, state, InfoTip, serialize marker | +| `packages/perseus-editor/src/editor-page.tsx` | Process marker in updateRenderer, strip in serialize | +| `packages/perseus-editor/src/widgets/image-editor/components/decorative-toggle.tsx` | Reference only (LabeledSwitch + InfoTip pattern) | + +## Verification + +1. `pnpm --filter perseus-editor test` — editor unit tests pass +2. `pnpm test` — all tests across the monorepo pass (no regressions) +3. `pnpm tsc` — no type errors +4. `pnpm lint` — no linting issues (run `pnpm lint --fix` if any arise) +5. `pnpm storybook` — manual testing in the Radio widget editor: + - Toggle Randomize on → sidebar preview should show unshuffled (default) + - Toggle "Shuffle preview" on → sidebar preview should show shuffled + - Toggle "Shuffle preview" off → sidebar preview returns to unshuffled + - Toggle Randomize off → "Shuffle preview" should be disabled, preview unshuffled + - Switch to Preview tab → choices should always be shuffled when Randomize is on + +--- + +*Co-authored by Claude Opus 4.6* \ No newline at end of file diff --git a/.claude/research-LEMS-3710.md b/.claude/research-LEMS-3710.md new file mode 100644 index 00000000000..bc045454e83 --- /dev/null +++ b/.claude/research-LEMS-3710.md @@ -0,0 +1,134 @@ +# LEMS-3710 Research: Radio Widget Shuffle Toggle + +## Overview + +This document captures research into how the Radio widget handles shuffling, how the Edit and Preview tabs differ architecturally, and what tooltip patterns exist in the editor. + +## Shuffle Mechanics + +### Core Shuffle Logic +- **`choiceTransform()`** in `packages/perseus/src/widgets/radio/util.ts` (lines 116-146) is the central orchestration function +- Applies three transforms in order: + 1. **Shuffle** (lines 140-141): If `randomize` is truthy, calls `shuffle(choicesWithMetadata, seed)` + 2. **Enforce ordering** (lines 96-113): Swaps True/False or Yes/No into canonical order + 3. **Move "None of the above" to end** (lines 74-94): Ensures `isNoneOfTheAbove` choice is always last + +### Shuffle Utility +- `packages/perseus-core/src/utils/random-util.ts` (lines 34-51) +- Uses seeded RNG (Robert Jenkins' 32-bit integer hash) with Fisher-Yates algorithm +- Seed is computed as `problemNum + widgetIndex` in `radio.ff.tsx` (lines 182-183) + +### Where Shuffling is Triggered +- **`Radio._mergePropsAndState()`** in `packages/perseus/src/widgets/radio/radio.ff.tsx` (lines 164-215) +- Calls `choiceTransform()` with the original choices, `randomize` flag, localized strings, and computed seed +- The shuffled `RadioChoiceWithMetadata[]` is passed down to `MultipleChoiceWidget` + +### Choice IDs +- Choice IDs are stable/opaque identifiers that survive shuffling +- No "unshuffling by index" is needed — IDs carry the mapping +- `getUserInputFromSerializedState()` in `util.ts` (lines 52-72) extracts `selectedChoiceIds` from choice states + +### Default Value +- `randomize` defaults to `false` in `packages/perseus-core/src/widgets/radio/index.ts` (line 25) + +## Edit Tab vs Preview Tab Architecture + +### Edit Tab (Sidebar Preview) +- **iframe-based rendering** +- `packages/perseus-editor/src/item-editor.tsx` (lines 236-250): Renders `IframeContentRenderer` inside `DeviceFramer` +- `packages/perseus-editor/src/editor-page.tsx` (lines 204-238): `updateRenderer()` serializes editor state and sends to iframe +- Always sends `reviewMode: true` (line 235) +- `problemNum` passed through from props (line 237) + +### Preview Tab (ContentPreview) +- **Inline rendering** (no iframe) +- `packages/perseus-editor/src/content-preview.tsx` (lines 33-116) +- `problemNum` is hardcoded to `0` (line 81) +- `reviewMode` passed through as a prop +- Uses `UserInputManager` for state management + +### Key Differences + +| Signal | Edit Tab (iframe) | Preview Tab (ContentPreview) | +|--------|-------------------|------------------------------| +| `reviewMode` | Always `true` | Passed through | +| `problemNum` | From parent props | Hardcoded to `0` | +| Rendering | `IframeContentRenderer` + `ServerItemRenderer` | Inline `Renderer` + `UserInputManager` | + +### Widget-Level Distinction +- The Radio widget receives `reviewMode`, `static`, `showSolutions`, `apiOptions.readOnly` +- There is **no `editMode` prop** on the widget — the Edit vs Preview distinction comes from `reviewMode` being set differently +- In `multiple-choice-widget.tsx` (line 362): `const isReviewMode = reviewMode || isStatic || showSolutions === "all"` + +## Data Flow: Editor to Widget + +``` +RadioEditor (author choices, unshuffled) + | + v serialize() +PerseusRadioWidgetOptions { choices, randomize, ... } + | + v stored in PerseusRenderer JSON +Renderer.getWidgetProps() adds problemNum, widgetIndex, reviewMode + | + v +Radio (radio.ff.tsx) _mergePropsAndState() -> choiceTransform() -> shuffle + | + v shuffled RadioChoiceWithMetadata[] +MultipleChoiceWidget (multiple-choice-widget.tsx) + | + v buildChoiceProps() -> ChoiceType[] +MultipleChoiceComponent (renders the UI) +``` + +## Randomize Toggle in Editor + +### Current Implementation +- **File:** `packages/perseus-editor/src/widgets/radio/editor.tsx` (lines 335-343) +- Uses `LabeledSwitch` component with label "Randomize order" +- No tooltip currently attached +- Import: `import LabeledSwitch from "../../components/labeled-switch";` + +### LabeledSwitch Component +- **File:** `packages/perseus-editor/src/components/labeled-switch.tsx` +- Wraps `Switch` from `@khanacademy/wonder-blocks-switch` +- Uses `LabelSmall` or `LabelMedium` from `@khanacademy/wonder-blocks-typography` + +## Existing Tooltip Patterns in Editor + +### Pattern 1: LabeledSwitch + InfoTip (Best Precedent) +- **File:** `packages/perseus-editor/src/widgets/image-editor/components/decorative-toggle.tsx` (lines 51-65) +- `LabeledSwitch` and `InfoTip` as siblings in a flex row +- CSS: `display: flex; align-items: center; gap: var(--wb-sizing-size_040);` + +### Pattern 2: Checkbox with InfoTip in Label +- **File:** `packages/perseus-editor/src/item-extras-editor.tsx` (lines 168-188) +- `InfoTip` nested inside `Checkbox` label prop + +### Pattern 3: Standalone InfoTip +- **File:** `packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figure-aria.tsx` (lines 39-51) +- `InfoTip` with multiline description next to a label + +### InfoTip Component +- **Wrapper:** `packages/perseus/src/components/info-tip/index.tsx` +- **Core:** `packages/perseus/src/components/info-tip/info-tip-base.tsx` (lines 13-26) +- Uses `Tooltip` from `@khanacademy/wonder-blocks-tooltip` + `PhosphorIcon` question mark icon +- Import pattern: `const {InfoTip} = components;` (from `@khanacademy/perseus`) + +## Key Files for Implementation + +| Purpose | File | +|---------|------| +| Radio editor UI | `packages/perseus-editor/src/widgets/radio/editor.tsx` | +| Edit tab preview data | `packages/perseus-editor/src/editor-page.tsx` | +| Preview tab rendering | `packages/perseus-editor/src/content-preview.tsx` | +| Item editor (tab container) | `packages/perseus-editor/src/item-editor.tsx` | +| Shuffle logic | `packages/perseus/src/widgets/radio/util.ts` | +| Radio widget wrapper | `packages/perseus/src/widgets/radio/radio.ff.tsx` | +| InfoTip component | `packages/perseus/src/components/info-tip/info-tip-base.tsx` | +| LabeledSwitch component | `packages/perseus-editor/src/components/labeled-switch.tsx` | +| Decorative toggle (tooltip precedent) | `packages/perseus-editor/src/widgets/image-editor/components/decorative-toggle.tsx` | + +--- + +*Co-authored by Claude Opus 4.6* \ No newline at end of file diff --git a/.claude/research-editor-preview-dataflow-LEMS-3710.md b/.claude/research-editor-preview-dataflow-LEMS-3710.md new file mode 100644 index 00000000000..076bca0f205 --- /dev/null +++ b/.claude/research-editor-preview-dataflow-LEMS-3710.md @@ -0,0 +1,181 @@ +# Editor Preview Data Flow Deep Dive — LEMS-3710 + +## Overview + +This document traces the complete data flow from when a content creator changes a setting in the Radio widget editor to when the Edit tab sidebar preview re-renders. Understanding this flow is critical for knowing where to intercept the `randomize` flag. + +## The Full onChange-to-Preview Flow + +When the user toggles "Randomize order" (or any setting), here's the complete chain: + +### 1. User clicks the Switch +**File:** `packages/perseus-editor/src/widgets/radio/editor.tsx` (lines 339-341) +```typescript +onChange={(value) => { + this.props.onChange({randomize: value}); +}} +``` + +### 2. WidgetEditor merges the change +**File:** `packages/perseus-editor/src/widget-editor.tsx` (lines 85-103) +```typescript +_handleWidgetChange = (newProps, cb, silent) => { + const newWidgetInfo = { + ...this.state.widgetInfo, + options: { + ...this.state.widgetInfo.options, + ...(this.widget.current?.serialize() ?? {}), + ...newProps, + }, + }; + this.props.onChange(newWidgetInfo, cb, silent); +}; +``` +Merges `{randomize: value}` into `widgetInfo.options`, creating a full `PerseusWidget` object. + +### 3. Editor updates widgets +**File:** `packages/perseus-editor/src/editor.tsx` (lines 271-285) + +Copies all widgets, updates the changed widget by ID: +```typescript +this.props.onChange({widgets}, cb, silent); +``` + +### 4. ItemEditor merges into question +**File:** `packages/perseus-editor/src/item-editor.tsx` (lines 156-159) +```typescript +const question = _.extend({}, this.props.question, newProps); +this.updateProps({question}, cb, silent); +``` + +### 5. ItemEditor.updateProps propagates up +**File:** `packages/perseus-editor/src/item-editor.tsx` (lines 145-149) + +Merges into `{question, answerArea}`, calls `this.props.onChange(...)`. + +### 6. EditorPage.handleChange +**File:** `packages/perseus-editor/src/editor-page.tsx` (lines 264-268) + +Calls `this.props.onChange(newProps, cb, silent)` to the host application. + +### 7. Host re-renders EditorPage → componentDidUpdate fires +**File:** `packages/perseus-editor/src/editor-page.tsx` (lines 139-149) +```typescript +componentDidUpdate(previousProps, prevState, snapshot) { + setTimeout(() => { + this.updateRenderer(); + }); +} +``` +Every prop change triggers `updateRenderer()` via `setTimeout`. + +### 8. updateRenderer() serializes and sends to iframe +**File:** `packages/perseus-editor/src/editor-page.tsx` (lines 204-239) +```typescript +updateRenderer() { + // ... + this.itemEditor.current?.triggerPreviewUpdate({ + type: "question", + data: _({ + item: this.serialize(), // <-- serializes entire item + apiOptions: deviceBasedApiOptions, + initialHintsVisible: 0, + device: this.props.previewDevice, + linterContext: { ... }, + reviewMode: true, // <-- always true for Edit tab + legacyPerseusLint: ..., + }).extend(_(this.props).pick("problemNum")), + }); +} +``` + +### 9. Serialization chain +`this.serialize()` walks the tree: +- `EditorPage.serialize()` (line 254) → `ItemEditor.serialize()` → `Editor.serialize()` → iterates widget refs → `RadioEditor.serialize()` +- `RadioEditor.serialize()` returns `{choices, randomize, multipleSelect, ...}` +- The `randomize` value flows through unchanged into `item.question.widgets["radio 1"].options.randomize` + +### 10. Data sent to iframe +**File:** `packages/perseus-editor/src/item-editor.tsx` (lines 151-153) +```typescript +triggerPreviewUpdate: (newData?: any) => void = (newData: any) => { + this.frame.current?.sendNewData(newData); +}; +``` + +### 11. IframeContentRenderer posts to iframe +**File:** `packages/perseus-editor/src/iframe-content-renderer.tsx` (lines 179-190) +```typescript +sendNewData(data: any) { + if (this._isMounted && data && frame?.contentWindow) { + this._lastData = data; + window.iframeDataStore[this.iframeID] = data; + frame.contentWindow.postMessage(this.iframeID, "*"); + } +} +``` +Data stored in `window.iframeDataStore[id]`, iframe notified via `postMessage`. + +### 12. Iframe renders the preview +The iframe reads `window.parent.iframeDataStore[id]` and renders the item with the received data, including the `randomize` value. + +## Data Structure Sent to Iframe + +```typescript +{ + type: "question", + data: { + item: { + question: { + content: "...", + widgets: { + "radio 1": { + type: "radio", + options: { + choices: [...], + randomize: boolean, // <-- THIS IS THE KEY FIELD + multipleSelect: boolean, + // ... + } + } + } + }, + answerArea: {...}, + hints: [...] + }, + apiOptions: {...}, + reviewMode: true, + problemNum: number, + // ... + } +} +``` + +## Key Insight: serialize() Dual Use + +`RadioEditor.serialize()` is called in TWO contexts: +1. **Preview updates** — via `updateRenderer()` → `this.serialize()` chain +2. **Saving content** — via `EditorPage.serialize()` when the host app saves + +Both paths call the same `serialize()` method, so any modification to `serialize()` output affects both preview and saving. This is why a marker field approach (include a temporary field, strip it on save) is needed to control preview behavior without affecting saved data. + +## Edit Tab vs Preview Tab: Key Differences + +| Aspect | Edit Tab (sidebar) | Preview Tab | +|--------|-------------------|-------------| +| Rendering | iframe via `IframeContentRenderer` | Inline via `ContentPreview` → `Renderer` | +| `reviewMode` | Always `true` | Passed through | +| `problemNum` | From parent props | Hardcoded to `0` | +| Data source | `EditorPage.updateRenderer()` | Widget options from props directly | +| Shuffle control | Through iframe data pipeline | Through widget options directly | + +## Interception Point + +The best place to modify `randomize` for the preview is in `EditorPage.updateRenderer()`, after `this.serialize()` returns but before the data is sent to `triggerPreviewUpdate()`. This lets us: +- Leave `RadioEditor.serialize()` mostly unchanged (just add a marker) +- Override `randomize` only for the Edit tab preview +- Keep the Preview tab unaffected (it doesn't go through `updateRenderer()`) + +--- + +*Co-authored by Claude Opus 4.6* \ No newline at end of file diff --git a/.claude/ticket-summary-LEMS-3710.md b/.claude/ticket-summary-LEMS-3710.md new file mode 100644 index 00000000000..9dbfb651fe9 --- /dev/null +++ b/.claude/ticket-summary-LEMS-3710.md @@ -0,0 +1,25 @@ +# LEMS-3710: [Radio] | (CX) | Add toggle for showing preview unshuffled + +**Status:** To Do +**Type:** Software Task +**Assignee:** Tamara Bozich + +## Summary + +Content creators using the Radio Widget want the ability to preview choice options **unshuffled** in the editor sidebar, even when the widget is configured for randomization. This makes it easier to review content in a predictable order while authoring. + +## Acceptance Criteria + +1. **Edit tab preview**: Always shows choices **unshuffled** (regardless of randomize setting) +2. **Preview tab**: Shows choices **shuffled** only when the widget is configured to randomize +3. **Info tooltip**: Add a tooltip next to the "Randomize" toggle explaining that only the "Preview" tab shows shuffled choices + +## Key Context + +- The request came from content creators who find shuffled previews unhelpful while editing +- The Edit tab sidebar preview currently shuffles choices when randomize is on, making it harder to map choices to the editor fields +- The Preview tab is where the learner experience is simulated, so shuffling belongs there + +--- + +*Co-authored by Claude Opus 4.6* \ No newline at end of file From 6cdb43e3536b61fc78d301d2a1b9853193afb6d2 Mon Sep 17 00:00:00 2001 From: Tamara Date: Wed, 11 Feb 2026 13:13:40 -0500 Subject: [PATCH 04/12] [tb/LEMS-3710/toggle-for-shuffle] Add "Shuffle preview" toggle to Radio editor, first try The Edit tab sidebar preview now defaults to showing choices unshuffled, even when Randomize is enabled. A new "Shuffle preview" toggle lets content creators opt into seeing the shuffled order in the sidebar. The Preview tab continues to always respect the Randomize setting. --- packages/perseus-editor/src/editor-page.tsx | 46 +++++- .../widgets/__tests__/radio-editor.test.tsx | 139 ++++++++++++++++++ .../src/widgets/radio/editor.tsx | 71 +++++++-- 3 files changed, 245 insertions(+), 11 deletions(-) diff --git a/packages/perseus-editor/src/editor-page.tsx b/packages/perseus-editor/src/editor-page.tsx index 79ef6523b3e..dcfe3603413 100644 --- a/packages/perseus-editor/src/editor-page.tsx +++ b/packages/perseus-editor/src/editor-page.tsx @@ -222,7 +222,7 @@ class EditorPage extends React.Component { this.itemEditor.current?.triggerPreviewUpdate({ type: "question", data: _({ - item: this.serialize(), + item: this.serializeForPreview(), apiOptions: deviceBasedApiOptions, initialHintsVisible: 0, device: this.props.previewDevice, @@ -255,9 +255,51 @@ class EditorPage extends React.Component { if (this.props.jsonMode) { return this.state.json; } - return _.extend(this.itemEditor.current?.serialize(options), { + const result = _.extend(this.itemEditor.current?.serialize(options), { hints: this.hintsEditor.current?.serialize(options), }); + + // Strip editor-only marker fields from radio widget options + // so they are never persisted to content data. + const widgets = result?.question?.widgets; + if (widgets) { + for (const widgetId of Object.keys(widgets)) { + const widget = widgets[widgetId]; + if (widget?.type === "radio" && widget.options) { + delete widget.options._showShuffledPreview; + } + } + } + + return result; + } + + // Serializes item data for the Edit tab iframe preview. + // Unlike serialize() (the save path), this preserves the + // _showShuffledPreview marker to control preview shuffling, + // then applies the preview-specific shuffle override. + serializeForPreview(): PerseusItem { + const item = _.extend(this.itemEditor.current?.serialize(), { + hints: this.hintsEditor.current?.serialize(), + }); + + // For the Edit tab preview, default radio widgets to unshuffled + // unless the editor's "Shuffle preview" toggle is on. + const widgets = item?.question?.widgets; + if (widgets) { + for (const widgetId of Object.keys(widgets)) { + const widget = widgets[widgetId]; + if (widget?.type === "radio" && widget.options) { + if (widget.options._showShuffledPreview) { + delete widget.options._showShuffledPreview; + } else { + widget.options.randomize = false; + } + } + } + } + + return item; } // eslint-disable-next-line import/no-deprecated diff --git a/packages/perseus-editor/src/widgets/__tests__/radio-editor.test.tsx b/packages/perseus-editor/src/widgets/__tests__/radio-editor.test.tsx index dd114ad5a5c..2a6f73f8b81 100644 --- a/packages/perseus-editor/src/widgets/__tests__/radio-editor.test.tsx +++ b/packages/perseus-editor/src/widgets/__tests__/radio-editor.test.tsx @@ -1363,6 +1363,145 @@ describe("radio-editor", () => { }); }); + describe("Shuffle preview toggle", () => { + it("renders the shuffle preview toggle", () => { + renderRadioEditor(); + + expect( + screen.getByRole("switch", {name: "Shuffle preview"}), + ).toBeInTheDocument(); + }); + + it("is disabled when randomize is off", () => { + renderRadioEditor(() => {}, {randomize: false}); + + const toggle = screen.getByRole("switch", { + name: "Shuffle preview", + }); + expect(toggle).toHaveAttribute("aria-disabled", "true"); + }); + + it("is enabled when randomize is on", () => { + renderRadioEditor(() => {}, {randomize: true}); + + const toggle = screen.getByRole("switch", { + name: "Shuffle preview", + }); + expect(toggle).not.toHaveAttribute("aria-disabled", "true"); + }); + + it("does not include _showShuffledPreview in serialize when toggle is off", () => { + const editorRef = React.createRef(); + + render( + {}} + apiOptions={ApiOptions.defaults} + static={false} + choices={fourChoices} + randomize={true} + />, + {wrapper: RenderStateRoot}, + ); + + const options = editorRef.current?.serialize(); + expect(options?._showShuffledPreview).toBeUndefined(); + }); + + it("includes _showShuffledPreview in serialize when prop is true and randomize is on", () => { + const editorRef = React.createRef(); + + render( + {}} + apiOptions={ApiOptions.defaults} + static={false} + choices={fourChoices} + randomize={true} + _showShuffledPreview={true} + />, + {wrapper: RenderStateRoot}, + ); + + const options = editorRef.current?.serialize(); + expect(options?._showShuffledPreview).toBe(true); + }); + + it("does not include _showShuffledPreview when randomize is off even if prop is true", () => { + const editorRef = React.createRef(); + + render( + {}} + apiOptions={ApiOptions.defaults} + static={false} + choices={fourChoices} + randomize={false} + _showShuffledPreview={true} + />, + {wrapper: RenderStateRoot}, + ); + + const options = editorRef.current?.serialize(); + expect(options?._showShuffledPreview).toBeUndefined(); + expect(options?.randomize).toBe(false); + }); + + it("calls onChange with _showShuffledPreview when toggled", async () => { + const onChangeMock = jest.fn(); + + renderRadioEditor(onChangeMock, {randomize: true}); + + await userEvent.click( + screen.getByRole("switch", {name: "Shuffle preview"}), + ); + + expect(onChangeMock).toHaveBeenCalledWith({ + _showShuffledPreview: true, + }); + }); + + it("resets _showShuffledPreview when randomize is turned off", async () => { + const onChangeMock = jest.fn(); + + renderRadioEditor(onChangeMock, { + randomize: true, + _showShuffledPreview: true, + }); + + await userEvent.click( + screen.getByRole("switch", {name: "Randomize order"}), + ); + + expect(onChangeMock).toHaveBeenCalledWith({ + randomize: false, + _showShuffledPreview: false, + }); + }); + + it("preserves the real randomize value in serialize", () => { + const editorRef = React.createRef(); + + render( + {}} + apiOptions={ApiOptions.defaults} + static={false} + choices={fourChoices} + randomize={true} + />, + {wrapper: RenderStateRoot}, + ); + + const options = editorRef.current?.serialize(); + expect(options?.randomize).toBe(true); + }); + }); + describe("ensureValidIds", () => { it("should generate new ID for empty string", () => { // Reset mock and set specific return value diff --git a/packages/perseus-editor/src/widgets/radio/editor.tsx b/packages/perseus-editor/src/widgets/radio/editor.tsx index 6d8e234abc9..6241bfadb52 100644 --- a/packages/perseus-editor/src/widgets/radio/editor.tsx +++ b/packages/perseus-editor/src/widgets/radio/editor.tsx @@ -1,8 +1,9 @@ /* eslint-disable jsx-a11y/anchor-is-valid */ +import {components} from "@khanacademy/perseus"; import {radioLogic, deriveNumCorrect} from "@khanacademy/perseus-core"; import Button from "@khanacademy/wonder-blocks-button"; import Link from "@khanacademy/wonder-blocks-link"; -import {sizing} from "@khanacademy/wonder-blocks-tokens"; +import {semanticColor, sizing} from "@khanacademy/wonder-blocks-tokens"; import {Footnote} from "@khanacademy/wonder-blocks-typography"; import plusIcon from "@phosphor-icons/core/bold/plus-bold.svg"; import * as React from "react"; @@ -10,6 +11,8 @@ import _ from "underscore"; import LabeledSwitch from "../../components/labeled-switch"; +const {InfoTip} = components; + import {RadioOptionSettings} from "./radio-option-settings"; import {getMovedChoices} from "./utils"; @@ -22,6 +25,10 @@ import type { RadioDefaultWidgetOptions, } from "@khanacademy/perseus-core"; +type RadioSerializedOptions = PerseusRadioWidgetOptions & { + _showShuffledPreview?: boolean; +}; + // Exported for testing export interface RadioEditorProps { apiOptions: APIOptions; @@ -32,9 +39,10 @@ export interface RadioEditorProps { multipleSelect: boolean; deselectEnabled: boolean; static: boolean; + _showShuffledPreview?: boolean; onChange: ( - values: Partial, + values: Partial, callback?: (() => void) | null, ) => void; } @@ -298,7 +306,7 @@ class RadioEditor extends React.Component { return []; }; - serialize(): PerseusRadioWidgetOptions { + serialize(): RadioSerializedOptions { const { choices, randomize, @@ -306,9 +314,10 @@ class RadioEditor extends React.Component { countChoices, hasNoneOfTheAbove, deselectEnabled, + _showShuffledPreview, } = this.props; - return { + const options: RadioSerializedOptions = { choices, randomize, multipleSelect, @@ -317,6 +326,12 @@ class RadioEditor extends React.Component { deselectEnabled, numCorrect: deriveNumCorrect(choices), }; + + if (randomize && _showShuffledPreview) { + options._showShuffledPreview = true; + } + + return options; } render(): React.ReactNode { @@ -332,14 +347,52 @@ class RadioEditor extends React.Component { Multiple choice best practices
+
+ { + this.props.onChange({ + randomize: value, + ...(!value && { + _showShuffledPreview: false, + }), + }); + }} + /> + +

+ The editor preview shows choices unshuffled by + default. Use "Shuffle preview" to see + the randomized order. The Preview tab always + shows the randomized order when enabled. +

+ +
{ - this.props.onChange({randomize: value}); + this.props.onChange({ + _showShuffledPreview: value, + }); + }} + style={{ + marginBlockEnd: sizing.size_060, + ...(!this.props.randomize && { + color: semanticColor.core.foreground.disabled + .default, + }), }} - style={{marginBlockEnd: sizing.size_060}} /> Date: Wed, 11 Feb 2026 15:13:12 -0500 Subject: [PATCH 05/12] [tb/LEMS-3710/toggle-for-shuffle] Remove p tag to fix spacing --- packages/perseus-editor/src/widgets/radio/editor.tsx | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/perseus-editor/src/widgets/radio/editor.tsx b/packages/perseus-editor/src/widgets/radio/editor.tsx index 6241bfadb52..54e5a7ef8ed 100644 --- a/packages/perseus-editor/src/widgets/radio/editor.tsx +++ b/packages/perseus-editor/src/widgets/radio/editor.tsx @@ -369,12 +369,10 @@ class RadioEditor extends React.Component { }} /> -

- The editor preview shows choices unshuffled by - default. Use "Shuffle preview" to see - the randomized order. The Preview tab always - shows the randomized order when enabled. -

+ The editor preview shows choices unshuffled by + default. Use "Shuffle preview" to see + the randomized order. The Preview tab always + shows the randomized order when enabled.
Date: Wed, 11 Feb 2026 15:17:52 -0500 Subject: [PATCH 06/12] [tb/LEMS-3710/toggle-for-shuffle] Lint fix --- packages/perseus-editor/src/widgets/radio/editor.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/perseus-editor/src/widgets/radio/editor.tsx b/packages/perseus-editor/src/widgets/radio/editor.tsx index 54e5a7ef8ed..a8ca149ddfc 100644 --- a/packages/perseus-editor/src/widgets/radio/editor.tsx +++ b/packages/perseus-editor/src/widgets/radio/editor.tsx @@ -370,9 +370,9 @@ class RadioEditor extends React.Component { /> The editor preview shows choices unshuffled by - default. Use "Shuffle preview" to see - the randomized order. The Preview tab always - shows the randomized order when enabled. + default. Use "Shuffle preview" to see the + randomized order. The Preview tab always shows the + randomized order when enabled.
Date: Wed, 11 Feb 2026 15:19:19 -0500 Subject: [PATCH 07/12] [tb/LEMS-3710/toggle-for-shuffle] docs(changeset): Add a shuffle choices toggle for radio widgets in the content editor --- .changeset/flat-nails-tell.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/flat-nails-tell.md diff --git a/.changeset/flat-nails-tell.md b/.changeset/flat-nails-tell.md new file mode 100644 index 00000000000..d650a71347d --- /dev/null +++ b/.changeset/flat-nails-tell.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus-editor": minor +--- + +Add a shuffle choices toggle for radio widgets in the content editor From b4c8ff4f86750b3313536eb3205dd446d9929712 Mon Sep 17 00:00:00 2001 From: Tamara Date: Wed, 11 Feb 2026 15:24:46 -0500 Subject: [PATCH 08/12] Revert "[tb/claude-progressive-disclosure] Add prompt library docs and update CLAUDE.md" This reverts commit e6df507f51b7cf5a75c4b7d2df7532a14c3bb833. --- .claude/architecture-research.md | 612 -------------------- .claude/prompts/README.md | 33 -- .claude/prompts/codebase-map.md | 95 --- .claude/prompts/component-best-practices.md | 324 ----------- .claude/prompts/file-organization.md | 271 --------- .claude/prompts/iteration-and-feedback.md | 421 -------------- .claude/prompts/math-and-content.md | 270 --------- .claude/prompts/testing-best-practices.md | 181 ------ .claude/prompts/widget-development.md | 286 --------- .claude/widget-patterns-research.md | 551 ------------------ CLAUDE.md | 240 +++++--- 11 files changed, 164 insertions(+), 3120 deletions(-) delete mode 100644 .claude/architecture-research.md delete mode 100644 .claude/prompts/README.md delete mode 100644 .claude/prompts/codebase-map.md delete mode 100644 .claude/prompts/component-best-practices.md delete mode 100644 .claude/prompts/file-organization.md delete mode 100644 .claude/prompts/iteration-and-feedback.md delete mode 100644 .claude/prompts/math-and-content.md delete mode 100644 .claude/prompts/testing-best-practices.md delete mode 100644 .claude/prompts/widget-development.md delete mode 100644 .claude/widget-patterns-research.md diff --git a/.claude/architecture-research.md b/.claude/architecture-research.md deleted file mode 100644 index 5a89eb08279..00000000000 --- a/.claude/architecture-research.md +++ /dev/null @@ -1,612 +0,0 @@ -# Perseus Architecture Research - -**Date**: February 5, 2026 -**Researcher**: Claude (AI Assistant) -**Focus**: Current widget state management, data flow, onChange patterns, and package structure - -## Executive Summary - -Perseus is a well-structured React monorepo with a clear separation of concerns across multiple packages. The current architecture uses a props-driven approach where widgets receive their state through `handleUserInput` callbacks and `userInput` props, rather than the deprecated `onChange` pattern. State management has been intentionally refactored to move away from the old widget-centric model toward a more centralized approach managed by the `UserInputManager` and `Renderer`. - ---- - -## 1. Widget State Management - -### Current Pattern: Props-Driven with Callbacks - -Modern widgets do NOT use the old `onChange` handler. Instead, they: - -1. **Receive state through props**: `userInput: TUserInput` (typed per widget) -2. **Call `handleUserInput` callback** when state changes: `handleUserInput(newUserInput, callback?, silent?)` -3. **Call `trackInteraction` callback** to notify about interactions - -#### Radio Widget Example (Multiple Choice) - -From `/packages/perseus/src/widgets/radio/multiple-choice-widget.tsx`: - -```typescript -type Props = WidgetProps; - -const handleChoiceChange = (choiceId: string, newCheckedState: boolean): void => { - // Build list of checked choice IDs - const checkedChoiceIds: string[] = []; - // ... logic to determine new selection state ... - - // Create new choice states - const newChoiceStates: ChoiceState[] = choiceStates - ? choiceStates.map((state) => ({...state})) - : choices.map(() => ({ - selected: false, - // ... other state fields ... - })); - - // Update states based on selection - newChoiceStates.forEach((choiceState: ChoiceState, i) => { - const choiceId = choices[i].id; - choiceState.selected = checkedChoiceIds.includes(choiceId); - }); - - // Notify parent of change - onChange({choiceStates: newChoiceStates}); // ← Still has onChange! - trackInteraction(); - announceChoiceChange(newChoiceStates); -}; -``` - -**Note**: The Radio widget still uses `onChange`, but this is marked for deprecation (LEMS-3542, LEMS-3245). This is a transitional state during the Radio Revitalization Project. - -#### NumericInput Widget Example - -From `/packages/perseus/src/widgets/numeric-input/numeric-input.tsx`: - -```typescript -const handleChange = (newValue: string): void => { - // Call handleUserInput with new state - props.handleUserInput({currentValue: newValue}); - props.trackInteraction(); -}; - -const handleFocus = (): void => { - props.onFocus([]); - setIsFocused(true); -}; - -const handleBlur = (): void => { - props.onBlur([]); - setIsFocused(false); -}; -``` - -**Key differences from Radio**: -- Uses `handleUserInput` callback (modern pattern) -- No local component state for user input -- Props-driven: receives `props.userInput.currentValue` and sends updates via callback - -### State Storage Location - -**User Input State**: Managed by `UserInputManager` component -- Location: `/packages/perseus/src/user-input-manager.tsx` -- Stored in a central `userInput` object: `UserInputMap` (type from `@khanacademy/perseus-core`) -- Updated via `handleUserInput(widgetId, newUserInput, widgetsEmpty)` callback - -**Widget-Specific State**: -- Some widgets maintain internal component state (see Radio's `choiceStates`) -- This is being transitioned to centralized state management -- Focus management state: `onFocus`, `onBlur` callbacks update parent renderer - ---- - -## 2. Data Flow Architecture - -### High-Level Flow - -``` -ServerItemRenderer - ↓ -UserInputManager (manages userInput state) - ↓ (provides userInput, handleUserInput, initializeUserInput) - ↓ -Renderer (question rendering) - ├─ renderWidget() → WidgetContainer - │ ├─ Passes WidgetProps (from getWidgetProps()) - │ └─ WidgetContainer → Widget Component - │ ├─ Receives userInput prop - │ ├─ Calls handleUserInput on change - │ └─ Calls trackInteraction on user action - │ - └─ Returns UserInputMap to parent -``` - -### Detailed Props Flow - -**Where**: `/packages/perseus/src/renderer.tsx` lines 511-575 - -```typescript -getWidgetProps(widgetId: string): WidgetProps { - const apiOptions = this.getApiOptions(); - const widgetProps = this.props.widgets[widgetId].options; // ← Config from question - const widgetInfo = this.state.widgetInfo[widgetId]; - - return { - ...widgetProps, // ← Widget-specific options - userInput: this.props.userInput?.[widgetId], // ← Current user input - widgetId: widgetId, - widgetIndex: this._getWidgetIndexById(widgetId), - alignment: widgetInfo && widgetInfo.alignment, - static: widgetInfo?.static, - problemNum: this.props.problemNum, - apiOptions: this.getApiOptions(), // ← API configuration - keypadElement: this.props.keypadElement, - showSolutions: this.props.showSolutions, - onFocus: _.partial(this._onWidgetFocus, widgetId), - onBlur: _.partial(this._onWidgetBlur, widgetId), - findWidgets: this.findWidgets, // ← Inter-widget communication - reviewMode: this.props.reviewMode, - - // ← Core state management callback - handleUserInput: (newUserInput: UserInput) => { - const updatedUserInput = { - ...this.props.userInput, - [widgetId]: newUserInput, - }; - const emptyWidgetIds = emptyWidgetsFunctional( - this.state.widgetInfo, - this.widgetIds, - updatedUserInput, - this.context.locale, - ); - const widgetsEmpty = emptyWidgetIds.length > 0; - this.props.handleUserInput?.( - widgetId, - newUserInput, - widgetsEmpty, - ); - this.props.apiOptions?.interactionCallback?.(updatedUserInput); - }, - - trackInteraction: interactionTracker.track, - }; -} -``` - -### User Input Manager Flow - -**Where**: `/packages/perseus/src/user-input-manager.tsx` - -```typescript -export function sharedInitializeUserInput( - widgetOptions: PerseusWidgetsMap | undefined, - problemNum: number, -): UserInputMap { - const startUserInput: UserInputMap = {}; - - // For each widget, call widget's initialization function - Object.entries(widgetOptions).forEach(([id, widgetInfo]) => { - const widgetExports = Widgets.getWidgetExport(widgetInfo.type); - - // Static widgets use correct answer - if (widgetInfo.static && widgetExports?.getCorrectUserInput) { - startUserInput[id] = widgetExports.getCorrectUserInput( - widgetInfo.options, - ); - } - // Other widgets use start input - else if (widgetExports?.getStartUserInput) { - startUserInput[id] = widgetExports.getStartUserInput( - widgetInfo.options, - problemNum ?? 0, - ); - } - }); - - return startUserInput; -} - -// Then in component: -function handleUserInput( - id: string, - nextUserInput: UserInputMap[keyof UserInputMap], - widgetsEmpty: boolean, -) { - const next = { - ...userInput, - [id]: nextUserInput, // ← Update only this widget's input - }; - setUserInput(next); - props.handleUserInput?.(next, widgetsEmpty); -} -``` - ---- - -## 3. onChange Pattern Status - -### Current Status: DEPRECATION IN PROGRESS - -**The `onChange` pattern is being removed but still exists in legacy code.** - -#### Where it's still used: - -1. **Radio Widget** (partially during migration) - - File: `/packages/perseus/src/widgets/radio/radio.ff.tsx` - - Type definition: `/packages/perseus/src/types.ts` line 117 - - Status: TODO marked for removal (LEMS-3542, LEMS-3245) - -2. **Types Still Define It** - - File: `/packages/perseus/src/types.ts` - ```typescript - /** - * TODO(LEMS-3245) remove ChangeHandler - * @deprecated - */ - export type ChangeHandler = ( - arg1: { - hints?: ReadonlyArray; - replace?: boolean; - content?: string; - widgets?: PerseusWidgetsMap; - images?: ImageDict; - choiceStates?: ReadonlyArray; - // ... more fields ... - }, - callback?: () => void, - silent?: boolean, - ) => unknown; - ``` - -#### Where it's NOT used: - -- **NumericInput**: Uses `handleUserInput` directly -- **Most modern widgets**: Use `handleUserInput` callback -- **All new widget code**: Should use the props-driven approach - -#### Migration Path - -The Radio widget wrapper (`radio.ff.tsx`) bridges old and new: - -```typescript -class Radio extends React.Component implements Widget { - _handleChange(arg: {choiceStates?: ReadonlyArray}) { - const newChoiceStates = arg.choiceStates; - // ... converts ChoiceState to UserInput ... - const props = this._mergePropsAndState(); - // Use getUserInputFromSerializedState to get proper UserInput format - const userInput = getUserInputFromSerializedState(mergedProps, ...); - this.props.handleUserInput(userInput); // ← Calls new pattern - } -} -``` - ---- - -## 4. Widget Interface and Lifecycle - -### Widget Interface Definition - -**File**: `/packages/perseus/src/types.ts` lines 60-93 - -```typescript -export interface Widget { - /** - * don't use isWidget; it's just a dummy property to help TypeScript's - * weak typing to recognize non-interactive widgets as Widgets - * @deprecated - */ - isWidget?: true; - - focus?: () => { - id: string; - path: FocusPath; - } | boolean; - - getDOMNodeForPath?: (path: FocusPath) => Element | Text | null; - - /** - * @deprecated - do not use in new code. - */ - getSerializedState?: () => SerializedState; - - blurInputPath?: (path: FocusPath) => void; - focusInputPath?: (path: FocusPath) => void; - getInputPaths?: () => ReadonlyArray; - - getPromptJSON?: () => WidgetPromptJSON; -} -``` - -### WidgetExports Pattern - -**File**: `/packages/perseus/src/types.ts` lines 468-512 - -```typescript -export type WidgetExports< - T extends React.ComponentType & Widget = React.ComponentType, - TUserInput = Empty, -> = Readonly<{ - name: string; - displayName: string; - - widget: T; // The component itself - - version?: Version; - isLintable?: boolean; - tracking?: Tracking; - - // Static widget support - getOneCorrectAnswerFromRubric?: (rubric: WidgetOptions) => string | null; - - // ← Initialization functions - getCorrectUserInput?: (widgetOptions: WidgetOptions) => TUserInput; - getStartUserInput?: ( - widgetOptions: WidgetOptions, - problemNum: number, - ) => TUserInput; - - // ← Deprecation functions - getUserInputFromSerializedState?: ( - serializedState: unknown, - widgetOptions?: WidgetOptions, - ) => TUserInput; -}>; -``` - ---- - -## 5. Package Structure and Dependencies - -### Package Organization - -``` -packages/ -├── perseus/ # Main renderer package (74.0.2) -│ ├── src/ -│ │ ├── renderer.tsx # Core renderer component -│ │ ├── server-item-renderer.tsx # Exercise renderer -│ │ ├── widget-container.tsx # Widget wrapper -│ │ ├── user-input-manager.tsx # State management -│ │ ├── widgets/ # Widget implementations -│ │ │ ├── radio/ -│ │ │ ├── numeric-input/ -│ │ │ ├── interactive-graphs/ -│ │ │ └── ... (40+ widget types) -│ │ ├── types.ts # Core type definitions -│ │ ├── widgets.ts # Widget registry -│ │ └── index.ts # Main exports -│ └── package.json -│ -├── perseus-core/ # Shared types & utilities (23.0.0) -│ ├── src/ -│ │ ├── data-schema.ts # Data type definitions -│ │ ├── scoring-functions/ # Widget-specific scoring -│ │ └── utils/generators # Test data generators -│ └── package.json -│ -├── perseus-score/ # Server-side scoring -│ ├── src/widgets/[type]/score-[type].ts -│ └── widgets/widget-registry.ts -│ -├── perseus-linter/ # Content validation -├── perseus-editor/ # Editor components -├── math-input/ # Math keypad components -└── ... (other packages) -``` - -### Dependencies - -**Perseus Package** depends on: -- `@khanacademy/perseus-core` - Type definitions -- `@khanacademy/perseus-score` - Scoring logic -- `@khanacademy/perseus-linter` - Content validation -- `@khanacademy/math-input` - Math input components -- Wonder Blocks components (UI library) -- React/ReactDOM - -**Perseus-Core** dependencies: -- `@khanacademy/kas` - CAS system -- `@khanacademy/perseus-utils` - Shared utilities -- `@khanacademy/pure-markdown` - Markdown parser -- `tiny-invariant` - Assertions - -### Import Boundaries - -**Rules** (from CLAUDE.md): -- Use package aliases: `@khanacademy/perseus`, `@khanacademy/perseus-core` -- NO file extensions in imports (TypeScript file) -- NO cross-package relative imports -- Import order: builtin > external > internal > relative > types - -**Example correct imports**: -```typescript -import React from "react"; // builtin -import {ApiOptions} from "@khanacademy/perseus"; // internal package -import {WidgetContainer} from "../widget-container"; // relative - -import type {WidgetProps} from "@khanacademy/perseus-core"; -``` - ---- - -## 6. Module Boundaries and Inter-Widget Communication - -### WidgetContainer Boundary - -**File**: `/packages/perseus/src/widget-container.tsx` - -```typescript -type Props = { - shouldHighlight: boolean; - type: string; // ← Widget type - id: string; // ← Widget ID - widgetProps: WidgetProps; // ← All props - linterContext: LinterContextProps; -}; - -class WidgetContainer extends React.Component { - render() { - const WidgetType = Widgets.getWidget(this.props.type); - if (WidgetType == null) { - console.warn(`Widget type '${this.props.type}' not found!`); - return
; - } - - return ( - - - - ); - } -} -``` - -**Responsibilities**: -- Looks up widget component from registry -- Handles error boundaries -- Measures container size on mobile -- Passes props through without modification - -### Inter-Widget Communication - -**File**: `/packages/perseus/src/renderer.tsx` lines 626-683 - -```typescript -findInternalWidgets = (filterCriterion: FilterCriterion) => { - // Returns widgets matching filter - // Filters can be: - // - Widget ID: "interactive-graph 3" - // - Widget type: "interactive-graph" - // - Function: (id, widgetInfo, widget) => boolean -}; - -findWidgets = (filterCriterion: FilterCriterion) => { - // Combines internal and external widgets - return [ - ...this.findInternalWidgets(filterCriterion), - ...this.props.findExternalWidgets(filterCriterion), - ]; -}; -``` - -**Usage**: Passed to all widgets as `findWidgets` prop, enabling: -- Graded group widgets to find their children -- Custom widgets to communicate with other widgets -- Cross-renderer communication through parent's `findExternalWidgets` - ---- - -## 7. Key Files and Their Responsibilities - -| File | Responsibility | -|------|-----------------| -| `server-item-renderer.tsx` | Top-level exercise renderer; manages hints | -| `renderer.tsx` | Core question renderer; widget lifecycle | -| `widget-container.tsx` | Widget wrapper; error boundaries | -| `user-input-manager.tsx` | Central state management for user input | -| `types.ts` | Core type definitions (Widget, WidgetProps, etc.) | -| `widgets.ts` | Widget registry and lookup | -| `widgets/[type]/index.ts` | Widget export entry point | -| `widgets/[type]/[type].ts` | Widget registration (WidgetExports) | -| `widgets/[type]/[component].tsx` | Widget component implementation | - ---- - -## 8. Recent Architecture Changes - -### Recent PRs (from git log): -1. **Upgrade to Jest v30** (#3230) - Testing infrastructure -2. **Fix parsing of answerless Label Image widgets** (#3231) - Bug fix -3. **Remove unused `isItemRenderableByVersion`** (#3229) - Code cleanup -4. **Delete TODOs we're never going to fix** (#3197) - Debt reduction -5. **Simplify applyDefaultsToWidgets** (#3217) - Refactoring - -### Known Deprecations - -- `getSerializedState()` - Use `getStartUserInput()` instead (LEMS-3185) -- `onChange` handler - Use `handleUserInput()` instead (LEMS-3245, LEMS-3542) -- `ChangeHandler` type - Being phased out -- Old Radio widget files - Being replaced (LEMS-2994) - ---- - -## 9. Testing Architecture - -### Test Patterns - -**Location**: Widgets typically have `[widget].test.ts` or `.test.tsx` files - -**Key testing helpers** (from `packages/perseus-core/src/utils/generators`): -- Widget generators for creating test data -- testdata files for example questions - -**Example structure** (from CLAUDE.md): -```typescript -import {render, screen} from "@testing-library/react"; -import {userEvent} from "@testing-library/user-event"; -import {question1} from "../__testdata__/widget.testdata"; -import WidgetComponent from "../widget-component"; - -describe("WidgetComponent", () => { - it("renders correctly", () => { - render(); - expect(screen.getByRole("button")).toBeInTheDocument(); - }); - - it("handles user interaction", async () => { - const user = userEvent.setup(); - const onChange = jest.fn(); - render(); - await user.click(screen.getByRole("button")); - expect(onChange).toHaveBeenCalled(); - }); -}); -``` - ---- - -## 10. Key Insights and Findings - -### Ground Truth: Modern Widget Pattern - -1. **No onChange**: Modern widgets use `handleUserInput(newState)` callback -2. **Props-driven**: Widget state is passed via props, not stored internally -3. **Centralized state**: `UserInputManager` holds all widget states in one place -4. **Lazy initialization**: Widgets use `getStartUserInput()` to initialize state - -### Current Transition State - -- **Radio widget**: Partially migrated (still uses `onChange` internally) -- **NumericInput widget**: Fully migrated to modern pattern -- **Legacy code**: Still contains `onChange` and `ChangeHandler` types -- **Feature flags**: Used to conditionally render new Radio widget (radio.ff.tsx) - -### Architecture Strengths - -1. **Clear separation**: Renderer, WidgetContainer, Widgets have distinct roles -2. **Type safety**: Extensive use of TypeScript generics for widget props -3. **Extensibility**: Widget registry system allows plugins -4. **Testability**: Props-driven design makes widgets easy to test -5. **Error handling**: Error boundaries at widget level - -### Areas of Complexity - -1. **State shape duality**: Some widgets still have both `ChoiceState` and `UserInput` -2. **Serialization deprecation**: Old serialized state format still supported for backwards compatibility -3. **Focus management**: FocusPath system is complex but necessary for accessibility -4. **Feature flags**: Multiple codepaths for old vs. new Radio implementation - ---- - -## Conclusion - -Perseus has a well-designed, modern React architecture with clear data flow. The system is transitioning from widget-centric state management (onChange) to centralized state management (UserInputManager + handleUserInput callbacks). This research shows the actual current state of the codebase, which differs from some of the documentation in that modern widgets are fully prop-driven with no onChange pattern at all. - -The key takeaway: **When building or modifying widgets, use `handleUserInput()` callbacks, NOT `onChange` handlers**. The latter is deprecated and being actively removed. - ---- - -**Co-authored by**: Claude (AI Assistant) -**Research Depth**: Thorough - examined 15+ source files and traced data flow through the entire system diff --git a/.claude/prompts/README.md b/.claude/prompts/README.md deleted file mode 100644 index e7644b8c47f..00000000000 --- a/.claude/prompts/README.md +++ /dev/null @@ -1,33 +0,0 @@ -# Perseus Prompt Library - -This directory contains modular documentation for AI assistants working on the Perseus codebase. Each document focuses on a specific aspect of development. - -## Available Documents - -### Core References -- **[codebase-map.md](./codebase-map.md)** - Package structure, data flow, and module boundaries -- **[widget-development.md](./widget-development.md)** - Complete guide for creating and maintaining widgets -- **[testing-best-practices.md](./testing-best-practices.md)** - Testing patterns, utilities, and guidelines - -### Development Guidelines -- **[component-best-practices.md](./component-best-practices.md)** - React components, Wonder Blocks, accessibility -- **[file-organization.md](./file-organization.md)** - Directory structure, naming conventions, imports -- **[iteration-and-feedback.md](./iteration-and-feedback.md)** - When to ask for help vs. iterate autonomously - -## How to Use - -These documents are referenced from the main `CLAUDE.md` file. When working on specific tasks, the relevant documents will be loaded into context. - -## Maintenance - -When updating these documents: -1. Verify information against actual code (not just documentation) -2. Use concrete examples from the codebase -3. Include file paths and line numbers where helpful -4. Mark deprecated patterns clearly -5. Date any time-sensitive information - -## Recent Updates - -- **2024-02-05**: Initial prompt library created based on current codebase analysis -- **2024-02-05**: Corrected widget patterns after discovering Rubric is still used (but only for scoring) \ No newline at end of file diff --git a/.claude/prompts/codebase-map.md b/.claude/prompts/codebase-map.md deleted file mode 100644 index 14ffc21d23e..00000000000 --- a/.claude/prompts/codebase-map.md +++ /dev/null @@ -1,95 +0,0 @@ -# Perseus Codebase Map - -## Package Structure Overview - -``` -packages/ -├── perseus/ (v74.0.2) # Core rendering engine -│ ├── src/ -│ │ ├── widgets/ # Interactive components -│ │ ├── components/ # Reusable UI components -│ │ ├── renderers/ # Content renderers -│ │ ├── util/ # Utility functions -│ │ └── __docs__/ # Storybook documentation -│ └── __testdata__/ # Test fixtures -│ -├── perseus-editor/ # Content authoring tools -│ ├── src/ -│ │ ├── components/ # Editor UI components -│ │ └── widgets/ # Widget editors -│ -├── perseus-core/ (v23.0.0) # Shared types and utilities -│ ├── src/ -│ │ ├── data-schema.ts # Core type definitions -│ │ ├── utils/ -│ │ │ └── generators/ # Test data generators -│ │ └── types/ # Type exports -│ -├── perseus-score/ # Server-side scoring -│ ├── src/ -│ │ └── widgets/ # Widget-specific scoring -│ -├── math-input/ # Math keypad and input -│ ├── src/ -│ │ ├── components/ # Math UI components -│ │ └── services/ # Math processing -│ -└── perseus-linter/ # Content validation - └── src/ - └── rules/ # Linting rules -``` - -## Key Entry Points - -### Renderers & State Management -- `packages/perseus/src/server-item-renderer.tsx` - Main exercise renderer -- `packages/perseus/src/user-input-manager.ts` - Centralized state management -- `packages/perseus/src/renderer.tsx` - Base renderer component -- `packages/perseus/src/widget-container.tsx` - Widget boundary layer - -### Widget System -- `packages/perseus/src/widgets.ts` - Widget registry -- Individual widgets in `packages/perseus/src/widgets/[name]/` -- Modern widgets use `handleUserInput()` callback pattern -- Legacy widgets being migrated from `onChange` pattern - -### Type System -- `packages/perseus-core/src/data-schema.ts` - Core types -- `packages/perseus-core/src/types/` - Type exports -- Widget-specific types in each widget directory - -## Modern Data Flow (Post-Migration) - -1. **State Initialization**: - - ServerItemRenderer → UserInputManager.getStartUserInput() - -2. **User Interaction**: - - Widget → handleUserInput() → UserInputManager → Re-render - -3. **Props Distribution**: - - UserInputManager.getWidgetProps() → WidgetContainer → Widget - -4. **Scoring**: - - UserInputManager state → Perseus Score → Validation result - -## Important Migration Notes - -- **onChange is DEPRECATED** (LEMS-3245, LEMS-3542) -- New widgets must use `handleUserInput()` pattern -- Radio widget currently bridges old/new patterns during migration -- NumericInput is fully modernized example - -## Module Boundaries - -- **perseus**: Public rendering API (React-based) -- **perseus-editor**: Editor-only functionality -- **perseus-core**: Shared types (no React dependencies) -- **perseus-score**: Server-side scoring (no React) -- **math-input**: Standalone math components - -## Import Rules - -- Use package aliases: `@khanacademy/perseus`, etc. -- NO file extensions in imports -- NO cross-package relative imports -- Import order: builtin > external > internal > relative > types \ No newline at end of file diff --git a/.claude/prompts/component-best-practices.md b/.claude/prompts/component-best-practices.md deleted file mode 100644 index cf4cfcd69d2..00000000000 --- a/.claude/prompts/component-best-practices.md +++ /dev/null @@ -1,324 +0,0 @@ -# Component Development Best Practices - -## Component Structure - -### Prefer Functional Components -```typescript -// ✅ Good - Named function -function ComponentName(props: Props) { - return
{props.content}
; -} -ComponentName.displayName = "ComponentName"; // Explicit for debugging - -// ❌ Avoid - Arrow function requires displayName -const ComponentName = (props: Props) => { - return
{props.content}
; -}; -ComponentName.displayName = "ComponentName"; // Extra step -``` - -### Type Definitions -```typescript -// Place types at the top of the file -type Props = { - content: string; - onAction: () => void; - optional?: boolean; -}; - -// For complex props, use intersection types -type Props = BaseProps & { - specificProp: string; -}; -``` - -## Wonder Blocks Integration - -Perseus uses Khan Academy's Wonder Blocks design system: - -```typescript -import {View} from "@khanacademy/wonder-blocks-core"; -import {Button} from "@khanacademy/wonder-blocks-button"; -import {TextField} from "@khanacademy/wonder-blocks-form"; -import {LabelMedium} from "@khanacademy/wonder-blocks-typography"; - -function MyComponent() { - return ( - - Label text - - - - ); -} -``` - -## State Management Patterns - -### Component State (UI only) -```typescript -// Local state for UI concerns only -function Component() { - const [isExpanded, setIsExpanded] = React.useState(false); - const [hoveredIndex, setHoveredIndex] = React.useState(null); - - // User answer state goes through handleUserInput, not local state! -} -``` - -### Derived State -```typescript -// Calculate values instead of storing them -function Component({items, filter}: Props) { - // ✅ Good - Derived from props - const filteredItems = React.useMemo( - () => items.filter(item => item.matches(filter)), - [items, filter] - ); - - // ❌ Bad - Duplicating prop data in state - const [filteredItems, setFilteredItems] = React.useState([]); - React.useEffect(() => { - setFilteredItems(items.filter(...)); - }, [items, filter]); -} -``` - -## Performance Optimization - -### Memoization -```typescript -// Memoize expensive calculations -const expensiveValue = React.useMemo(() => { - return computeExpensiveValue(data); -}, [data]); - -// Memoize callbacks passed to children -const handleClick = React.useCallback(() => { - doSomething(value); -}, [value]); - -// Memoize component when props comparison is cheaper than re-render -const MemoizedChild = React.memo(ChildComponent); -``` - -### Avoid Inline Objects/Functions -```typescript -// ❌ Bad - Creates new object every render - - -// ✅ Good - Stable reference -const style = {margin: 10}; - - -// Or use Wonder Blocks styling - -``` - -## Accessibility - -### ARIA Labels -```typescript - -``` - -### Focus Management -```typescript -function Component() { - const inputRef = React.useRef(null); - - // Auto-focus on mount when appropriate - React.useEffect(() => { - if (shouldAutoFocus) { - inputRef.current?.focus(); - } - }, [shouldAutoFocus]); - - return ; -} -``` - -### Keyboard Navigation -```typescript -function handleKeyDown(e: React.KeyboardEvent) { - switch (e.key) { - case "Enter": - submitAnswer(); - break; - case "Escape": - clearInput(); - break; - case "ArrowUp": - selectPrevious(); - e.preventDefault(); // Prevent scroll - break; - } -} -``` - -## Error Handling - -### Error Boundaries -```typescript -// Components are wrapped by error boundaries -// Provide meaningful error states -function Component({data}: Props) { - if (!data) { - return No data available; - } - - try { - return ; - } catch (error) { - return Failed to render content; - } -} -``` - -## Mobile Support - -### Touch-Friendly Interfaces -```typescript -// Use Wonder Blocks components (mobile-optimized) -import {Button} from "@khanacademy/wonder-blocks-button"; - -// Ensure touch targets are at least 44x44px - -``` - -### Responsive Layout -```typescript -import {useIsMobile} from "../hooks/use-is-mobile"; - -function Component() { - const isMobile = useIsMobile(); - - return ( - - {/* Content adapts to screen size */} - - ); -} -``` - -## Component Organization - -### File Structure -```typescript -// component-name.tsx - -// 1. Imports -import React from "react"; -import {View} from "@khanacademy/wonder-blocks-core"; - -// 2. Type definitions -type Props = { - // ... -}; - -// 3. Constants -const DEFAULT_VALUE = ""; - -// 4. Main component -function ComponentName(props: Props) { - // ... -} - -// 5. Display name -ComponentName.displayName = "ComponentName"; - -// 6. Sub-components (if needed) -function SubComponent() { - // ... -} - -// 7. Export -export default ComponentName; -``` - -### Props Patterns -```typescript -// Destructure common props -function Component({ - userInput, - handleUserInput, - trackInteraction, - ...restProps -}: Props) { - // Use restProps for pass-through - return ; -} -``` - -## Testing Components - -### Render Testing -```typescript -import {render, screen} from "@testing-library/react"; -import {userEvent} from "@testing-library/user-event"; - -it("handles interaction", async () => { - const user = userEvent.setup(); - const onClick = jest.fn(); - - render(); - await user.click(screen.getByRole("button")); - - expect(onClick).toHaveBeenCalled(); -}); -``` - -### Snapshot Testing (use sparingly) -```typescript -it("renders complex structure", () => { - const {container} = render(); - expect(container).toMatchSnapshot(); -}); -``` - -## Common Patterns - -### Conditional Rendering -```typescript -// Simple conditions -{showHint && } - -// Multiple conditions -{(() => { - if (loading) return ; - if (error) return ; - return ; -})()} -``` - -### Lists and Keys -```typescript -{items.map((item, index) => ( - handleSelect(item.id)} - /> -))} -``` - -## Anti-Patterns to Avoid - -1. **Don't mutate props or state directly** -2. **Don't use array index as key when list can reorder** -3. **Don't put business logic in components** - Keep in utilities -4. **Don't use inline styles** - Use Wonder Blocks or CSS modules -5. **Don't forget React.memo for expensive child components** -6. **Don't use useEffect for derived state** - Use useMemo instead \ No newline at end of file diff --git a/.claude/prompts/file-organization.md b/.claude/prompts/file-organization.md deleted file mode 100644 index 164c6b47064..00000000000 --- a/.claude/prompts/file-organization.md +++ /dev/null @@ -1,271 +0,0 @@ -# File and Folder Organization - -## Directory Naming Conventions - -### Use Kebab-Case for Directories -``` -✅ Good: -packages/perseus/src/widgets/numeric-input/ -packages/perseus/src/widgets/interactive-graph/ - -❌ Bad: -packages/perseus/src/widgets/NumericInput/ -packages/perseus/src/widgets/interactive_graph/ -``` - -## Widget File Structure - -Each widget should follow this structure: -``` -widgets/widget-name/ -├── index.ts # Public exports only -├── widget-name.tsx # Main component -├── widget-name.test.ts # Component tests -├── widget-name.testdata.ts # Test data using generators -├── types.ts # Widget-specific types (if complex) -├── utils.ts # Widget-specific utilities (if needed) -└── __docs__/ - ├── widget-name.stories.tsx # Storybook stories - ├── a11y.mdx # Accessibility documentation - └── examples/ # Example JSON fixtures (if needed) -``` - -## Component Organization - -### Simple Components (single file) -``` -components/ -├── simple-component.tsx -├── simple-component.test.tsx -└── simple-component.stories.tsx -``` - -### Complex Components (folder structure) -``` -components/complex-component/ -├── index.ts # Public exports -├── complex-component.tsx # Main component -├── complex-component.test.tsx # Tests -├── sub-component.tsx # Private sub-components -├── utils.ts # Component-specific utils -└── types.ts # Component-specific types -``` - -## File Naming Rules - -### TypeScript/JavaScript Files -```typescript -// Components - PascalCase file, named export -widget-name.tsx → export function WidgetName() {} - -// Utilities - kebab-case file, named exports -math-utils.ts → export function calculateSum() {} - -// Types - kebab-case file with .types.ts extension -validation.types.ts → export type ValidationResult = {} - -// Test files - match source with .test.ts -widget-name.tsx → widget-name.test.ts - -// Test data - match source with .testdata.ts -widget-name.tsx → widget-name.testdata.ts -``` - -### Index Files -```typescript -// index.ts should only re-export, no logic -export {default} from "./widget-name"; -export type {WidgetNameProps} from "./types"; - -// Don't put implementation in index files -``` - -## Import Organization - -### Import Order (enforced by ESLint) -```typescript -// 1. Node built-ins -import fs from "fs"; -import path from "path"; - -// 2. External packages -import React from "react"; -import {render} from "@testing-library/react"; - -// 3. Internal packages (Khan Academy) -import {View} from "@khanacademy/wonder-blocks-core"; -import {PerseusScore} from "@khanacademy/perseus-core"; - -// 4. Relative imports -import {WidgetContainer} from "../widget-container"; -import {calculateAnswer} from "./utils"; - -// 5. Type imports (always last) -import type {WidgetProps} from "@khanacademy/perseus-core"; -import type {LocalType} from "./types"; -``` - -## Test File Organization - -### Test Data Files -```typescript -// widget-name.testdata.ts -import {widgetNameGenerator} from "@khanacademy/perseus-core"; - -// Group related test cases -export const basicQuestions = { - simple: widgetNameGenerator.build({...}), - withHint: widgetNameGenerator.build({...}), - multipleChoice: widgetNameGenerator.build({...}), -}; - -export const edgeCases = { - empty: widgetNameGenerator.build({...}), - veryLong: widgetNameGenerator.build({...}), -}; -``` - -### Test Structure -```typescript -// widget-name.test.ts -describe("WidgetName", () => { - describe("rendering", () => { - it("renders with default props", () => {}); - it("renders with custom props", () => {}); - }); - - describe("user interaction", () => { - it("handles input changes", () => {}); - it("validates input", () => {}); - }); - - describe("scoring", () => { - it("scores correct answer", () => {}); - it("scores incorrect answer", () => {}); - }); -}); -``` - -## Package Exports - -### Main Package Exports (packages/perseus/src/index.ts) -```typescript -// Group exports by category -// Renderers -export {default as ServerItemRenderer} from "./server-item-renderer"; -export {default as ArticleRenderer} from "./article-renderer"; - -// Widgets -export {widgets} from "./widgets"; - -// Types -export type {WidgetProps} from "./types"; -``` - -### Package.json Exports -```json -{ - "exports": { - ".": "./dist/index.js", - "./styles": "./dist/styles.css" - } -} -``` - -## Documentation Files - -### Required Documentation -``` -widget-name/ -└── __docs__/ - ├── widget-name.stories.tsx # Interactive examples - ├── a11y.mdx # Accessibility guide - └── README.mdx # Usage documentation (optional) -``` - -### Storybook Organization -```typescript -// __docs__/widget-name.stories.tsx -export default { - title: "Perseus/Widgets/WidgetName", // Hierarchical organization - component: WidgetName, -}; - -// Stories in logical order -export const Default = {}; -export const WithHint = {}; -export const Mobile = {}; -export const ErrorState = {}; -``` - -## Asset Organization - -### Static Assets -``` -packages/perseus/src/ -├── assets/ -│ └── images/ # Shared images -├── widgets/ -│ └── widget-name/ -│ └── assets/ # Widget-specific assets -``` - -### Styles -``` -packages/perseus/src/ -├── styles/ -│ ├── global.less # Global styles -│ └── variables.less # Shared variables -├── widgets/ -│ └── widget-name/ -│ └── widget-name.less # Widget-specific styles -``` - -## Build Artifacts - -### Git Ignored -``` -# Never commit these -node_modules/ -dist/ -build/ -*.tsbuildinfo -coverage/ -.turbo/ -``` - -### Generated Files -```typescript -// Mark generated files clearly -// Generated by: script-name.js -// DO NOT EDIT MANUALLY -``` - -## Migration Patterns - -### Gradual Migration -``` -widgets/old-widget/ # Legacy structure -├── old-widget.jsx # To be migrated -├── old-widget-new.tsx # New implementation -└── __migration__/ # Migration utilities -``` - -### Deprecation -```typescript -// Mark deprecated code clearly -/** - * @deprecated Use NewWidget instead (LEMS-1234) - * @removeBefore 2024-12-01 - */ -export const OldWidget = () => {}; -``` - -## Best Practices - -1. **Keep files focused** - Single responsibility per file -2. **Co-locate related code** - Tests, stories, and component together -3. **Use index.ts for public API** - Hide internal implementation -4. **Consistent naming** - Follow conventions strictly -5. **Clean imports** - No circular dependencies -6. **Document complex structures** - Add README.md when needed \ No newline at end of file diff --git a/.claude/prompts/iteration-and-feedback.md b/.claude/prompts/iteration-and-feedback.md deleted file mode 100644 index bcf293a0b2b..00000000000 --- a/.claude/prompts/iteration-and-feedback.md +++ /dev/null @@ -1,421 +0,0 @@ -# Iteration and Feedback Guidelines - -## When to Seek Human Feedback - -### Always Ask When: -1. **Architectural decisions** - Choosing between different design patterns -2. **Breaking changes** - Modifications that affect external APIs -3. **Performance trade-offs** - Optimizing for speed vs. memory vs. readability -4. **Business logic ambiguity** - Multiple valid interpretations of requirements -5. **User experience changes** - Altering how users interact with features -6. **Data model changes** - Modifying core types or data structures - -### Example Decision Points: -```typescript -// ASK: Which error handling approach? -// Option A: Show inline error -// Option B: Toast notification -// Option C: Modal dialog - -// ASK: State management strategy? -// Option A: Local component state -// Option B: Centralized in UserInputManager -// Option C: Context provider -``` - -## Autonomous Iteration Patterns - -### Can Iterate Without Asking: -1. **Fixing clear bugs** - Null pointer, type errors, off-by-one errors -2. **Following established patterns** - Using existing widget patterns -3. **Code formatting** - Prettier, ESLint fixes -4. **Adding missing tests** - Improving coverage for existing code -5. **Refactoring for clarity** - Extracting functions, renaming variables -6. **Performance optimizations** - When metrics clearly show improvement - -### Iteration Workflow: -```typescript -// 1. Initial implementation -function calculateScore(answer: string): number { - return answer === "42" ? 1 : 0; -} - -// 2. Identify issue (too simplistic) -// Can iterate autonomously because pattern is clear - -// 3. Improved implementation -function calculateScore( - userInput: UserInput, - rubric: Rubric, -): PerseusScore { - const isCorrect = userInput.value === rubric.correctAnswer; - return { - type: "points", - earned: isCorrect ? 1 : 0, - total: 1, - message: null, - }; -} -``` - -## Testing Feedback Loop - -### Run Tests → Analyze → Fix → Repeat -```bash -# Autonomous iteration process -pnpm test widget-name - -# If test fails with clear error: -# ✅ Fix autonomously - -# If test fails with ambiguous requirement: -# ❌ Ask for clarification -``` - -### Test Failure Decision Tree: -``` -Test Failure -├── Type Error → Fix autonomously -├── Assertion Failure -│ ├── Clear expectation → Fix autonomously -│ └── Business logic unclear → Ask human -├── Timeout -│ ├── Missing async/await → Fix autonomously -│ └── Performance issue → Discuss approach -└── Snapshot Mismatch - ├── Intentional change → Update snapshot - └── Unexpected change → Investigate cause -``` - -## Code Review Self-Checklist - -### Before Presenting Code: -- [ ] Tests pass (`pnpm test`) -- [ ] Types check (`pnpm tsc`) -- [ ] Linted (`pnpm lint`) -- [ ] Formatted (`pnpm prettier . --write`) -- [ ] Accessibility documented -- [ ] Mobile tested (if applicable) -- [ ] Performance impact considered - -### Autonomous Fixes: -```typescript -// These issues should be fixed without asking: - -// ❌ Unused import (ESLint) -import {unused} from "./utils"; - -// ❌ Console statement (ESLint) -console.log("debug"); - -// ❌ Missing type (TypeScript) -function process(data) { // Should be: data: DataType - -// ❌ Formatting (Prettier) -const x={a:1,b:2}; // Should be: const x = {a: 1, b: 2}; -``` - -## Incremental Development - -### Build → Test → Refine Cycle -```typescript -// Iteration 1: Basic functionality -function Widget() { - return ; -} - -// Iteration 2: Add user input handling (autonomous) -function Widget({userInput, handleUserInput}) { - return ( - handleUserInput({value: e.target.value})} - /> - ); -} - -// Iteration 3: Add validation (check pattern first) -// If validation rules are clear → implement -// If business rules unclear → ask -``` - -## Performance Optimization Decisions - -### Autonomous Optimizations: -```typescript -// Clear performance win - optimize without asking -// Before: O(n²) -const hasDuplicate = items.some( - (item, i) => items.slice(i + 1).includes(item) -); - -// After: O(n) -const hasDuplicate = items.length !== new Set(items).size; -``` - -### Needs Discussion: -```typescript -// Performance vs. Readability trade-off -// Option A: Readable but slower -const result = items - .filter(item => item.active) - .map(item => item.value) - .reduce((sum, val) => sum + val, 0); - -// Option B: Faster but less readable -let result = 0; -for (let i = 0; i < items.length; i++) { - if (items[i].active) result += items[i].value; -} -// ASK: Which approach aligns with team preferences? -``` - -## Error Handling Patterns - -### Autonomous Error Handling: -```typescript -// Null checks - add without asking -function process(data?: DataType) { - if (!data) { - return defaultValue; // Clear fallback - } - // ... process data -} - -// Try-catch for parsing - add without asking -try { - const parsed = JSON.parse(userInput); - return parsed; -} catch { - return null; // Safe fallback -} -``` - -### Needs Human Input: -```typescript -// User-facing error messages -function validateInput(value: string) { - if (!value) { - // ASK: What message helps users best? - // Option A: "This field is required" - // Option B: "Please enter your answer" - // Option C: "Answer cannot be empty" - } -} -``` - -## Documentation Decisions - -### Auto-document: -- Public APIs (JSDoc for complex functions) -- Accessibility requirements -- Component props -- Test scenarios - -### Ask Before Documenting: -- Architecture decisions (ADRs) -- Migration guides -- Public-facing documentation -- API breaking changes - -## Useful Feedback Prompts - -When uncertain, use these prompts: - -1. **"I see two approaches here..."** - Present options with trade-offs -2. **"The tests suggest X but the code does Y..."** - Highlight inconsistencies -3. **"This would be cleaner if..."** - Propose refactoring -4. **"I'm assuming X because..."** - Make assumptions explicit -5. **"Would you prefer..."** - Get style/pattern preferences - -## Continuous Improvement - -### Track Patterns: -```typescript -// When you make the same fix repeatedly, suggest: -"I notice we're frequently fixing X. Should we add a linting rule?" -"This pattern appears often. Should we create a utility function?" -"Multiple widgets need this. Should we move it to shared code?" -``` - -### Learn from Feedback: -- Note preference corrections -- Update approach based on feedback -- Suggest codifying patterns in documentation - -## Debugging Practices - -### Console Debugging -```typescript -// Temporary debugging (remove before commit) -console.log("Widget state:", state); -console.log("Props received:", props); -console.log("Calculated value:", result); - -// Better: Use descriptive messages -console.log("[WidgetName] Handling user input:", userInput); -console.log("[WidgetName] Validation result:", isValid); -``` - -### Remember to Clean Up -- Remove ALL console statements before commit -- ESLint will catch these in pre-commit -- Use debugger statements sparingly -- Clean up TODO comments - -### Error Boundaries -Perseus widgets are wrapped in error boundaries. When debugging: - -1. **Check browser console** for widget-specific errors -2. **Look for error boundary messages** in the UI -3. **Implement graceful fallbacks**: -```typescript -function WidgetComponent({data}: Props) { - if (!data || !data.required) { - // Graceful fallback - return Unable to load widget content; - } - - try { - return ; - } catch (error) { - // Log for debugging but show user-friendly message - console.error("[WidgetName] Render error:", error); - return This content is temporarily unavailable; - } -} -``` - -### Storybook Development & Debugging -Use Storybook for isolated development: - -1. **Test different prop combinations** - Create stories for edge cases -2. **Verify accessibility** - Use the a11y addon to catch issues -3. **Check mobile layouts** - Use device frame addon -4. **Debug interactions** - Use actions addon to log callbacks -5. **Visual debugging** - Compare component states side-by-side - -```typescript -// Useful Storybook patterns for debugging -export const DebugStory = { - args: { - // Test with extreme values - value: "Very long text that might break layout...", - options: Array(100).fill("Option"), - }, - play: async ({canvasElement}) => { - // Automated interaction testing - const canvas = within(canvasElement); - const input = canvas.getByRole("textbox"); - await userEvent.type(input, "test"); - }, -}; -``` - -## Deployment & Pre-Submit Checklist - -### Before Submitting PR - -#### Required Checks -```bash -# 1. Run full test suite -pnpm test - -# 2. Type checking -pnpm tsc -# or -pnpm build:types - -# 3. Linting (with auto-fix) -pnpm lint --fix - -# 4. Format code -pnpm prettier . --write - -# 5. Build packages to ensure no build errors -pnpm build - -# 6. Test in Storybook -pnpm storybook -``` - -#### Manual Verification -- [ ] Accessibility documented (new widgets need a11y.mdx) -- [ ] Mobile tested (touch interactions work) -- [ ] Performance impact assessed (no unnecessary re-renders) -- [ ] Error cases handled gracefully -- [ ] Console clear of debug statements - -### Common Pre-commit Failures & Fixes - -#### ESLint Failures -```typescript -// ❌ Unused imports -import {unused} from "./utils"; // Remove - -// ❌ Console statements -console.log("debug"); // Remove or use proper logging - -// ❌ Missing dependencies in hooks -useEffect(() => { - doSomething(value); -}, []); // Add 'value' to dependency array - -// ❌ Prefer const -let unchangedVar = 5; // Change to const -``` - -#### Prettier Failures -```typescript -// Auto-fix with: pnpm prettier . --write - -// Common issues: -// - Inconsistent quotes (use double quotes) -// - Missing semicolons -// - Incorrect indentation (4 spaces) -// - Line length > 80 characters -``` - -#### TypeScript Failures -```typescript -// ❌ Implicit any -function process(data) {} // Add type: data: DataType - -// ❌ Missing return type -function calculate(x: number) { // Add : number - return x * 2; -} - -// ❌ Type mismatch -const value: string = 42; // Fix type or value -``` - -#### Test Failures -```bash -# Run specific test to debug -pnpm test path/to/test.test.ts - -# Update snapshots if changes are intentional -pnpm test -u - -# Run with coverage -pnpm test --coverage -``` - -### Post-Submit Monitoring - -After PR is merged: -1. Monitor for any reverted changes -2. Check for related bug reports -3. Watch performance metrics -4. Respond to code review feedback - -### Rollback Preparation - -Be prepared to revert if issues arise: -```bash -# Create revert PR if needed -git revert -git push origin revert-branch -# Create PR with explanation of issue -``` \ No newline at end of file diff --git a/.claude/prompts/math-and-content.md b/.claude/prompts/math-and-content.md deleted file mode 100644 index b3185d85292..00000000000 --- a/.claude/prompts/math-and-content.md +++ /dev/null @@ -1,270 +0,0 @@ -# Math and Content Rendering - -## Math Rendering (TeX) - -### Basic Syntax -- **Inline math**: Use `$...$` for math within text - - Example: `The answer is $x = 42$` -- **Display math**: Use `$$...$$` for centered, standalone equations - - Example: `$$\int_0^\infty e^{-x^2} dx = \frac{\sqrt{\pi}}{2}$$` - -### Complex Expressions - -#### Fractions -```latex -// ✅ Good - Use \dfrac for display-style fractions -$$\dfrac{a+b}{c+d}$$ - -// ❌ Avoid - \frac can be too small in complex expressions -$$\frac{a+b}{c+d}$$ - -// For inline, \frac is acceptable -The fraction $\frac{1}{2}$ is equivalent to 0.5 -``` - -#### Common Patterns -```latex -// Aligned equations -\begin{align} -x + y &= 10 \\ -2x - y &= 5 -\end{align} - -// Matrices -\begin{bmatrix} -a & b \\ -c & d -\end{bmatrix} - -// Cases -f(x) = \begin{cases} -x^2 & \text{if } x > 0 \\ -0 & \text{otherwise} -\end{cases} -``` - -### Testing Math Rendering - -Test math in different contexts: -1. **Articles** - Both inline and display math -2. **Exercise questions** - Main problem statement -3. **Hints** - Step-by-step solutions -4. **Answer choices** - In radio/multiple choice widgets -5. **Feedback messages** - Correct/incorrect explanations - -### MathJax Configuration - -Perseus uses MathJax for rendering. Key points: -- Automatic equation numbering disabled -- Custom macros available (check `math-input` package) -- Accessibility features enabled (screen reader support) - -### Common Math Issues - -#### Issue: Math not rendering -```typescript -// ❌ Problem: Unescaped backslashes -const tex = "The answer is $\frac{1}{2}$"; - -// ✅ Solution: Escape or use raw strings -const tex = "The answer is $\\frac{1}{2}$"; -const tex = String.raw`The answer is $\frac{1}{2}$`; -``` - -#### Issue: Spacing problems -```latex -// Add manual spacing when needed -$x\,\text{cm}$ // Thin space -$x\ \text{units}$ // Normal space -$x\quad\text{m}$ // Quad space -``` - -## Content Structure - -### Perseus Content Format -```json -{ - "question": { - "content": "What is $2 + 2$?", - "widgets": { - "numeric-input 1": { - "type": "numeric-input", - "options": { - "answers": [{ - "value": 4, - "status": "correct" - }] - } - } - } - }, - "hints": [ - {"content": "Think about counting on your fingers."}, - {"content": "The answer is $4$."} - ] -} -``` - -### Widget References - -Widgets are referenced in content using special syntax: -```markdown -Enter your answer: [[☃ numeric-input 1]] - -Choose the correct option: [[☃ radio 1]] -``` - -The snowman character (☃) is used as a unique delimiter. - -### Markdown Extensions - -Perseus extends standard Markdown: - -#### Columns -```markdown -::: columns -::: column -Left column content -::: -::: column -Right column content -::: -::: -``` - -#### Callout Boxes -```markdown -::: callout -**Important:** This is a callout box. -::: -``` - -#### Images with Alignment -```markdown -![Description](url.png){: .align-right} -``` - -## Content Validation - -### Linting Rules - -The `perseus-linter` package enforces: -- Valid widget references -- Proper math syntax -- Accessible image alt text -- No broken links -- Consistent formatting - -### Running the Linter -```bash -pnpm --filter perseus-linter lint-content content.json -``` - -## Accessibility in Content - -### Math Accessibility -- All math automatically gets ARIA labels -- MathJax provides screen reader support -- Test with screen readers for complex expressions - -### Image Guidelines -```markdown -// ✅ Good - Descriptive alt text -![Graph showing linear growth from 0 to 100 over 10 years](graph.png) - -// ❌ Bad - Generic or missing alt text -![Graph](graph.png) -![](graph.png) -``` - -### Widget Labels -```typescript -// Ensure widgets have proper labels - -``` - -## Performance Considerations - -### Math Rendering Performance -- Initial render can be slow for many expressions -- Use `React.memo` for components with static math -- Consider pagination for content with 50+ expressions - -### Content Loading -- Large exercises should lazy-load hints -- Images should have width/height to prevent reflow -- Preload critical assets - -## Testing Content Rendering - -### Unit Tests -```typescript -it("renders math expressions correctly", () => { - const {container} = render( - - ); - - // MathJax adds specific classes - expect(container.querySelector(".katex")).toBeInTheDocument(); -}); -``` - -### Visual Regression Tests -- Use Storybook for visual testing -- Snapshot complex math layouts -- Test responsive behavior - -### Manual Testing Checklist -- [ ] Math renders correctly -- [ ] Widgets display properly -- [ ] Images load and align correctly -- [ ] Content is responsive on mobile -- [ ] Screen reader announces math properly -- [ ] Print view works correctly - -## Common Content Patterns - -### Exercise with Multiple Parts -```json -{ - "question": { - "content": "Solve the system of equations:\n\n$$\\begin{align}x + y &= 10\\\\2x - y &= 5\\end{align}$$\n\nWhat is $x$? [[☃ numeric-input 1]]\n\nWhat is $y$? [[☃ numeric-input 2]]" - } -} -``` - -### Article with Interactive Elements -```markdown -# Understanding Quadratics - -The standard form of a quadratic equation is $ax^2 + bx + c = 0$. - -Try adjusting the parameters below: - -[[☃ interactive-graph 1]] - -Notice how changing $a$ affects the parabola's shape. -``` - -## Debugging Tips - -### Math Not Rendering -1. Check browser console for MathJax errors -2. Verify TeX syntax is valid -3. Ensure proper escaping in strings -4. Check for conflicting CSS - -### Widget Not Appearing -1. Verify widget ID matches reference -2. Check widget is registered -3. Ensure widget data is valid -4. Look for error boundaries in console - -### Content Layout Issues -1. Inspect element for CSS conflicts -2. Check responsive breakpoints -3. Verify image dimensions -4. Test with different viewport sizes \ No newline at end of file diff --git a/.claude/prompts/testing-best-practices.md b/.claude/prompts/testing-best-practices.md deleted file mode 100644 index 8ae66a4ddf9..00000000000 --- a/.claude/prompts/testing-best-practices.md +++ /dev/null @@ -1,181 +0,0 @@ -# Perseus Testing Best Practices - -## Test Structure - -### Follow the AAA Pattern -```typescript -describe("ComponentName", () => { - it("does something specific", () => { - // Arrange - const props = {...}; - - // Act - const result = doSomething(props); - - // Assert - expect(result).toBe(expected); - }); - - it("handles user interaction", async () => { - // Arrange, Act (when single action) - const {container} = render(); - - // Assert - expect(container).toMatchSnapshot(); - }); -}); -``` - -## Use Test Data Generators - -Located in `packages/perseus-core/src/utils/generators/`: - -```typescript -import {radioGenerator} from "@khanacademy/perseus-core"; - -// Generate complete widget props -const radioProps = radioGenerator.build({ - choices: ["Option A", "Option B"], - hasNoneOfTheAbove: false, -}); - -// Generate multiple variants -const testCases = radioGenerator.buildMany(3); -``` - -## Widget Testing Patterns - -### Modern Widget Pattern (with handleUserInput) -```typescript -import {renderWidget} from "../__tests__/test-utils"; -import {userEvent} from "@testing-library/user-event"; - -it("handles user input correctly", async () => { - // Arrange - const handleUserInput = jest.fn(); - const user = userEvent.setup(); - - renderWidget({ - widgetType: "numeric-input", - widgetProps: numericInputGenerator.build(), - handleUserInput, - }); - - // Act - await user.type(screen.getByRole("textbox"), "42"); - - // Assert - expect(handleUserInput).toHaveBeenCalledWith("42"); -}); -``` - -### Legacy Widget Pattern (with onChange - avoid for new tests) -```typescript -// Only for widgets not yet migrated -const onChange = jest.fn(); -render(); -``` - -## Testing Priorities - -1. **User interactions** - How users actually use the widget -2. **Accessibility** - Keyboard navigation, screen readers -3. **Edge cases** - Empty states, invalid input, boundaries -4. **Scoring logic** - Correct/incorrect answers -5. **Mobile behavior** - Touch interactions - -## Scoring Tests - -Located in `packages/perseus-score/src/widgets/[widget]/`: - -```typescript -import {score} from "../score-radio"; - -it("scores correct answer", () => { - const result = score(userInput, scoringData); - expect(result).toHaveBeenAnsweredCorrectly(); -}); - -it("validates input", () => { - const result = validate(userInput); - expect(result).toHaveInvalidInput("Choose an option"); -}); -``` - -## Snapshot Testing - -Use sparingly, only for: -- Complex rendered output structure -- Markdown/TeX rendering -- Error messages - -```typescript -it("renders complex math correctly", () => { - const {container} = render(); - expect(container).toMatchSnapshot(); -}); -``` - -## Test Utilities - -### Custom Matchers -```typescript -// In test files -expect(result).toHaveBeenAnsweredCorrectly(); -expect(result).toHaveInvalidInput("message"); -``` - -### Testing Library Preferences -- Use `screen` queries over container queries -- Prefer `getByRole` over `getByTestId` -- Use `userEvent` over `fireEvent` -- Wait for async operations with `waitFor` - -## Common Testing Patterns - -### Testing Focus Management -```typescript -it("manages focus correctly", async () => { - const user = userEvent.setup(); - render(); - - await user.tab(); - expect(screen.getByRole("button")).toHaveFocus(); -}); -``` - -### Testing Accessibility -```typescript -it("has accessible labels", () => { - render(); - expect(screen.getByLabelText("Answer")).toBeInTheDocument(); -}); -``` - -### Testing Mobile Interactions -```typescript -it("handles touch events", async () => { - const user = userEvent.setup(); - render(); - - await user.pointer({keys: "[TouchA>]", target: element}); - // Assert touch-specific behavior -}); -``` - -## What NOT to Test - -- Implementation details (internal state, private methods) -- Third-party library behavior -- Static types (TypeScript handles this) -- Styles and CSS (unless functionally important) - -## File Organization - -``` -widget-name/ -├── widget-name.tsx -├── widget-name.test.ts # Component tests -├── widget-name.testdata.ts # Test data using generators -└── score-widget-name.test.ts # Scoring tests (if applicable) -``` \ No newline at end of file diff --git a/.claude/prompts/widget-development.md b/.claude/prompts/widget-development.md deleted file mode 100644 index 4557afdadfe..00000000000 --- a/.claude/prompts/widget-development.md +++ /dev/null @@ -1,286 +0,0 @@ -# Widget Development Guide - -## Three-Layer Type Architecture - -Perseus widgets use a three-layer type system: - -1. **WidgetOptions** - Widget configuration/display settings (in data-schema.ts) -2. **ValidationData** - Client-side validation data, no sensitive info (in validation.types.ts) -3. **Rubric** - Full scoring data: `ValidationData & WidgetOptions` (in validation.types.ts) - -## Creating a New Widget - -### 1. Directory Structure -``` -packages/perseus/src/widgets/[widget-name]/ -├── index.ts # Exports -├── [widget-name].tsx # Main component -├── [widget-name].test.ts # Tests -├── [widget-name].testdata.ts # Test data using generators -└── __docs__/ - ├── [widget-name].stories.tsx # Storybook stories - └── a11y.mdx # Accessibility docs -``` - -### 2. Modern Widget Implementation - -```typescript -// [widget-name].tsx -import React from "react"; -import type {PerseusWidgetNameUserInput} from "@khanacademy/perseus-core"; - -// Props are just the widget options spread directly, plus system props -type Props = { - // Widget-specific options (from WidgetOptions) - correctAnswer?: string; - placeholder?: string; - // System props - userInput: PerseusWidgetNameUserInput; - handleUserInput: (input: PerseusWidgetNameUserInput) => void; - trackInteraction: () => void; - onFocus: () => void; - onBlur: () => void; -}; - -function WidgetName(props: Props) { - const {userInput, handleUserInput} = props; - - const handleChange = (newValue: string) => { - handleUserInput({value: newValue}); - props.trackInteraction(); - }; - - return ( -
- handleChange(e.target.value)} - onFocus={props.onFocus} - onBlur={props.onBlur} - placeholder={props.placeholder} - /> -
- ); -} - -// IMPORTANT: Set displayName for debugging -WidgetName.displayName = "WidgetName"; - -// Export with WidgetExports interface -export default { - name: "widget-name" as const, - displayName: "Widget Display Name", - widget: WidgetName, - isLintable: true, - - // Optional scoring helpers (Rubric only used here, not in props!) - getOneCorrectAnswerFromRubric: (rubric: PerseusWidgetNameRubric) => { - return rubric.correctAnswer || null; - }, -} as WidgetExports; -``` - -### 3. Define Types in perseus-core - -```typescript -// packages/perseus-core/src/data-schema.ts - -// Widget configuration (what content creators set) -export type PerseusWidgetNameOptions = { - correctAnswer?: string; - placeholder?: string; - static?: boolean; // Common option for most widgets -}; - -// User's input -export type PerseusWidgetNameUserInput = { - value: string; -}; - -// packages/perseus-core/src/validation.types.ts - -// Client-side validation data (subset of options, no answers) -export type PerseusWidgetNameValidationData = { - placeholder?: string; -}; - -// Full scoring data (intersection type) -export type PerseusWidgetNameRubric = - PerseusWidgetNameValidationData & - PerseusWidgetNameOptions; -``` - -### 4. Register the Widget - -```typescript -// packages/perseus/src/widgets.ts -import widgetNameWidget from "./widgets/widget-name"; - -export const widgets = { - // ... existing widgets - "widget-name": widgetNameWidget, -}; -``` - -### 5. Add Scoring Function (if scorable) - -```typescript -// packages/perseus-score/src/widgets/widget-name/score-widget-name.ts -import type { - PerseusWidgetNameUserInput, - PerseusWidgetNameRubric, -} from "@khanacademy/perseus-core"; - -export function scoreWidgetName( - userInput: PerseusWidgetNameUserInput, - rubric: PerseusWidgetNameRubric, -): PerseusScore { - if (userInput.value === rubric.correctAnswer) { - return { - type: "points", - earned: 1, - total: 1, - message: null, - }; - } - return { - type: "points", - earned: 0, - total: 1, - message: null, - }; -} - -// Register in widget-registry.ts -import {scoreWidgetName} from "./widget-name/score-widget-name"; - -widgets["widget-name"] = { - score: scoreWidgetName, -}; -``` - -## Important: Props Pattern - -**What widgets receive as props:** -- Widget options spread directly (NOT wrapped in `options` or `rubric`) -- `userInput` - Current user state -- `handleUserInput` - State update callback -- System props (`trackInteraction`, `onFocus`, `onBlur`, etc.) - -**What widgets DON'T receive:** -- `rubric` prop (only used in scoring functions) -- `scoringData` or `validationData` props -- `onChange` prop (deprecated) - -## Widget State Management - -### Modern Pattern (Required for new widgets) -```typescript -// Receive state via userInput -const currentValue = props.userInput?.value || ""; - -// Update state via handleUserInput -const handleChange = (newValue: string) => { - props.handleUserInput({value: newValue}); - props.trackInteraction(); // Track user interaction -}; -``` - -### NO Local State for Answers -- User answers must be stored in UserInputManager -- Local React state only for UI concerns (hover, focus, etc.) - -## Testing Your Widget - -### Use Test Generators -```typescript -// [widget-name].testdata.ts -import {widgetNameGenerator} from "@khanacademy/perseus-core"; - -export const question1 = widgetNameGenerator.build({ - correctAnswer: "42", - placeholder: "Enter your answer", -}); -``` - -### Test Pattern -```typescript -import {renderQuestion} from "../__tests__/renderQuestion"; -import {question1} from "./widget-name.testdata"; - -describe("WidgetName", () => { - it("accepts user input", async () => { - // Arrange - const {renderer} = renderQuestion(question1); - - // Act - await renderer.setInputValue({ - widgetId: "widget-name 1", - value: "42", - }); - - // Assert - const score = renderer.score(); - expect(score).toHaveBeenAnsweredCorrectly(); - }); -}); -``` - -## Common Widget Patterns - -### Multiple Inputs -```typescript -type UserInput = { - numerator: string; - denominator: string; -}; - -const handleNumeratorChange = (value: string) => { - handleUserInput({ - ...userInput, - numerator: value, - }); -}; -``` - -### Validation (Optional) -```typescript -// packages/perseus-score/src/widgets/widget-name/validate-widget-name.ts -export function validateWidgetName( - userInput: PerseusWidgetNameUserInput, -): ValidationResult { - if (!userInput.value) { - return { - type: "invalid", - message: "Please enter an answer", - }; - } - return {type: "valid"}; -} -``` - -### Focus Management -```typescript -const inputRef = React.useRef(null); - -React.useImperativeHandle(props.widgetRef, () => ({ - focus: () => inputRef.current?.focus(), - blur: () => inputRef.current?.blur(), -})); -``` - -## Migration Notes - -- Radio widget is transitioning (TODO(LEMS-2994)) -- Numeric-input is fully modernized (good reference) -- Expression uses class component pattern (older style) -- New widgets should follow functional component pattern - -## Common Pitfalls - -1. **Don't expect rubric in props** - It's only for scoring -2. **Don't use onChange** - Use handleUserInput -3. **Don't forget displayName** - Required for debugging -4. **Don't store answers in local state** - Use UserInputManager -5. **Don't skip trackInteraction** - Needed for analytics -6. **Don't use cross-package relative imports** - Use @khanacademy/* aliases \ No newline at end of file diff --git a/.claude/widget-patterns-research.md b/.claude/widget-patterns-research.md deleted file mode 100644 index 755029ce76c..00000000000 --- a/.claude/widget-patterns-research.md +++ /dev/null @@ -1,551 +0,0 @@ -# Perseus Widget Development Patterns Research - -**Research Date:** February 5, 2026 -**Conducted by:** Claude Code -**Co-Author:** Tamara Bozich - -## Executive Summary - -This document details the current widget development patterns in Perseus based on examination of actual codebase implementations. The key finding is that **Rubric is still the primary pattern**, not replaced by `scoringData` or `validationData` props. Widget types are split between widget configuration (in `PerseusWidgetOptions` from data-schema) and validation/scoring data (in validation.types.ts). - ---- - -## 1. Widget Type Definitions Architecture - -### 1.1 The Type System (Three-Layer Pattern) - -Perseus uses a three-layer type system for widget data, defined in `packages/perseus-core/src/validation.types.ts`: - -1. **WidgetOptions** (in `data-schema.ts`): The configuration/definition of the widget - - What a content creator sees in the editor - - Immutable across a session - - Example: `PerseusRadioWidgetOptions`, `PerseusNumericInputWidgetOptions` - -2. **ValidationData** (in `validation.types.ts`): The data needed for client-side validation - - Does NOT contain sensitive scoring information - - Can be checked before submission - - Used for accessibility, input validation - - Often overlaps with WidgetOptions - -3. **Rubric** (in `validation.types.ts`): The data needed for scoring - - Contains the "correct" answers and scoring logic parameters - - Always intersected with `ValidationData` via `&` operator - - Passed to scoring functions - - Example: `PerseusNumericInputRubric = { answers, coefficient } & PerseusNumericInputValidationData` - -### 1.2 Key Finding: Rubric is NOT Being Replaced - -**Status:** Rubric pattern is active and unchanged -- `PerseusNumericInputRubric` (line 223 in validation.types.ts): `{ answers, coefficient }` -- `PerseusRadioRubric` (line 261 in validation.types.ts): `{ choices, countChoices? }` -- Both are used as-is in widget implementations and scoring functions - -**No evidence of transition to:** -- `scoringData` prop -- `validationData` prop -- Alternative naming schemes - ---- - -## 2. Widget Implementation Patterns - -### 2.1 Current Widget Export Pattern - -All widgets follow the `WidgetExports` pattern (defined in `packages/perseus/src/types.ts` lines 468-512): - -```typescript -export type WidgetExports< - T extends React.ComponentType & Widget = React.ComponentType, - TUserInput = Empty, -> = Readonly<{ - name: string; // Widget identifier - displayName: string; // Display name in editor - widget: T; // The component - version?: Version; // Widget schema version - isLintable?: boolean; // Can be linted - tracking?: Tracking; // Tracking behavior - - // Scoring/Validation functions (OPTIONAL) - getOneCorrectAnswerFromRubric?: (rubric: WidgetOptions) => string | null; - getCorrectUserInput?: (widgetOptions: WidgetOptions) => TUserInput; - getStartUserInput?: (widgetOptions: WidgetOptions, problemNum: number) => TUserInput; - getUserInputFromSerializedState?: (serializedState: unknown, widgetOptions?: WidgetOptions) => TUserInput; -}>; -``` - -### 2.2 Numeric Input Widget (Modern Pattern) - -**File:** `packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx` - -```typescript -// Props type - combines widget options with universal props -type ExternalProps = WidgetProps< - PerseusNumericInputWidgetOptions, - PerseusNumericInputUserInput ->; - -// Widget component definition -export class NumericInput extends React.Component implements Widget { - // Widget interface methods - focus: () => boolean - focusInputPath: () => void - blurInputPath: () => void - getInputPaths: () => ReadonlyArray> - getPromptJSON(): NumericInputPromptJSON - // ... -} - -// Export follows WidgetExports pattern -export default { - name: "numeric-input", - displayName: "Numeric input", - widget: NumericInput, - isLintable: true, - getCorrectUserInput, // Function - getOneCorrectAnswerFromRubric(rubric: PerseusNumericInputRubric) { /* ... */ }, - getStartUserInput, // Function - getUserInputFromSerializedState, // Function -} satisfies WidgetExports; -``` - -**Actual Prop Names Used:** -- `answers: PerseusNumericInputAnswer[]` - from widget options -- `coefficient: boolean` - from widget options -- `userInput: { currentValue: string }` - user input (typed separately) -- `handleUserInput` - callback to update user input -- `trackInteraction` - analytics callback -- No `rubric`, `scoringData`, or `validationData` props passed directly to component - -### 2.3 Radio Widget (In Transition) - -**File:** `packages/perseus/src/widgets/radio/radio.ff.tsx` + `radio.ts` - -```typescript -type Props = WidgetProps; - -export default { - name: "radio", - displayName: "Radio / Multiple choice", - widget: Radio, - getStartUserInput, - version: radioLogic.version, - isLintable: true, - getUserInputFromSerializedState: (serializedState: unknown) => { - return getUserInputFromSerializedState(serializedState); - }, -} satisfies WidgetExports; -``` - -**Status:** -- Still using class component wrapper (line 52: `class Radio extends React.Component implements Widget`) -- Transitioning toward functional component (comment on line 50: "TODO(LEMS-2994): Clean up this file") -- Using old API with `ChoiceState` that combines UI state and user input - -### 2.4 Expression Widget (Functional) - -**File:** `packages/perseus/src/widgets/expression/expression.tsx` - -```typescript -type ExternalProps = WidgetProps< - PerseusExpressionWidgetOptions, - PerseusExpressionUserInput ->; - -type Props = ExternalProps & { - buttonSets: NonNullable; - functions: NonNullable; - times: NonNullable; -}; - -export class Expression extends React.Component implements Widget { - // Widget methods - focus: () => boolean - focusInputPath: () => void - // ... -} - -export default { - name: "expression", - displayName: "Expression / Equation", - widget: Expression, - version: expressionLogic.version, - isLintable: true, - getOneCorrectAnswerFromRubric, - getStartUserInput, - getCorrectUserInput, - getUserInputFromSerializedState, -} satisfies WidgetExports; -``` - ---- - -## 3. Props Flow Architecture - -### 3.1 How Props Are Passed to Widgets - -From `packages/perseus/src/renderer.tsx` (lines 511-575): - -```typescript -getWidgetProps(widgetId: string): WidgetProps { - const widgetProps = this.props.widgets[widgetId].options; // WidgetOptions - const widgetInfo = this.state.widgetInfo[widgetId]; // Metadata - - return { - ...widgetProps, // Spread widget options directly - userInput: this.props.userInput?.[widgetId], - widgetId: widgetId, - widgetIndex: this._getWidgetIndexById(widgetId), - alignment: widgetInfo && widgetInfo.alignment, - static: widgetInfo?.static, - apiOptions: this.getApiOptions(), // API options - onFocus: _.partial(this._onWidgetFocus, widgetId), - onBlur: _.partial(this._onWidgetBlur, widgetId), - handleUserInput: (newUserInput) => { /* ... */ }, - trackInteraction: interactionTracker.track, - }; -} -``` - -**Key Pattern:** -- Widget options are spread directly: `...widgetProps` -- UserInput is passed separately: `userInput` prop -- No `rubric` prop passed to widget component -- Rubric is only used during scoring (server-side or via scoring functions) - -### 3.2 UserInput vs WidgetOptions - -**UserInput Examples:** -```typescript -// Numeric Input -PerseusNumericInputUserInput = { currentValue: string } - -// Radio -PerseusRadioUserInput = { selectedChoiceIds: string[] } - -// Expression -PerseusExpressionUserInput = string - -// Label Image -PerseusLabelImageUserInput = { - markers: Array<{ selected?: string[], label: string }> -} -``` - -**WidgetOptions Examples:** -```typescript -// Numeric Input -PerseusNumericInputWidgetOptions = { - answers: PerseusNumericInputAnswer[], - labelText?: string, - size: string, - coefficient: boolean, - rightAlign?: boolean, - static: boolean, -} - -// Radio -PerseusRadioWidgetOptions = { - choices: PerseusRadioChoice[], - hasNoneOfTheAbove?: boolean, - countChoices?: boolean, - numCorrect?: number, - randomize?: boolean, - multipleSelect?: boolean, - deselectEnabled?: boolean, -} -``` - ---- - -## 4. Rubric Pattern in Scoring - -### 4.1 Rubric Types (validation.types.ts) - -Rubric types are ONLY used in: -1. Scoring functions (registered in perseus-score) -2. `getOneCorrectAnswerFromRubric` export in WidgetExports -3. Server-side validation/scoring - -**Examples:** - -```typescript -// Line 223-228 of validation.types.ts -export type PerseusNumericInputRubric = { - answers: PerseusNumericInputAnswer[]; - coefficient: boolean; -}; - -// Line 261-266 -export type PerseusRadioRubric = { - choices: PerseusRadioChoice[]; - countChoices?: boolean; -}; - -// Line 114-117 -export type PerseusExpressionRubric = { - answerForms: Array; - functions: string[]; -}; -``` - -### 4.2 Scoring Function Pattern - -From numeric-input implementation: - -```typescript -function getCorrectUserInput( - options: PerseusNumericInputWidgetOptions, -): PerseusNumericInputUserInput { - for (const answer of options.answers) { - if (answer.status === "correct" && answer.value != null) { - // Logic to find correct answer - if (answer.answerForms?.includes("decimal")) { - return {currentValue: answer.value.toString()}; - } - // ... more logic - } - } - return {currentValue: ""}; -} - -// Usage in WidgetExports: -getOneCorrectAnswerFromRubric( - rubric: PerseusNumericInputRubric, -): string | null | undefined { - const correctAnswers = rubric.answers.filter( - (answer) => answer.status === "correct" - ); - // ... logic to format for display -} -``` - ---- - -## 5. Type Relationships - -### 5.1 Data-Schema Hierarchy (PerseusWidgetTypes) - -``` -PerseusWidgetTypes (Interface in data-schema.ts) -├── categorizer: CategorizerWidget -├── dropdown: DropdownWidget -├── numeric-input: NumericInputWidget -├── radio: RadioWidget -├── expression: ExpressionWidget -└── ... (33+ widgets) - -Each entry like: -NumericInputWidget = WidgetOptions<'numeric-input', PerseusNumericInputWidgetOptions> -``` - -### 5.2 Validation Type Hierarchy (validation.types.ts) - -``` -RubricRegistry (Interface) -├── categorizer: PerseusCategorizerRubric -├── radio: PerseusRadioRubric -├── numeric-input: PerseusNumericInputRubric -└── ... (corresponds to PerseusWidgetTypes) - -UserInputRegistry (Interface) -├── categorizer: PerseusCategorizerUserInput -├── radio: PerseusRadioUserInput -└── ... -``` - -### 5.3 Type Parameter Naming in Widgets - -```typescript -// Standard pattern -type ExternalProps = WidgetProps< - PerseusNumericInputWidgetOptions, // TWidgetOptions - PerseusNumericInputUserInput // TUserInput ->; - -// WidgetProps definition (types.ts line 527-533): -export type WidgetProps< - TWidgetOptions, - TUserInput = Empty, - TrackingExtraArgs = Empty, -> = TWidgetOptions & UniversalWidgetProps; -``` - ---- - -## 6. Recent Widget Examples & Practices - -### 6.1 Numeric Input Widget - Fully Modernized ✓ - -**Status:** Class component with functional component integration (hybrid) -- Component: `NumericInputComponent` (functional, lines 28-132) -- Wrapper: `NumericInput` class (implements Widget interface) -- Props: Proper typing with `ExternalProps` and `NumericInputProps` -- Features: Focus management, accessibility (labelText, ariaLabel) - -### 6.2 Radio Widget - In Transition - -**Status:** Class wrapper around multiple-choice-widget -- Uses old `ChoiceState` API (combines UI state + user input) -- Comment: "TODO(LEMS-2994): Clean up this file" -- Issue: Hard to migrate due to component inheritance - -### 6.3 Expression Widget - Modern Pattern - -**Status:** Class component implementing Widget interface -- Proper prop typing -- Focus management via refs -- Comprehensive widget interface implementation - ---- - -## 7. Named Conventions Summary - -### 7.1 Type Naming - -| Pattern | Location | Purpose | -|---------|----------|---------| -| `PerseusWidgetOptions` | data-schema.ts | Widget configuration | -| `PerseusRubric` | validation.types.ts | Scoring data | -| `PerseusUserInput` | validation.types.ts | User's answer | -| `PerseusValidationData` | validation.types.ts | Client-side validation data | - -### 7.2 Prop Naming (Actual Usage) - -**What IS passed to widget components:** -- Widget option fields (spread from `widgetProps`) -- `userInput: TUserInput` -- `handleUserInput: (newUserInput: TUserInput) => void` -- `widgetId: string` -- `apiOptions: APIOptionsWithDefaults` -- `trackInteraction: () => void` -- `onFocus`, `onBlur`, `linterContext`, etc. - -**What IS NOT passed:** -- No `rubric` prop (used only in scoring) -- No `scoringData` prop -- No `validationData` prop -- No separate validation/scoring objects - -### 7.3 Focus Management Requirements - -Every widget must implement: -```typescript -focus?: () => { id: string; path: FocusPath } | boolean; -focusInputPath?: (path: FocusPath) => void; -blurInputPath?: (path: FocusPath) => void; -getInputPaths?: () => ReadonlyArray; -``` - ---- - -## 8. Widget Registration & Discovery - -### 8.1 Registration System (widgets.ts) - -```typescript -const widgets = new Registry("Perseus widget registry"); - -export const registerWidget = (type: string, widget: WidgetExports) => { - widgets.set(type, widget); -}; - -// All widgets register via: -// registerWidgets([numericInput, radio, expression, /* ... */]) -``` - -### 8.2 PerseusWidgetTypes Interface - -```typescript -export interface PerseusWidgetTypes { - categorizer: CategorizerWidget; - dropdown: DropdownWidget; - radio: RadioWidget; - "numeric-input": NumericInputWidget; - // ... 30+ more widgets -} -``` - -This interface: -- Is open for extension (can be module-augmented) -- Used by `MakeWidgetMap` to create `PerseusWidgetsMap` -- Provides strong typing for widget data throughout Perseus - ---- - -## 9. Current State Assessment - -### What Works Well -1. **Type Safety:** Three-layer system (WidgetOptions → ValidationData → Rubric) is robust -2. **Props Architecture:** Clear separation between widget config and user input -3. **Widget Interface:** Consistent interface for all widgets via `Widget` type -4. **Registration:** Flexible registry system supports custom widgets - -### Transition Areas -1. **Radio Widget:** Still transitioning from old `ChoiceState` API -2. **Class Components:** Some widgets still using class components (being converted to functional) -3. **Serialized State:** Deprecated API still present but marked `@deprecated` - -### No Evidence Of -1. Planned move to `scoringData` or `validationData` props -2. Changes to Rubric pattern -3. Alternative type naming schemes - ---- - -## 10. Key Findings for Implementation - -### When Creating a New Widget: - -1. **Define types in data-schema.ts:** - ```typescript - type MyNewWidget = WidgetOptions<'my-new-widget', MyNewWidgetOptions>; - // Add to PerseusWidgetTypes interface - ``` - -2. **Define scoring types in validation.types.ts:** - ```typescript - type PerseuMyNewWidgetRubric = { /* scoring data */ } & PerseuMyNewWidgetValidationData; - type PerseuMyNewWidgetUserInput = { /* user data */ }; - ``` - -3. **Create widget component with WidgetProps typing:** - ```typescript - type Props = WidgetProps; - ``` - -4. **Export via WidgetExports pattern:** - ```typescript - export default { - name: "my-new-widget", - displayName: "My New Widget", - widget: MyNewWidget, - isLintable: true, - getStartUserInput, - getCorrectUserInput, - // ... scoring functions - } satisfies WidgetExports; - ``` - -5. **Implement Widget interface methods:** - - `focus(): boolean` - - `focusInputPath(path: FocusPath): void` - - `blurInputPath(path: FocusPath): void` - - `getInputPaths(): ReadonlyArray` - - `getPromptJSON?(): WidgetPromptJSON` - ---- - -## Files Examined - -- `/packages/perseus-core/src/data-schema.ts` - Widget type definitions -- `/packages/perseus-core/src/validation.types.ts` - Scoring/validation types -- `/packages/perseus/src/types.ts` - Widget interface definitions -- `/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx` - Example widget -- `/packages/perseus/src/widgets/radio/radio.ff.tsx` - Example widget (in transition) -- `/packages/perseus/src/widgets/expression/expression.tsx` - Example widget -- `/packages/perseus/src/renderer.tsx` - Props passing mechanism -- `/packages/perseus/src/widgets.ts` - Widget registration - ---- - -**End of Research Document** diff --git a/CLAUDE.md b/CLAUDE.md index 132d578894b..f6c5da9c7e9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,42 +1,24 @@ # Perseus Development Guide for AI Assistants -This document provides essential context for AI assistants working on the Perseus codebase. Detailed documentation for specific topics is available in `.claude/prompts/`. +This document provides essential information for AI assistants working on the Perseus codebase. ## Project Overview -Perseus is Khan Academy's educational content rendering system that powers all exercises and articles. It's a TypeScript monorepo that extends Markdown with interactive widgets and beautiful math rendering. +Perseus is Khan Academy's educational content rendering system that powers all exercises and articles. It's a TypeScript +monorepo that extends Markdown with interactive widgets and beautiful math rendering. -**Core Components:** +**Core Architecture:** - **Renderers**: Display content (ServerItemRenderer for exercises, ArticleRenderer for articles) - **Widgets**: Interactive components (radio, numeric-input, interactive-graph, etc.) -- **State Management**: Centralized via UserInputManager with `handleUserInput` pattern +- **Editors**: Authoring interfaces for content creators - **Math**: TeX expressions rendered via MathJax -## Prompt Library Reference - -Comprehensive documentation is organized by topic in `.claude/prompts/`: - -### Architecture & Organization -- **[codebase-map.md](.claude/prompts/codebase-map.md)** - Package structure, data flow, module boundaries -- **[file-organization.md](.claude/prompts/file-organization.md)** - Directory structure, naming conventions, imports - -### Development Guides -- **[widget-development.md](.claude/prompts/widget-development.md)** - Creating and maintaining widgets -- **[component-best-practices.md](.claude/prompts/component-best-practices.md)** - React components, Wonder Blocks, accessibility -- **[testing-best-practices.md](.claude/prompts/testing-best-practices.md)** - Testing patterns and utilities -- **[math-and-content.md](.claude/prompts/math-and-content.md)** - Math rendering, content structure, MathJax - -### Workflow & Process -- **[iteration-and-feedback.md](.claude/prompts/iteration-and-feedback.md)** - When to ask for help, debugging, deployment checklist - ## Quick Start Commands ### Development ```bash pnpm storybook # Launch Storybook documentation pnpm test # Run tests -pnpm build:types # Build TypeScript types -pnpm build # Build all packages ``` ### Code Quality @@ -48,68 +30,176 @@ pnpm prettier . --write # Auto-format code pnpm tsc # Type-check all packages ``` -### Testing +### Testing Specific Packages ```bash pnpm --filter perseus test # Test main perseus package pnpm --filter perseus-editor test # Test editor package pnpm test packages/perseus/src/widgets/radio # Test specific widget -pnpm test -u # Update snapshots -pnpm test --coverage # Run with coverage ``` -## Key Architectural Decisions +## Key Directories + +``` +packages/ +├── perseus/ # Main package (renderers, widgets, components) +│ ├── src/__docs__/ # Main Storybook stories +│ ├── src/widgets/ # Widget implementations +│ └── src/components/ # Reusable components +├── perseus-editor/ # Editor UI components +├── math-input/ # Math keypad and input components +├── perseus-core/ # Shared types and utilities +├── perseus-linter/ # Content validation tools +└── perseus-score/ # Server-side scoring functions +``` + +## Common Development Patterns + +### Creating a New Widget +1. **Create widget directory**: `packages/perseus/src/widgets/[widget-name]/` +2. **Implement widget files**: + - `[widget-name].tsx` - Main component + - `[widget-name].test.ts` - Tests + - `index.ts` - Exports + - `__docs__/[widget-name].stories.tsx` - Storybook story + - `__docs__/a11y.mdx` - Accessibility documentation +3. **Register widget** in `packages/perseus/src/widgets.ts` +4. **If scorable, add scoring functions** in `packages/perseus-score/src/widgets/[widget-name]/`: + - `score-[widget-name].ts` - Scoring logic + - `score-[widget-name].test.ts` - Scoring tests + - `validate-[widget-name].ts` - Input validation (optional) + - `validate-[widget-name].test.ts` - Validation tests (optional) +5. **Register scoring** in `packages/perseus-score/src/widgets/widget-registry.ts` +6. **Add types** to `packages/perseus-core/src/data-schema.ts` + +### Widget Implementation Pattern +```typescript +// Export interface following WidgetExports pattern +export default { + name: "widget-name", + displayName: "Widget Display Name", + widget: WidgetComponent, + isLintable: true, // For use by the editor +} as WidgetExports; +``` -### State Management Evolution -- **Legacy**: Widgets used `onChange` callbacks with local state -- **Current**: Transitioning to `handleUserInput` with centralized UserInputManager -- **Migration Status**: Radio widget bridging old/new patterns (LEMS-2994) -- **Example**: NumericInput fully modernized, use as reference +### Focus Management +All widgets must implement proper focus management for accessibility. -### Type System -- **Three-layer architecture**: - 1. `WidgetOptions` - Configuration from content creators - 2. `ValidationData` - Client-side validation (no answers) - 3. `Rubric` - Full scoring data (`ValidationData & WidgetOptions`) -- **Important**: Rubric never passed as prop to widgets, only used in scoring +## Package Dependencies -### Import Rules -- Use package aliases: `@khanacademy/perseus`, etc. -- NO file extensions in imports +### Import Guidelines +- Use package aliases: `@khanacademy/perseus`, `@khanacademy/perseus-editor` +- NO file extensions in imports (`.ts`, `.tsx` banned by ESLint) - NO cross-package relative imports - Import order: builtin > external > internal > relative > types -## Before Submitting Code - -### Pre-commit Checklist -1. `pnpm test` - All tests pass -2. `pnpm tsc` - No type errors -3. `pnpm lint --fix` - No linting issues -4. `pnpm prettier . --write` - Code formatted -5. `pnpm build` - Build succeeds -6. `pnpm storybook` - Components render correctly -7. Accessibility documented (for new widgets) -8. Mobile tested (if applicable) -9. Console clear of debug statements - -### Common Issues to Check -- Console statements left in code -- Unused imports -- Missing displayName on components -- onChange used instead of handleUserInput -- Local state for user answers (should use UserInputManager) -- Unescaped backslashes in TeX strings - -## Migration & Deprecation - -### Deprecated Patterns -- `onChange` prop - Use `handleUserInput` instead (LEMS-3245, LEMS-3542) -- Local widget state for answers - Use UserInputManager -- Class components for new widgets - Use functional components - -### In Transition -- Radio widget - Bridging old/new patterns -- Some widgets still using legacy patterns -- Gradual migration to centralized state +### Example Correct Imports +```typescript +import React from "react"; // external +import {ApiOptions} from "@khanacademy/perseus"; // internal package +import {WidgetContainer} from "../widget-container"; // relative + +import type {WidgetProps} from "@khanacademy/perseus-core"; +``` + +## Testing Guidelines + +### Test Structure +1. Follow the AAA pattern: Arrange, Act, Assert +1.1 If Arrange and Act are one action, combine them to `//Arrange, Act` +2. Use widget generators to build test data and test data options. + You can find generators for all widgets in packages/perseus-core/src/utils/generators. + An example usage can be seen here: packages/perseus/src/widgets/expression/expression.testdata.ts. +3. Follow the test structure below: +```typescript +import {render, screen} from "@testing-library/react"; +import {userEvent} from "@testing-library/user-event"; + +import {question1} from "../__testdata__/widget.testdata"; +import WidgetComponent from "../widget-component"; + +describe("WidgetComponent", () => { + it("renders correctly", () => { + render(); + expect(screen.getByRole("button")).toBeInTheDocument(); + }); + + it("handles user interaction", async () => { + const user = userEvent.setup(); + const onChange = jest.fn(); + + render(); + await user.click(screen.getByRole("button")); + + expect(onChange).toHaveBeenCalled(); + }); +}); +``` + +### Writing Tests +- use `it` for individual test cases and not `test` +- use `describe` to group related tests + +## Common Issues & Solutions + +### Math Rendering (TeX) +- Use `$...$` for inline math, `$$...$$` for display math +- For complex expressions, use `\dfrac` instead of `\frac` +- Test math rendering in different contexts (articles, exercises, hints) + +### Widget State Management +- Use `useState` for local component state +- Props flow down from parent renderer +- Call `onChange` to notify parent of state changes +- Implement proper serialization for persistent state + +### Mobile Considerations +- All widgets must work on mobile devices +- Support touch interactions +- Consider on-screen keypad for math inputs +- Test with different screen sizes using Storybook + +### Performance Optimization +- Use `React.memo()` for expensive components +- Implement `useMemo()` for complex calculations +- Avoid unnecessary re-renders in widget hierarchies +- Profile performance with React DevTools + +## Debugging Tips + +### Storybook Development +- Use Storybook for isolated component development +- Test different props combinations +- Verify accessibility with Storybook a11y addon +- Check mobile layouts with device frame addon + +### Console Debugging +```typescript +// Temporary debugging (remove before commit) +console.log("Widget state:", this.state); +console.log("Props received:", this.props); +``` + +### Error Boundaries +- Widgets are wrapped in error boundaries +- Check browser console for widget-specific errors +- Implement graceful fallbacks for failed widgets + +## Deployment Notes + +### Before Submitting PR +1. Run full test suite: `pnpm test` +2. Check types: `pnpm build:types` +3. Lint and format: `pnpm lint --fix && pnpm prettier . --write` +4. Test in Storybook: `pnpm storybook` +5. Verify accessibility compliance + +### Common Pre-commit Failures +- ESLint errors (unused imports, console statements) +- Prettier formatting (spacing, quotes, semicolons) +- TypeScript type errors +- Missing accessibility documentation +- Test failures ## Additional Resources @@ -118,9 +208,7 @@ pnpm test --coverage # Run with coverage - **Widget Gallery**: Browse existing widgets in Storybook for patterns - **Accessibility Guidelines**: Each widget should have `a11y.mdx` documentation - **Khan Academy Design System**: Wonder Blocks components for consistent UI -- **Test Generators**: `@khanacademy/perseus-core/utils/generators` -- **Type Definitions**: `packages/perseus-core/src/data-schema.ts` --- -*This document provides an overview and quick reference. For detailed information on any topic, refer to the appropriate document in `.claude/prompts/`. For human developers, see the main README.md files in each package.* \ No newline at end of file +*This document is maintained for AI assistants. For human developers, see the main README.md files in each package.* From fab8f00ab989fed0a106e80ec7e2785bc4e20f75 Mon Sep 17 00:00:00 2001 From: Tamara Date: Wed, 11 Feb 2026 15:25:30 -0500 Subject: [PATCH 09/12] [tb/LEMS-3710/toggle-for-shuffle] Remove Claude planning docs from final version --- .claude/implementation-plan-LEMS-3710.md | 101 ---------- .claude/research-LEMS-3710.md | 134 ------------- ...earch-editor-preview-dataflow-LEMS-3710.md | 181 ------------------ .claude/ticket-summary-LEMS-3710.md | 25 --- 4 files changed, 441 deletions(-) delete mode 100644 .claude/implementation-plan-LEMS-3710.md delete mode 100644 .claude/research-LEMS-3710.md delete mode 100644 .claude/research-editor-preview-dataflow-LEMS-3710.md delete mode 100644 .claude/ticket-summary-LEMS-3710.md diff --git a/.claude/implementation-plan-LEMS-3710.md b/.claude/implementation-plan-LEMS-3710.md deleted file mode 100644 index 46ab6385e74..00000000000 --- a/.claude/implementation-plan-LEMS-3710.md +++ /dev/null @@ -1,101 +0,0 @@ -# LEMS-3710: Add Toggle for Showing Preview Unshuffled - -## Context - -Content creators using the Radio widget find the Edit tab sidebar preview unhelpful when choices are shuffled — it's hard to match shuffled preview choices back to the editor fields. They want the sidebar preview to default to unshuffled, with an option to see the shuffled view when needed. The Preview tab should always respect the `randomize` setting. - -## Approach - -### Core Idea - -Add an ephemeral "Shuffle preview" toggle to `RadioEditor` that controls whether the Edit tab sidebar preview shuffles choices. The toggle state is communicated to the preview via a marker field in `serialize()` output, which `EditorPage.updateRenderer()` processes before sending data to the iframe. - -### Why This Approach - -- `serialize()` is the only channel between `RadioEditor` and the preview iframe -- `serialize()` is also used for saving — so we use a marker field that gets stripped on the save path -- No changes to core shuffle logic (`choiceTransform`, `radio.ff.tsx`, etc.) -- Fully editor-only change - -## Implementation Steps - -### Step 1: Add toggle UI and state to RadioEditor - -**File:** `packages/perseus-editor/src/widgets/radio/editor.tsx` - -- Add component state: `state = { showShuffledPreview: false }` -- Add a "Shuffle preview" `LabeledSwitch` immediately after the "Randomize order" switch - - Disabled when `!this.props.randomize` or `isEditingDisabled` - - Checked: `this.state.showShuffledPreview` - - When toggled off or when randomize is turned off, reset to `false` -- Add `InfoTip` next to "Randomize order" explaining: "The editor preview shows choices unshuffled by default. Use 'Shuffle preview' to see the randomized order. The Preview tab always shows the randomized order when enabled." -- Follow the `LabeledSwitch + InfoTip` pattern from `decorative-toggle.tsx` (flex row with gap) - -### Step 2: Add marker field to serialize() - -**File:** `packages/perseus-editor/src/widgets/radio/editor.tsx` - -- In `serialize()`, when `this.props.randomize && this.state.showShuffledPreview`, include `_showShuffledPreview: true` in the return value -- The real `randomize` field always reflects the true prop value (unchanged) -- Widen the return type to allow the extra field - -### Step 3: Process marker in EditorPage.updateRenderer() - -**File:** `packages/perseus-editor/src/editor-page.tsx` - -- After `this.serialize()` on line 225, post-process the item: - - Walk `item.question.widgets` entries - - For any widget where `type === "radio"`: - - If `options._showShuffledPreview` is truthy → leave `randomize` as-is, delete the marker - - Otherwise → set `options.randomize = false` (default: unshuffled in editor preview) -- This ensures the Edit tab preview defaults to unshuffled unless the creator explicitly enables shuffle preview - -### Step 4: Strip marker from save path - -**File:** `packages/perseus-editor/src/editor-page.tsx` - -- In `serialize()` (line 254), after serializing, walk widgets and remove `_showShuffledPreview` from any radio widget options -- This prevents the marker from ever being persisted to content data - -### Step 5: Tests - -**Files:** - -- `packages/perseus-editor/src/widgets/__tests__/radio-editor.test.tsx` — Toggle UI behavior -- `packages/perseus-editor/src/__tests__/editor-page.test.tsx` (or similar) — Preview/save data processing - -Test cases: - -- Toggle renders and is disabled when randomize is off -- Toggle is enabled when randomize is on -- Toggle resets to false when randomize is turned off -- `serialize()` includes `_showShuffledPreview: true` only when both randomize and toggle are on -- `serialize()` does NOT include marker when toggle is off -- `updateRenderer()` sets `randomize: false` for radio widgets without the marker -- `updateRenderer()` leaves `randomize: true` for radio widgets with the marker -- Save path strips the marker field - -## Key Files - -| File | Change | -| ----------------------------------------------------------------------------------- | ---------------------------------------------------- | -| `packages/perseus-editor/src/widgets/radio/editor.tsx` | Add toggle, state, InfoTip, serialize marker | -| `packages/perseus-editor/src/editor-page.tsx` | Process marker in updateRenderer, strip in serialize | -| `packages/perseus-editor/src/widgets/image-editor/components/decorative-toggle.tsx` | Reference only (LabeledSwitch + InfoTip pattern) | - -## Verification - -1. `pnpm --filter perseus-editor test` — editor unit tests pass -2. `pnpm test` — all tests across the monorepo pass (no regressions) -3. `pnpm tsc` — no type errors -4. `pnpm lint` — no linting issues (run `pnpm lint --fix` if any arise) -5. `pnpm storybook` — manual testing in the Radio widget editor: - - Toggle Randomize on → sidebar preview should show unshuffled (default) - - Toggle "Shuffle preview" on → sidebar preview should show shuffled - - Toggle "Shuffle preview" off → sidebar preview returns to unshuffled - - Toggle Randomize off → "Shuffle preview" should be disabled, preview unshuffled - - Switch to Preview tab → choices should always be shuffled when Randomize is on - ---- - -*Co-authored by Claude Opus 4.6* \ No newline at end of file diff --git a/.claude/research-LEMS-3710.md b/.claude/research-LEMS-3710.md deleted file mode 100644 index bc045454e83..00000000000 --- a/.claude/research-LEMS-3710.md +++ /dev/null @@ -1,134 +0,0 @@ -# LEMS-3710 Research: Radio Widget Shuffle Toggle - -## Overview - -This document captures research into how the Radio widget handles shuffling, how the Edit and Preview tabs differ architecturally, and what tooltip patterns exist in the editor. - -## Shuffle Mechanics - -### Core Shuffle Logic -- **`choiceTransform()`** in `packages/perseus/src/widgets/radio/util.ts` (lines 116-146) is the central orchestration function -- Applies three transforms in order: - 1. **Shuffle** (lines 140-141): If `randomize` is truthy, calls `shuffle(choicesWithMetadata, seed)` - 2. **Enforce ordering** (lines 96-113): Swaps True/False or Yes/No into canonical order - 3. **Move "None of the above" to end** (lines 74-94): Ensures `isNoneOfTheAbove` choice is always last - -### Shuffle Utility -- `packages/perseus-core/src/utils/random-util.ts` (lines 34-51) -- Uses seeded RNG (Robert Jenkins' 32-bit integer hash) with Fisher-Yates algorithm -- Seed is computed as `problemNum + widgetIndex` in `radio.ff.tsx` (lines 182-183) - -### Where Shuffling is Triggered -- **`Radio._mergePropsAndState()`** in `packages/perseus/src/widgets/radio/radio.ff.tsx` (lines 164-215) -- Calls `choiceTransform()` with the original choices, `randomize` flag, localized strings, and computed seed -- The shuffled `RadioChoiceWithMetadata[]` is passed down to `MultipleChoiceWidget` - -### Choice IDs -- Choice IDs are stable/opaque identifiers that survive shuffling -- No "unshuffling by index" is needed — IDs carry the mapping -- `getUserInputFromSerializedState()` in `util.ts` (lines 52-72) extracts `selectedChoiceIds` from choice states - -### Default Value -- `randomize` defaults to `false` in `packages/perseus-core/src/widgets/radio/index.ts` (line 25) - -## Edit Tab vs Preview Tab Architecture - -### Edit Tab (Sidebar Preview) -- **iframe-based rendering** -- `packages/perseus-editor/src/item-editor.tsx` (lines 236-250): Renders `IframeContentRenderer` inside `DeviceFramer` -- `packages/perseus-editor/src/editor-page.tsx` (lines 204-238): `updateRenderer()` serializes editor state and sends to iframe -- Always sends `reviewMode: true` (line 235) -- `problemNum` passed through from props (line 237) - -### Preview Tab (ContentPreview) -- **Inline rendering** (no iframe) -- `packages/perseus-editor/src/content-preview.tsx` (lines 33-116) -- `problemNum` is hardcoded to `0` (line 81) -- `reviewMode` passed through as a prop -- Uses `UserInputManager` for state management - -### Key Differences - -| Signal | Edit Tab (iframe) | Preview Tab (ContentPreview) | -|--------|-------------------|------------------------------| -| `reviewMode` | Always `true` | Passed through | -| `problemNum` | From parent props | Hardcoded to `0` | -| Rendering | `IframeContentRenderer` + `ServerItemRenderer` | Inline `Renderer` + `UserInputManager` | - -### Widget-Level Distinction -- The Radio widget receives `reviewMode`, `static`, `showSolutions`, `apiOptions.readOnly` -- There is **no `editMode` prop** on the widget — the Edit vs Preview distinction comes from `reviewMode` being set differently -- In `multiple-choice-widget.tsx` (line 362): `const isReviewMode = reviewMode || isStatic || showSolutions === "all"` - -## Data Flow: Editor to Widget - -``` -RadioEditor (author choices, unshuffled) - | - v serialize() -PerseusRadioWidgetOptions { choices, randomize, ... } - | - v stored in PerseusRenderer JSON -Renderer.getWidgetProps() adds problemNum, widgetIndex, reviewMode - | - v -Radio (radio.ff.tsx) _mergePropsAndState() -> choiceTransform() -> shuffle - | - v shuffled RadioChoiceWithMetadata[] -MultipleChoiceWidget (multiple-choice-widget.tsx) - | - v buildChoiceProps() -> ChoiceType[] -MultipleChoiceComponent (renders the UI) -``` - -## Randomize Toggle in Editor - -### Current Implementation -- **File:** `packages/perseus-editor/src/widgets/radio/editor.tsx` (lines 335-343) -- Uses `LabeledSwitch` component with label "Randomize order" -- No tooltip currently attached -- Import: `import LabeledSwitch from "../../components/labeled-switch";` - -### LabeledSwitch Component -- **File:** `packages/perseus-editor/src/components/labeled-switch.tsx` -- Wraps `Switch` from `@khanacademy/wonder-blocks-switch` -- Uses `LabelSmall` or `LabelMedium` from `@khanacademy/wonder-blocks-typography` - -## Existing Tooltip Patterns in Editor - -### Pattern 1: LabeledSwitch + InfoTip (Best Precedent) -- **File:** `packages/perseus-editor/src/widgets/image-editor/components/decorative-toggle.tsx` (lines 51-65) -- `LabeledSwitch` and `InfoTip` as siblings in a flex row -- CSS: `display: flex; align-items: center; gap: var(--wb-sizing-size_040);` - -### Pattern 2: Checkbox with InfoTip in Label -- **File:** `packages/perseus-editor/src/item-extras-editor.tsx` (lines 168-188) -- `InfoTip` nested inside `Checkbox` label prop - -### Pattern 3: Standalone InfoTip -- **File:** `packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figure-aria.tsx` (lines 39-51) -- `InfoTip` with multiline description next to a label - -### InfoTip Component -- **Wrapper:** `packages/perseus/src/components/info-tip/index.tsx` -- **Core:** `packages/perseus/src/components/info-tip/info-tip-base.tsx` (lines 13-26) -- Uses `Tooltip` from `@khanacademy/wonder-blocks-tooltip` + `PhosphorIcon` question mark icon -- Import pattern: `const {InfoTip} = components;` (from `@khanacademy/perseus`) - -## Key Files for Implementation - -| Purpose | File | -|---------|------| -| Radio editor UI | `packages/perseus-editor/src/widgets/radio/editor.tsx` | -| Edit tab preview data | `packages/perseus-editor/src/editor-page.tsx` | -| Preview tab rendering | `packages/perseus-editor/src/content-preview.tsx` | -| Item editor (tab container) | `packages/perseus-editor/src/item-editor.tsx` | -| Shuffle logic | `packages/perseus/src/widgets/radio/util.ts` | -| Radio widget wrapper | `packages/perseus/src/widgets/radio/radio.ff.tsx` | -| InfoTip component | `packages/perseus/src/components/info-tip/info-tip-base.tsx` | -| LabeledSwitch component | `packages/perseus-editor/src/components/labeled-switch.tsx` | -| Decorative toggle (tooltip precedent) | `packages/perseus-editor/src/widgets/image-editor/components/decorative-toggle.tsx` | - ---- - -*Co-authored by Claude Opus 4.6* \ No newline at end of file diff --git a/.claude/research-editor-preview-dataflow-LEMS-3710.md b/.claude/research-editor-preview-dataflow-LEMS-3710.md deleted file mode 100644 index 076bca0f205..00000000000 --- a/.claude/research-editor-preview-dataflow-LEMS-3710.md +++ /dev/null @@ -1,181 +0,0 @@ -# Editor Preview Data Flow Deep Dive — LEMS-3710 - -## Overview - -This document traces the complete data flow from when a content creator changes a setting in the Radio widget editor to when the Edit tab sidebar preview re-renders. Understanding this flow is critical for knowing where to intercept the `randomize` flag. - -## The Full onChange-to-Preview Flow - -When the user toggles "Randomize order" (or any setting), here's the complete chain: - -### 1. User clicks the Switch -**File:** `packages/perseus-editor/src/widgets/radio/editor.tsx` (lines 339-341) -```typescript -onChange={(value) => { - this.props.onChange({randomize: value}); -}} -``` - -### 2. WidgetEditor merges the change -**File:** `packages/perseus-editor/src/widget-editor.tsx` (lines 85-103) -```typescript -_handleWidgetChange = (newProps, cb, silent) => { - const newWidgetInfo = { - ...this.state.widgetInfo, - options: { - ...this.state.widgetInfo.options, - ...(this.widget.current?.serialize() ?? {}), - ...newProps, - }, - }; - this.props.onChange(newWidgetInfo, cb, silent); -}; -``` -Merges `{randomize: value}` into `widgetInfo.options`, creating a full `PerseusWidget` object. - -### 3. Editor updates widgets -**File:** `packages/perseus-editor/src/editor.tsx` (lines 271-285) - -Copies all widgets, updates the changed widget by ID: -```typescript -this.props.onChange({widgets}, cb, silent); -``` - -### 4. ItemEditor merges into question -**File:** `packages/perseus-editor/src/item-editor.tsx` (lines 156-159) -```typescript -const question = _.extend({}, this.props.question, newProps); -this.updateProps({question}, cb, silent); -``` - -### 5. ItemEditor.updateProps propagates up -**File:** `packages/perseus-editor/src/item-editor.tsx` (lines 145-149) - -Merges into `{question, answerArea}`, calls `this.props.onChange(...)`. - -### 6. EditorPage.handleChange -**File:** `packages/perseus-editor/src/editor-page.tsx` (lines 264-268) - -Calls `this.props.onChange(newProps, cb, silent)` to the host application. - -### 7. Host re-renders EditorPage → componentDidUpdate fires -**File:** `packages/perseus-editor/src/editor-page.tsx` (lines 139-149) -```typescript -componentDidUpdate(previousProps, prevState, snapshot) { - setTimeout(() => { - this.updateRenderer(); - }); -} -``` -Every prop change triggers `updateRenderer()` via `setTimeout`. - -### 8. updateRenderer() serializes and sends to iframe -**File:** `packages/perseus-editor/src/editor-page.tsx` (lines 204-239) -```typescript -updateRenderer() { - // ... - this.itemEditor.current?.triggerPreviewUpdate({ - type: "question", - data: _({ - item: this.serialize(), // <-- serializes entire item - apiOptions: deviceBasedApiOptions, - initialHintsVisible: 0, - device: this.props.previewDevice, - linterContext: { ... }, - reviewMode: true, // <-- always true for Edit tab - legacyPerseusLint: ..., - }).extend(_(this.props).pick("problemNum")), - }); -} -``` - -### 9. Serialization chain -`this.serialize()` walks the tree: -- `EditorPage.serialize()` (line 254) → `ItemEditor.serialize()` → `Editor.serialize()` → iterates widget refs → `RadioEditor.serialize()` -- `RadioEditor.serialize()` returns `{choices, randomize, multipleSelect, ...}` -- The `randomize` value flows through unchanged into `item.question.widgets["radio 1"].options.randomize` - -### 10. Data sent to iframe -**File:** `packages/perseus-editor/src/item-editor.tsx` (lines 151-153) -```typescript -triggerPreviewUpdate: (newData?: any) => void = (newData: any) => { - this.frame.current?.sendNewData(newData); -}; -``` - -### 11. IframeContentRenderer posts to iframe -**File:** `packages/perseus-editor/src/iframe-content-renderer.tsx` (lines 179-190) -```typescript -sendNewData(data: any) { - if (this._isMounted && data && frame?.contentWindow) { - this._lastData = data; - window.iframeDataStore[this.iframeID] = data; - frame.contentWindow.postMessage(this.iframeID, "*"); - } -} -``` -Data stored in `window.iframeDataStore[id]`, iframe notified via `postMessage`. - -### 12. Iframe renders the preview -The iframe reads `window.parent.iframeDataStore[id]` and renders the item with the received data, including the `randomize` value. - -## Data Structure Sent to Iframe - -```typescript -{ - type: "question", - data: { - item: { - question: { - content: "...", - widgets: { - "radio 1": { - type: "radio", - options: { - choices: [...], - randomize: boolean, // <-- THIS IS THE KEY FIELD - multipleSelect: boolean, - // ... - } - } - } - }, - answerArea: {...}, - hints: [...] - }, - apiOptions: {...}, - reviewMode: true, - problemNum: number, - // ... - } -} -``` - -## Key Insight: serialize() Dual Use - -`RadioEditor.serialize()` is called in TWO contexts: -1. **Preview updates** — via `updateRenderer()` → `this.serialize()` chain -2. **Saving content** — via `EditorPage.serialize()` when the host app saves - -Both paths call the same `serialize()` method, so any modification to `serialize()` output affects both preview and saving. This is why a marker field approach (include a temporary field, strip it on save) is needed to control preview behavior without affecting saved data. - -## Edit Tab vs Preview Tab: Key Differences - -| Aspect | Edit Tab (sidebar) | Preview Tab | -|--------|-------------------|-------------| -| Rendering | iframe via `IframeContentRenderer` | Inline via `ContentPreview` → `Renderer` | -| `reviewMode` | Always `true` | Passed through | -| `problemNum` | From parent props | Hardcoded to `0` | -| Data source | `EditorPage.updateRenderer()` | Widget options from props directly | -| Shuffle control | Through iframe data pipeline | Through widget options directly | - -## Interception Point - -The best place to modify `randomize` for the preview is in `EditorPage.updateRenderer()`, after `this.serialize()` returns but before the data is sent to `triggerPreviewUpdate()`. This lets us: -- Leave `RadioEditor.serialize()` mostly unchanged (just add a marker) -- Override `randomize` only for the Edit tab preview -- Keep the Preview tab unaffected (it doesn't go through `updateRenderer()`) - ---- - -*Co-authored by Claude Opus 4.6* \ No newline at end of file diff --git a/.claude/ticket-summary-LEMS-3710.md b/.claude/ticket-summary-LEMS-3710.md deleted file mode 100644 index 9dbfb651fe9..00000000000 --- a/.claude/ticket-summary-LEMS-3710.md +++ /dev/null @@ -1,25 +0,0 @@ -# LEMS-3710: [Radio] | (CX) | Add toggle for showing preview unshuffled - -**Status:** To Do -**Type:** Software Task -**Assignee:** Tamara Bozich - -## Summary - -Content creators using the Radio Widget want the ability to preview choice options **unshuffled** in the editor sidebar, even when the widget is configured for randomization. This makes it easier to review content in a predictable order while authoring. - -## Acceptance Criteria - -1. **Edit tab preview**: Always shows choices **unshuffled** (regardless of randomize setting) -2. **Preview tab**: Shows choices **shuffled** only when the widget is configured to randomize -3. **Info tooltip**: Add a tooltip next to the "Randomize" toggle explaining that only the "Preview" tab shows shuffled choices - -## Key Context - -- The request came from content creators who find shuffled previews unhelpful while editing -- The Edit tab sidebar preview currently shuffles choices when randomize is on, making it harder to map choices to the editor fields -- The Preview tab is where the learner experience is simulated, so shuffling belongs there - ---- - -*Co-authored by Claude Opus 4.6* \ No newline at end of file From 01bb9ffad97fb3f915797f62832cdf28f37ff252 Mon Sep 17 00:00:00 2001 From: Tamara Date: Wed, 11 Feb 2026 16:13:42 -0500 Subject: [PATCH 10/12] [tb/LEMS-3710/toggle-for-shuffle] Address subagent code review findings --- .claude/code-review-LEMS-3710.md | 67 +++++++++++++ .../perseus-editor/src/editor-page.test.tsx | 95 +++++++++++++++++++ packages/perseus-editor/src/editor-page.tsx | 34 ++++--- .../src/widgets/radio/editor.tsx | 4 +- 4 files changed, 185 insertions(+), 15 deletions(-) create mode 100644 .claude/code-review-LEMS-3710.md diff --git a/.claude/code-review-LEMS-3710.md b/.claude/code-review-LEMS-3710.md new file mode 100644 index 00000000000..ec97b7862f6 --- /dev/null +++ b/.claude/code-review-LEMS-3710.md @@ -0,0 +1,67 @@ +# Code Review: LEMS-3710 — Shuffle Preview Toggle + +## Review Summary + +Three parallel reviews were conducted focusing on: (1) Correctness & Functionality, (2) Code Quality & Maintainability, and (3) Security & Performance. + +**No critical issues found.** Three warnings converged across multiple reviewers and should be addressed before merging. + +--- + +## Converging Findings (flagged by 2+ reviewers) + +### 1. Mutation of shared serialized objects (all 3 reviewers) + +Both `serializeForPreview()` and `serialize()` mutate the object returned by `itemEditor.serialize()` in place (`delete` and property assignment). If the serialized objects share references with the widget editor's internal state, this could corrupt state between frames — particularly `widget.options.randomize = false` being set on every preview tick. + +**Fix:** Shallow-copy `widget.options` before mutating: + +```typescript +if (widget?.type === "radio" && widget.options) { + widget.options = {...widget.options}; + // then delete/modify safely +} +``` + +### 2. Missing EditorPage-level tests (all 3 reviewers) + +No tests exist for `EditorPage.serialize()` stripping or `serializeForPreview()` override logic. The RadioEditor tests cover the widget level well, but the save-path safety net and preview-path correctness live in EditorPage and are untested. + +### 3. `getSnapshotBeforeUpdate` bypasses stripping (2 reviewers) + +When toggling into JSON mode, `getSnapshotBeforeUpdate` calls `itemEditor.serialize()` directly — not `EditorPage.serialize()`. This means `_showShuffledPreview` could appear in the JSON editor and potentially be saved from there. Low risk (developer-only JSON mode) but an inconsistency. + +--- + +## Additional Findings + +### Code Quality + +- **Import ordering:** `const {InfoTip} = components;` sits between import blocks. Should be moved after all imports to match patterns in `input-number-editor.tsx` and `dropdown-editor.tsx`. +- **Duplicated serialization base:** `serialize()` and `serializeForPreview()` both call `_.extend(this.itemEditor.current?.serialize(), {...})`. Could extract a shared `serializeBase()` helper. +- **Conditional spread readability:** `...(!value && { _showShuffledPreview: false })` is concise but harder to parse at a glance. A plain `if` would be more readable. +- **Disabled color override:** Manual `color: semanticColor.core.foreground.disabled.default` may be unnecessary if `LabeledSwitch` already handles disabled styling via the `disabled` prop. Worth verifying. +- **`serializeForPreview()` return type:** Claims `PerseusItem` but is built from the same `any`-returning pattern as `serialize()`. Inconsistency worth noting. + +### Security & Performance + +- **No security vulnerabilities identified.** Editor-only changes, no user-controlled data in dangerous sinks. +- **No performance issues.** Widget iteration is O(n) over a small set (<10 widgets). No double-serialization on the hot path. +- **`delete` operator:** Negligible performance impact in modern V8. Acceptable. + +--- + +## Recommended Actions + +| Priority | Action | Effort | +|----------|--------|--------| +| **High** | Add shallow copy before mutating widget.options in both serialize methods | Small | +| **High** | Add EditorPage tests for stripping and preview override | Medium | +| **Medium** | Fix import ordering in editor.tsx | Trivial | +| **Medium** | Strip marker in `getSnapshotBeforeUpdate` or extract shared helper | Small | +| **Low** | Extract `serializeBase()` to reduce duplication | Small | +| **Low** | Verify disabled color override is needed | Trivial | + +--- + +*Co-authored by Claude Opus 4.6* \ No newline at end of file diff --git a/packages/perseus-editor/src/editor-page.test.tsx b/packages/perseus-editor/src/editor-page.test.tsx index 358f037df59..2ecb895442f 100644 --- a/packages/perseus-editor/src/editor-page.test.tsx +++ b/packages/perseus-editor/src/editor-page.test.tsx @@ -1,5 +1,7 @@ import {Dependencies} from "@khanacademy/perseus"; import { + generateRadioWidget, + generateRadioOptions, type PerseusOrdererWidgetOptions, type PerseusRenderer, } from "@khanacademy/perseus-core"; @@ -278,4 +280,97 @@ describe("EditorPage", () => { screen.getByDisplayValue(/Updated content from parent/), ).toBeInTheDocument(); }); + + describe("radio shuffle preview stripping", () => { + function radioQuestion( + optionOverrides: Record = {}, + ): PerseusRenderer { + return { + content: "[[☃ radio 1]]", + images: {}, + widgets: { + "radio 1": generateRadioWidget({ + options: generateRadioOptions({ + choices: [ + {id: "id-1", content: "A", correct: true}, + {id: "id-2", content: "B"}, + ], + randomize: true, + ...optionOverrides, + }), + }), + }, + }; + } + + it("strips _showShuffledPreview from serialize() output", () => { + const editorRef = React.createRef(); + + render( + {}} + onPreviewDeviceChange={() => {}} + previewDevice="desktop" + previewURL="" + itemId="itemId" + developerMode={false} + jsonMode={false} + />, + ); + + const serialized = editorRef.current?.serialize(); + const options = serialized?.question?.widgets?.["radio 1"]?.options; + + expect(options?._showShuffledPreview).toBeUndefined(); + expect(options?.randomize).toBe(true); + }); + + it("strips _showShuffledPreview from non-radio widgets without error", () => { + const editorRef = React.createRef(); + + const question: PerseusRenderer = { + content: "[[☃ categorizer 1]]", + images: {}, + widgets: { + "categorizer 1": { + type: "categorizer", + static: false, + options: { + static: false, + items: ["A"], + categories: ["B"], + values: [0], + randomizeItems: false, + }, + }, + }, + }; + + render( + {}} + onPreviewDeviceChange={() => {}} + previewDevice="desktop" + previewURL="" + itemId="itemId" + developerMode={false} + jsonMode={false} + />, + ); + + // Should not throw + const serialized = editorRef.current?.serialize(); + expect( + serialized?.question?.widgets?.["categorizer 1"], + ).toBeDefined(); + }); + }); }); diff --git a/packages/perseus-editor/src/editor-page.tsx b/packages/perseus-editor/src/editor-page.tsx index dcfe3603413..e7a1e25c8f8 100644 --- a/packages/perseus-editor/src/editor-page.tsx +++ b/packages/perseus-editor/src/editor-page.tsx @@ -87,6 +87,21 @@ type State = { widgetsAreOpen: boolean; }; +// Strips editor-only marker fields (e.g. _showShuffledPreview) from +// radio widget options so they are never persisted to content data. +function stripEditorOnlyRadioFields(item: any) { + const widgets = item?.question?.widgets; + if (widgets) { + for (const widgetId of Object.keys(widgets)) { + const widget = widgets[widgetId]; + if (widget?.type === "radio" && widget.options) { + widget.options = {...widget.options}; + delete widget.options._showShuffledPreview; + } + } + } +} + class EditorPage extends React.Component { _isMounted: boolean; @@ -124,7 +139,7 @@ class EditorPage extends React.Component { getSnapshotBeforeUpdate(prevProps: Props, prevState: State) { if (!prevProps.jsonMode && this.props.jsonMode) { - return { + const snapshot = { ...(this.itemEditor.current?.serialize({ keepDeletedWidgets: true, }) ?? {}), @@ -132,6 +147,8 @@ class EditorPage extends React.Component { keepDeletedWidgets: true, }), }; + stripEditorOnlyRadioFields(snapshot); + return snapshot; } return null; } @@ -258,18 +275,7 @@ class EditorPage extends React.Component { const result = _.extend(this.itemEditor.current?.serialize(options), { hints: this.hintsEditor.current?.serialize(options), }); - - // Strip editor-only marker fields from radio widget options - // so they are never persisted to content data. - const widgets = result?.question?.widgets; - if (widgets) { - for (const widgetId of Object.keys(widgets)) { - const widget = widgets[widgetId]; - if (widget?.type === "radio" && widget.options) { - delete widget.options._showShuffledPreview; - } - } - } + stripEditorOnlyRadioFields(result); return result; } @@ -290,6 +296,8 @@ class EditorPage extends React.Component { for (const widgetId of Object.keys(widgets)) { const widget = widgets[widgetId]; if (widget?.type === "radio" && widget.options) { + // Shallow copy to avoid mutating the source object + widget.options = {...widget.options}; if (widget.options._showShuffledPreview) { delete widget.options._showShuffledPreview; } else { diff --git a/packages/perseus-editor/src/widgets/radio/editor.tsx b/packages/perseus-editor/src/widgets/radio/editor.tsx index a8ca149ddfc..ec44c2cc18e 100644 --- a/packages/perseus-editor/src/widgets/radio/editor.tsx +++ b/packages/perseus-editor/src/widgets/radio/editor.tsx @@ -11,8 +11,6 @@ import _ from "underscore"; import LabeledSwitch from "../../components/labeled-switch"; -const {InfoTip} = components; - import {RadioOptionSettings} from "./radio-option-settings"; import {getMovedChoices} from "./utils"; @@ -25,6 +23,8 @@ import type { RadioDefaultWidgetOptions, } from "@khanacademy/perseus-core"; +const {InfoTip} = components; + type RadioSerializedOptions = PerseusRadioWidgetOptions & { _showShuffledPreview?: boolean; }; From 1e6151e6474f9142005b5102d5b2f9a08a663c4a Mon Sep 17 00:00:00 2001 From: Tamara Date: Wed, 11 Feb 2026 16:14:06 -0500 Subject: [PATCH 11/12] [tb/LEMS-3710/toggle-for-shuffle] Remove review doc from final PR --- .claude/code-review-LEMS-3710.md | 67 -------------------------------- 1 file changed, 67 deletions(-) delete mode 100644 .claude/code-review-LEMS-3710.md diff --git a/.claude/code-review-LEMS-3710.md b/.claude/code-review-LEMS-3710.md deleted file mode 100644 index ec97b7862f6..00000000000 --- a/.claude/code-review-LEMS-3710.md +++ /dev/null @@ -1,67 +0,0 @@ -# Code Review: LEMS-3710 — Shuffle Preview Toggle - -## Review Summary - -Three parallel reviews were conducted focusing on: (1) Correctness & Functionality, (2) Code Quality & Maintainability, and (3) Security & Performance. - -**No critical issues found.** Three warnings converged across multiple reviewers and should be addressed before merging. - ---- - -## Converging Findings (flagged by 2+ reviewers) - -### 1. Mutation of shared serialized objects (all 3 reviewers) - -Both `serializeForPreview()` and `serialize()` mutate the object returned by `itemEditor.serialize()` in place (`delete` and property assignment). If the serialized objects share references with the widget editor's internal state, this could corrupt state between frames — particularly `widget.options.randomize = false` being set on every preview tick. - -**Fix:** Shallow-copy `widget.options` before mutating: - -```typescript -if (widget?.type === "radio" && widget.options) { - widget.options = {...widget.options}; - // then delete/modify safely -} -``` - -### 2. Missing EditorPage-level tests (all 3 reviewers) - -No tests exist for `EditorPage.serialize()` stripping or `serializeForPreview()` override logic. The RadioEditor tests cover the widget level well, but the save-path safety net and preview-path correctness live in EditorPage and are untested. - -### 3. `getSnapshotBeforeUpdate` bypasses stripping (2 reviewers) - -When toggling into JSON mode, `getSnapshotBeforeUpdate` calls `itemEditor.serialize()` directly — not `EditorPage.serialize()`. This means `_showShuffledPreview` could appear in the JSON editor and potentially be saved from there. Low risk (developer-only JSON mode) but an inconsistency. - ---- - -## Additional Findings - -### Code Quality - -- **Import ordering:** `const {InfoTip} = components;` sits between import blocks. Should be moved after all imports to match patterns in `input-number-editor.tsx` and `dropdown-editor.tsx`. -- **Duplicated serialization base:** `serialize()` and `serializeForPreview()` both call `_.extend(this.itemEditor.current?.serialize(), {...})`. Could extract a shared `serializeBase()` helper. -- **Conditional spread readability:** `...(!value && { _showShuffledPreview: false })` is concise but harder to parse at a glance. A plain `if` would be more readable. -- **Disabled color override:** Manual `color: semanticColor.core.foreground.disabled.default` may be unnecessary if `LabeledSwitch` already handles disabled styling via the `disabled` prop. Worth verifying. -- **`serializeForPreview()` return type:** Claims `PerseusItem` but is built from the same `any`-returning pattern as `serialize()`. Inconsistency worth noting. - -### Security & Performance - -- **No security vulnerabilities identified.** Editor-only changes, no user-controlled data in dangerous sinks. -- **No performance issues.** Widget iteration is O(n) over a small set (<10 widgets). No double-serialization on the hot path. -- **`delete` operator:** Negligible performance impact in modern V8. Acceptable. - ---- - -## Recommended Actions - -| Priority | Action | Effort | -|----------|--------|--------| -| **High** | Add shallow copy before mutating widget.options in both serialize methods | Small | -| **High** | Add EditorPage tests for stripping and preview override | Medium | -| **Medium** | Fix import ordering in editor.tsx | Trivial | -| **Medium** | Strip marker in `getSnapshotBeforeUpdate` or extract shared helper | Small | -| **Low** | Extract `serializeBase()` to reduce duplication | Small | -| **Low** | Verify disabled color override is needed | Trivial | - ---- - -*Co-authored by Claude Opus 4.6* \ No newline at end of file From 3c7e5bd458a46da9fcf85768de4f346bb6d7de28 Mon Sep 17 00:00:00 2001 From: Tamara Date: Wed, 11 Feb 2026 16:30:09 -0500 Subject: [PATCH 12/12] [tb/LEMS-3710/toggle-for-shuffle] Remove changeset from progressive disclosure branch This branch was based off a branch with some additional documentation for Claude and some changes to its CLAUDE.md. Those files are not yet landed, so removed those. Missed this changeset. --- .changeset/wicked-eggs-cheer.md | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 .changeset/wicked-eggs-cheer.md diff --git a/.changeset/wicked-eggs-cheer.md b/.changeset/wicked-eggs-cheer.md deleted file mode 100644 index a845151cc84..00000000000 --- a/.changeset/wicked-eggs-cheer.md +++ /dev/null @@ -1,2 +0,0 @@ ---- ----