Refactor LabelImage to separate out answers from userInput into scoringData

.changeset/many-penguins-hug.md

@@ -0,0 +1,6 @@
+---
+"@khanacademy/perseus": major
+"@khanacademy/perseus-editor": patch
+---
+
+Refactor the LabelImage widget to separate out answers from userInput into scoringData

packages/perseus-editor/src/widgets/__stories__/label-image-editor.stories.tsx

@@ -4,7 +4,10 @@ import * as React from "react";
 
 import LabelImageEditor from "../label-image-editor";
 
-import type {MarkerType} from "@khanacademy/perseus";
+import type {
+    LabelImageMarker,
+    LabelImageMarkerScoringData,
+} from "@khanacademy/perseus";
 
 type StoryArgs = Record<any, any>;
 
@@ -29,7 +32,7 @@ type State = {
     imageUrl: string;
     imageWidth: number;
     imageHeight: number;
-    markers: ReadonlyArray<MarkerType>;
+    markers: ReadonlyArray<LabelImageMarker & LabelImageMarkerScoringData>;
 };
 
 class WithState extends React.Component<Empty, State> {

packages/perseus-editor/src/widgets/label-image-editor.tsx

@@ -17,7 +17,10 @@ import Behavior from "./label-image/behavior";
 import QuestionMarkers from "./label-image/question-markers";
 import SelectImage from "./label-image/select-image";
 
-import type {MarkerType} from "@khanacademy/perseus";
+import type {
+    LabelImageMarker,
+    LabelImageMarkerScoringData,
+} from "@khanacademy/perseus";
 
 type Props = {
     // List of answer choices to label question image with.
@@ -28,7 +31,7 @@ type Props = {
     imageWidth: number;
     imageHeight: number;
     // The list of label markers on the question image.
-    markers: ReadonlyArray<MarkerType>;
+    markers: ReadonlyArray<LabelImageMarker & LabelImageMarkerScoringData>;
     // Whether multiple answer choices may be selected for markers.
     multipleAnswers: boolean;
     // Whether to hide answer choices from user instructions.
@@ -176,8 +179,10 @@ class LabelImageEditor extends React.Component<Props> {
         this.props.onChange({choices});
     };
 
-    handleMarkersChange: (markers: ReadonlyArray<MarkerType>) => void = (
-        markers: ReadonlyArray<MarkerType>,
+    handleMarkersChange: (
+        markers: ReadonlyArray<LabelImageMarker & LabelImageMarkerScoringData>,
+    ) => void = (
+        markers: ReadonlyArray<LabelImageMarker & LabelImageMarkerScoringData>,
     ) => {
         this.props.onChange({markers});
     };

packages/perseus-editor/src/widgets/label-image/__stories__/question-markers.stories.tsx

@@ -3,7 +3,10 @@ import * as React from "react";
 
 import QuestionMarkers from "../question-markers";
 
-import type {MarkerType} from "@khanacademy/perseus";
+import type {
+    LabelImageMarker,
+    LabelImageMarkerScoringData,
+} from "@khanacademy/perseus";
 
 type StoryArgs = Record<any, any>;
 
@@ -31,7 +34,7 @@ const Wrapper = (props) => (
 class WithState extends React.Component<
     Record<any, any>,
     {
-        markers: ReadonlyArray<MarkerType>;
+        markers: ReadonlyArray<LabelImageMarker & LabelImageMarkerScoringData>;
     }
 > {
     state = {

packages/perseus-editor/src/widgets/label-image/marker.tsx

@@ -14,16 +14,22 @@ import Option, {OptionGroup} from "../../components/dropdown-option";
 import FormWrappedTextField from "../../components/form-wrapped-text-field";
 import {gray17, gray85, gray98} from "../../styles/global-colors";
 
-import type {MarkerType} from "@khanacademy/perseus";
-
-type Props = MarkerType & {
-    // The list of possible answer choices.
-    choices: ReadonlyArray<string>;
-    // Callback for when any of the marker props are changed.
-    onChange: (marker: MarkerType) => void;
-    // Callback to remove marker from the question image.
-    onRemove: () => void;
-};
+import type {
+    LabelImageMarker,
+    LabelImageMarkerScoringData,
+} from "@khanacademy/perseus";
+
+type Props = LabelImageMarker &
+    LabelImageMarkerScoringData & {
+        // The list of possible answer choices.
+        choices: ReadonlyArray<string>;
+        // Callback for when any of the marker props are changed.
+        onChange: (
+            marker: LabelImageMarker & LabelImageMarkerScoringData,
+        ) => void;
+        // Callback to remove marker from the question image.
+        onRemove: () => void;
+    };
 
 type State = {
     // Whether answer choices dropdown is shown, controlled by the user clicking

packages/perseus-editor/src/widgets/label-image/question-markers.tsx

@@ -3,16 +3,20 @@
  * label image with answers from list of choices.
  */
 
-import {Util, Dependencies, bodyXsmallBold} from "@khanacademy/perseus";
+import {
+    Util,
+    Dependencies,
+    bodyXsmallBold,
+    type LabelImageMarker,
+    type LabelImageMarkerScoringData,
+} from "@khanacademy/perseus";
 import {StyleSheet, css} from "aphrodite";
 import * as React from "react";
 
 import {gray17, gray68} from "../../styles/global-colors";
 
 import Marker from "./marker";
 
-import type {MarkerType} from "@khanacademy/perseus";
-
 type Props = {
     // The list of possible answers in a specific order.
     choices: ReadonlyArray<string>;
@@ -21,9 +25,11 @@ type Props = {
     imageWidth: number;
     imageHeight: number;
     // The list of markers placed on the question image.
-    markers: ReadonlyArray<MarkerType>;
+    markers: ReadonlyArray<LabelImageMarker & LabelImageMarkerScoringData>;
     // Callback for when any of markers change.
-    onChange: (markers: ReadonlyArray<MarkerType>) => void;
+    onChange: (
+        markers: ReadonlyArray<LabelImageMarker & LabelImageMarkerScoringData>,
+    ) => void;
 };
 
 export default class QuestionMarkers extends React.Component<Props> {

packages/perseus/src/index.ts

@@ -234,7 +234,10 @@ export type {
 } from "./perseus-types";
 export type {UserInputMap} from "./validation.types";
 export type {Coord} from "./interactive2/types";
-export type {MarkerType} from "./widgets/label-image/types";
+export type {
+    LabelImageMarker,
+    LabelImageMarkerScoringData,
+} from "./widgets/label-image/types";
 
 /**
  * Multi-items

packages/perseus/src/perseus-types.ts

@@ -2,6 +2,10 @@
 // TODO(FEI-4011): Use types generated by https://github.com/jaredly/generate-perseus-flowtypes
 
 import type {Coord} from "./interactive2/types";
+import type {
+    LabelImageMarker,
+    LabelImageMarkerScoringData,
+} from "./widgets/label-image/types";
 import type {Interval, vec} from "mafs";
 
 // Range is replaced within this file with Interval, but it is used elsewhere
@@ -1050,7 +1054,7 @@ export type PerseusLabelImageWidgetOptions = {
     // The width of the image
     imageWidth: number;
     // A list of markers to display on the image
-    markers: ReadonlyArray<PerseusLabelImageMarker>;
+    markers: ReadonlyArray<LabelImageMarker & LabelImageMarkerScoringData>;
     // Do not display answer choices in instructions
     hideChoicesFromInstructions: boolean;
     // Allow multiple answers per marker
@@ -1059,17 +1063,6 @@ export type PerseusLabelImageWidgetOptions = {
     static: boolean;
 };
 
-export type PerseusLabelImageMarker = {
-    // A list of correct answers for this marker.  Often only one but can have multiple
-    answers: ReadonlyArray<string>;
-    // Translatable Text; The text to show for the marker. Not displayed directly to the user
-    label: string;
-    // X Coordiate location of the marker on the image
-    x: number;
-    // Y Coordinate location of the marker on the image
-    y: number;
-};
-
 export type PerseusMatcherWidgetOptions = {
     // Translatable Text; Labels to adorn the headings for the columns.  Only 2 values [left, right]. e.g. ["Concepts", "Things"]
     labels: ReadonlyArray<string>;

packages/perseus/src/validation.types.ts

@@ -43,7 +43,11 @@ import type {
     PerseusRadioChoice,
     PerseusGraphCorrectType,
 } from "./perseus-types";
-import type {InteractiveMarkerType} from "./widgets/label-image/types";
+import type {
+    LabelImageMarker,
+    LabelImageMarkerScoringData,
+    LabelImageMarkerUserInput,
+} from "./widgets/label-image/types";
 import type {Relationship} from "./widgets/number-line/number-line";
 
 export type UserInputStatus = "correct" | "incorrect" | "incomplete";
@@ -138,12 +142,12 @@ export type PerseusInteractiveGraphRubric = {
 
 export type PerseusInteractiveGraphUserInput = PerseusGraphType;
 
-/* TODO(LEMS-2440): Should be removed or refactored. Grading info may need
-    to be moved to the rubric from userInput. */
-export type PerseusLabelImageRubric = Empty;
+export type PerseusLabelImageScoringData = {
+    markers: ReadonlyArray<LabelImageMarker & LabelImageMarkerScoringData>;
+};
 
 export type PerseusLabelImageUserInput = {
-    markers: ReadonlyArray<InteractiveMarkerType>;
+    markers: ReadonlyArray<LabelImageMarker & LabelImageMarkerUserInput>;
 };
 
 export type PerseusMatcherRubric = PerseusMatcherWidgetOptions;
@@ -247,7 +251,7 @@ export type Rubric =
     | PerseusIFrameRubric
     | PerseusInputNumberRubric
     | PerseusInteractiveGraphRubric
-    | PerseusLabelImageRubric
+    | PerseusLabelImageScoringData
     | PerseusMatcherRubric
     | PerseusMatrixRubric
     | PerseusNumberLineScoringData

packages/perseus/src/widgets/label-image/label-image.tsx

@@ -27,13 +27,13 @@ import {HideAnswersToggle} from "./hide-answers-toggle";
 import Marker from "./marker";
 import scoreLabelImage, {scoreMarker} from "./score-label-image";
 
-import type {InteractiveMarkerType} from "./types";
+import type {LabelImageFullMarker} from "./types";
 import type {DependencyProps} from "../../dependencies";
 import type {ChangeableProps} from "../../mixins/changeable";
 import type {PerseusLabelImageWidgetOptions} from "../../perseus-types";
 import type {APIOptions, Widget, WidgetExports} from "../../types";
 import type {
-    PerseusLabelImageRubric,
+    PerseusLabelImageScoringData,
     PerseusLabelImageUserInput,
 } from "../../validation.types";
 import type {LabelImagePromptJSON} from "../../widget-ai-utils/label-image/label-image-ai-utils";
@@ -72,12 +72,10 @@ type Point = {
 
 type LabelImageProps = ChangeableProps &
     DependencyProps &
-    // TODO: there's some weirdness in our types between
-    // PerseusLabelImageMarker and InteractiveMarkerType
     Omit<PerseusLabelImageWidgetOptions, "markers"> & {
         apiOptions: APIOptions;
         // The list of label markers on the question image.
-        markers: ReadonlyArray<InteractiveMarkerType>;
+        markers: ReadonlyArray<LabelImageFullMarker>;
         // Whether the question has been answered by the user.
         questionCompleted: boolean;
         // preferred placement for popover (preference, not MUST)
@@ -194,7 +192,7 @@ export class LabelImage
      */
     static navigateToMarkerIndex(
         navigateDirection: Direction,
-        markers: ReadonlyArray<InteractiveMarkerType>,
+        markers: PerseusLabelImageUserInput["markers"],
         thisIndex: number,
     ): number {
         const thisMarker = markers[thisIndex];
@@ -318,13 +316,16 @@ export class LabelImage
         return _getPromptJSON(this.props, this.getUserInput());
     }
 
-    // TODO(LEMS-2544): Investigate impact on scoring; possibly pull out &/or remove rubric parameter.
-    showRationalesForCurrentlySelectedChoices(rubric: PerseusLabelImageRubric) {
+    // TODO(LEMS-2544): Investigate impact on scoring; possibly pull out &/or remove scoringData parameter.
+    // Also consider how scoreMarker is being called as it seems to require the marker.answers property.
+    showRationalesForCurrentlySelectedChoices(
+        scoringData: PerseusLabelImageScoringData,
+    ) {
         const {markers} = this.props;
         const {onChange} = this.props;
 
         const updatedMarkers = markers.map((marker) => {
-            const score = scoreMarker(marker);
+            const score = scoreMarker(marker.selected, marker.answers);
 
             return {
                 ...marker,
@@ -342,7 +343,10 @@ export class LabelImage
         onChange({markers: updatedMarkers}, null, true);
     }
 
-    handleMarkerChange(index: number, marker: InteractiveMarkerType) {
+    handleMarkerChange(
+        index: number,
+        marker: PerseusLabelImageUserInput["markers"][number],
+    ) {
         const {markers, onChange} = this.props;
 
         // Replace marker with a changed version at the specified index.
@@ -476,7 +480,7 @@ export class LabelImage
                 }[markerPosition];
             }
 
-            const score = scoreMarker(marker);
+            const score = scoreMarker(marker.selected, marker.answers);
             // Once the question is answered, show markers
             // with correct answers, otherwise passthrough
             // the correctness state.

packages/perseus/src/widgets/label-image/marker.tsx

@@ -16,12 +16,12 @@ import {iconCheck, iconChevronDown, iconMinus} from "../../icon-paths";
 
 import {AnswerPill} from "./answer-pill";
 
-import type {InteractiveMarkerType} from "./types";
+import type {LabelImageFullMarker} from "./types";
 import type {IconType} from "../../components/icon";
 import type {AnalyticsEventHandlerFn} from "@khanacademy/perseus-core";
 import type {CSSProperties} from "aphrodite";
 
-type Props = InteractiveMarkerType & {
+type Props = LabelImageFullMarker & {
     // Whether this marker has been selected by user.
     showSelected: boolean;
     // Whether this marker should pulsate to draw user attention.