diff --git a/.changeset/famous-ghosts-type.md b/.changeset/famous-ghosts-type.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/famous-ghosts-type.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/.changeset/short-drinks-tap.md b/.changeset/short-drinks-tap.md new file mode 100644 index 00000000000..17d9325e6d8 --- /dev/null +++ b/.changeset/short-drinks-tap.md @@ -0,0 +1,8 @@ +--- +"@khanacademy/perseus": minor +"@khanacademy/perseus-editor": minor +"@khanacademy/perseus-score": minor +"@khanacademy/perseus-core": patch +--- + +Implement "ungraded" InteractiveGraphs: interactive IGs that aren't scored as part of the regular scoring flow diff --git a/__docs__/sample-data.ts b/__docs__/sample-data.ts index 05ba30dccd4..b98e91afb2b 100644 --- a/__docs__/sample-data.ts +++ b/__docs__/sample-data.ts @@ -4,7 +4,7 @@ import type {PerseusRenderer} from "@khanacademy/perseus-core"; // it. export const graphExample: PerseusRenderer = { content: - "An example of a the beautiful**interactive-graph** widget:\n\n[[☃ interactive-graph 1]]", + "An example of a the beautiful **interactive-graph** widget:\n\n[[☃ interactive-graph 1]]", widgets: { "interactive-graph 1": { type: "interactive-graph", diff --git a/packages/perseus-core/src/data-schema.ts b/packages/perseus-core/src/data-schema.ts index 789cc991ae5..8e26d7cb3e4 100644 --- a/packages/perseus-core/src/data-schema.ts +++ b/packages/perseus-core/src/data-schema.ts @@ -376,8 +376,19 @@ export type WidgetOptions< */ static?: boolean; /** - * Whether a widget is scored. Usually true except for IFrame widgets (deprecated). + * Whether a widget is scored. * Default: true + * + * The behavior depends on how the widget decides to implement it. + * For example, Interactive Graph will render an ungraded graph + * that is still interactive that learners can use to visualize + * math. + * + * Historical uses seem questionable (See LEMS-3958): + * - IFrame + * - Explanation + * - Image + * - Transformer (deprecated) */ graded?: boolean; /** diff --git a/packages/perseus-core/src/feature-flags.ts b/packages/perseus-core/src/feature-flags.ts index 3c43c376211..c8cd85f4e8f 100644 --- a/packages/perseus-core/src/feature-flags.ts +++ b/packages/perseus-core/src/feature-flags.ts @@ -18,6 +18,7 @@ const PerseusFeatureFlags = [ "interactive-graph-logarithm", // TODO(LEMS-3976): clean up feature flag "interactive-graph-exponent", // TODO(LEMS-3976): clean up feature flag "interactive-graph-vector", // TODO(LEMS-3976): clean up feature flag + "interactive-graph-not-scored", // TODO(LEMS-3976): clean up feature flag ] as const; export default PerseusFeatureFlags; diff --git a/packages/perseus-core/src/utils/generators/interactive-graph-widget-generator.ts b/packages/perseus-core/src/utils/generators/interactive-graph-widget-generator.ts index dcf31d36e67..a8527032c25 100644 --- a/packages/perseus-core/src/utils/generators/interactive-graph-widget-generator.ts +++ b/packages/perseus-core/src/utils/generators/interactive-graph-widget-generator.ts @@ -265,9 +265,15 @@ export function generateInteractiveGraphQuestion( options?: Partial & { content?: string; isStatic?: boolean; + graded?: boolean; }, ): PerseusRenderer { - const {content, isStatic, ...widgetOptions} = options ?? {}; + const { + content = "[[☃ interactive-graph 1]]", + isStatic = false, + graded = true, + ...widgetOptions + } = options ?? {}; // The `graph` and `correct` fields share all fields except for // the answers (coords, center, etc.) When only `correct` is provided, @@ -293,10 +299,11 @@ export function generateInteractiveGraphQuestion( }; return generateTestPerseusRenderer({ - content: content ?? "[[☃ interactive-graph 1]]", + content: content, widgets: { "interactive-graph 1": generateInteractiveGraphWidget({ - static: isStatic ?? false, + static: isStatic, + graded, options: generateInteractiveGraphOptions(optionsWithDefaults), }), }, diff --git a/packages/perseus-editor/src/components/__tests__/widget-editor.test.tsx b/packages/perseus-editor/src/components/__tests__/widget-editor.test.tsx index a1fd860e2c1..bf822c725b3 100644 --- a/packages/perseus-editor/src/components/__tests__/widget-editor.test.tsx +++ b/packages/perseus-editor/src/components/__tests__/widget-editor.test.tsx @@ -8,7 +8,6 @@ import {testDependencies} from "../../testing/test-dependencies"; import {registerAllWidgetsAndEditorsForTesting} from "../../util/register-all-widgets-and-editors-for-testing"; import WidgetEditor from "../widget-editor"; -import type {Alignment} from "@khanacademy/perseus-core"; import type {UserEvent} from "@testing-library/user-event"; describe("WidgetEditor", () => { @@ -37,7 +36,7 @@ describe("WidgetEditor", () => { "getSupportedAlignments", ).mockReturnValue(["block", "inline", "full-width"]); - const {container} = render( + render( { // Assert // Even though getSupportedAlignments returns multiple alignments, // the dropdown should NOT be shown because showAlignmentOptions is false - const alignmentDropdown = - // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access - container.querySelector("select.alignment"); - expect(alignmentDropdown).not.toBeInTheDocument(); + expect( + screen.queryByRole("combobox", {name: "Alignment"}), + ).not.toBeInTheDocument(); }); it("should display alignment dropdown when widget supports multiple alignments", () => { @@ -91,9 +89,10 @@ describe("WidgetEditor", () => { ); // Assert - const dropdown = screen.getByRole("combobox"); + const dropdown = screen.getByRole("combobox", { + name: "Alignment", + }); expect(dropdown).toBeInTheDocument(); - expect(dropdown).toHaveClass("alignment"); }); it("should NOT display alignment dropdown when widget supports only one alignment", () => { @@ -123,45 +122,9 @@ describe("WidgetEditor", () => { ); // Assert - expect(screen.queryByRole("combobox")).not.toBeInTheDocument(); - }); - - it("should display all alignment options in the dropdown", () => { - // Arrange - const alignments: readonly Alignment[] = [ - "block", - "inline", - "full-width", - ]; - jest.spyOn( - CoreWidgetRegistry, - "getSupportedAlignments", - ).mockReturnValue(alignments); - - render( - {}} - onRemove={() => {}} - apiOptions={{ - ...ApiOptions.defaults, - showAlignmentOptions: true, - }} - />, - ); - - // Assert - const options = screen.getAllByRole("option"); - expect(options).toHaveLength(3); - expect(options[0]).toHaveTextContent("block"); - expect(options[1]).toHaveTextContent("inline"); - expect(options[2]).toHaveTextContent("full-width"); + expect( + screen.queryByRole("combobox", {name: "Alignment"}), + ).not.toBeInTheDocument(); }); it("should call onChange when alignment is changed", async () => { @@ -191,8 +154,12 @@ describe("WidgetEditor", () => { ); // Act - const dropdown = screen.getByRole("combobox"); - await userEvent.selectOptions(dropdown, "inline"); + const dropdown = screen.getByRole("combobox", { + name: "Alignment", + }); + await userEvent.click(dropdown); + const option = screen.getByRole("option", {name: "inline"}); + await userEvent.click(option); // Assert expect(onChangeMock).toHaveBeenCalledWith( @@ -229,8 +196,10 @@ describe("WidgetEditor", () => { ); // Assert - const dropdown = screen.getByRole("combobox"); - expect(dropdown).toBeDisabled(); + const dropdown = screen.getByRole("combobox", { + name: "Alignment", + }); + expect(dropdown).toHaveAttribute("aria-disabled", "true"); }); }); }); diff --git a/packages/perseus-editor/src/components/alignment-select.tsx b/packages/perseus-editor/src/components/alignment-select.tsx index 7d81dc8b5fc..162caf258fd 100644 --- a/packages/perseus-editor/src/components/alignment-select.tsx +++ b/packages/perseus-editor/src/components/alignment-select.tsx @@ -1,10 +1,16 @@ import {components} from "@khanacademy/perseus"; +import {View} from "@khanacademy/wonder-blocks-core"; +import {OptionItem, SingleSelect} from "@khanacademy/wonder-blocks-dropdown"; import {sizing} from "@khanacademy/wonder-blocks-tokens"; +import {BodyText} from "@khanacademy/wonder-blocks-typography"; +import {StyleSheet} from "aphrodite"; import * as React from "react"; +import {useId} from "react"; import {alignmentInfoMap} from "./util"; import type {Alignment, PerseusWidget} from "@khanacademy/perseus-core"; +import type {StyleType} from "@khanacademy/wonder-blocks-core"; const {InfoTip} = components; @@ -13,6 +19,7 @@ interface Props { widgetInfo: PerseusWidget; isEditingDisabled: boolean; onChange: (e: React.ChangeEvent) => void; + style?: StyleType; } export function AlignmentSelect({ @@ -20,17 +27,52 @@ export function AlignmentSelect({ widgetInfo, isEditingDisabled, onChange, + style, }: Props) { + const labelId = useId(); return ( - <> + + + Alignment + + { + // Create a synthetic-like event to match the existing + // onChange signature expected by WidgetEditor + const syntheticEvent = { + currentTarget: {value}, + } as React.ChangeEvent; + onChange(syntheticEvent); + }} + placeholder="Select alignment" + style={styles.singleSelectShort} + > + {supportedAlignments.map((alignment) => ( + + ))} +
    {supportedAlignments.map((alignment, index) => (
  • - - + ); } + +const styles = StyleSheet.create({ + singleSelectShort: { + height: sizing.size_260, + }, +}); diff --git a/packages/perseus-editor/src/components/widget-editor-settings.css b/packages/perseus-editor/src/components/widget-editor-settings.css new file mode 100644 index 00000000000..ca80ad1508c --- /dev/null +++ b/packages/perseus-editor/src/components/widget-editor-settings.css @@ -0,0 +1,10 @@ +/* We have to use !important until wonder blocks is in the shared layer. */ +/* TODO(LEMS-3686): Remove the !important once we don't need it anymore. */ +.widget-editor-settings-container { + padding-inline: var(--wb-sizing-size_120) !important; + padding-block-start: var(--wb-sizing-size_120) !important; +} + +.best-practices-container { + margin-block-end: var(--wb-sizing-size_060) !important; +} diff --git a/packages/perseus-editor/src/components/widget-editor-settings.tsx b/packages/perseus-editor/src/components/widget-editor-settings.tsx new file mode 100644 index 00000000000..6e8c71528da --- /dev/null +++ b/packages/perseus-editor/src/components/widget-editor-settings.tsx @@ -0,0 +1,114 @@ +import {isFeatureOn} from "@khanacademy/perseus-core"; +import {View} from "@khanacademy/wonder-blocks-core"; +import Link from "@khanacademy/wonder-blocks-link"; +import {sizing} from "@khanacademy/wonder-blocks-tokens"; +import * as React from "react"; + +import {AlignmentSelect} from "./alignment-select"; +import LabeledSwitch from "./labeled-switch"; + +import "./widget-editor-settings.css"; + +import type {APIOptions} from "@khanacademy/perseus"; +import type {Alignment, PerseusWidget} from "@khanacademy/perseus-core"; + +interface BestPracticesLink { + url: string; + label: string; +} + +interface WidgetEditorSettingsProps { + bestPractices?: BestPracticesLink; + supportsStaticMode: boolean; + isStatic: boolean; + onStaticChange: (value: boolean) => unknown; + supportsGradedToggle: boolean; + isGraded: boolean; + onGradedChange: (value: boolean) => unknown; + supportedAlignments: ReadonlyArray; + widgetInfo: PerseusWidget; + onAlignmentChange: (e: React.SyntheticEvent) => unknown; + isEditingDisabled: boolean; + // TODO(LEMS-3976): clean up feature flag + apiOptions: APIOptions; +} + +function WidgetEditorSettings(props: WidgetEditorSettingsProps) { + const { + bestPractices, + supportsStaticMode, + isStatic, + onStaticChange, + supportsGradedToggle, + isGraded, + onGradedChange, + supportedAlignments, + widgetInfo, + onAlignmentChange, + isEditingDisabled, + // TODO(LEMS-3976): clean up feature flag + apiOptions, + } = props; + + const hasControls = + supportsStaticMode || + supportsGradedToggle || + supportedAlignments.length > 1; + + if (!bestPractices && !hasControls) { + return null; + } + + // TODO(LEMS-3976): clean up feature flag + const notScoredFeatureEnabled = isFeatureOn( + {apiOptions: apiOptions}, + "interactive-graph-not-scored", + ); + + return ( + + {bestPractices && ( + + + {bestPractices.label} + + + )} + {hasControls && ( +
    + {supportsStaticMode && ( + + )} + {notScoredFeatureEnabled && supportsGradedToggle && ( + { + onGradedChange(!e); + }} + style={{marginBlockEnd: sizing.size_060}} + /> + )} + {supportedAlignments.length > 1 && ( + + )} +
    + )} +     + ); +} + +export default WidgetEditorSettings; diff --git a/packages/perseus-editor/src/components/widget-editor.tsx b/packages/perseus-editor/src/components/widget-editor.tsx index 3d92c58c317..81c9f5e37dd 100644 --- a/packages/perseus-editor/src/components/widget-editor.tsx +++ b/packages/perseus-editor/src/components/widget-editor.tsx @@ -5,16 +5,12 @@ import { applyDefaultsToWidget, } from "@khanacademy/perseus-core"; import {View} from "@khanacademy/wonder-blocks-core"; -import {Strut} from "@khanacademy/wonder-blocks-layout"; -import Switch from "@khanacademy/wonder-blocks-switch"; -import {spacing} from "@khanacademy/wonder-blocks-tokens"; import trashIcon from "@phosphor-icons/core/bold/trash-bold.svg"; import * as React from "react"; -import {useId} from "react"; -import {AlignmentSelect} from "./alignment-select"; import SectionControlButton from "./section-control-button"; import ToggleableCaret from "./toggleable-caret"; +import WidgetEditorSettings from "./widget-editor-settings"; import type Editor from "../editor"; import type {APIOptions} from "@khanacademy/perseus"; @@ -102,6 +98,22 @@ class WidgetEditor extends React.Component< const newWidgetInfo = { ...this.state.widgetInfo, static: value, + // if it's "interactive but ungraded" (ungraded) + // we don't also want it to be "non-interactive" (static) + // because "interactive" and "non-interactive" are mutually exclusive concepts + graded: true, + } satisfies PerseusWidget; + this.props.onChange(newWidgetInfo); + }; + + _setGraded = (value: boolean) => { + const newWidgetInfo = { + ...this.state.widgetInfo, + graded: value, + // if it's "interactive but ungraded" (ungraded) + // we don't also want it to be "non-interactive" (static) + // because "interactive" and "non-interactive" are mutually exclusive concepts + static: false, } satisfies PerseusWidget; this.props.onChange(newWidgetInfo); }; @@ -153,6 +165,7 @@ class WidgetEditor extends React.Component< } const supportsStaticMode = Widgets.supportsStaticMode(widgetInfo.type); + const supportsGradedToggle = Widgets.supportsUngraded(widgetInfo.type); return (
    @@ -179,22 +192,6 @@ class WidgetEditor extends React.Component<
    - {supportsStaticMode && ( - - )} - {supportedAlignments.length > 1 && ( - - )} + {this.state.showWidget && ( + + )}
    @@ -225,21 +239,4 @@ class WidgetEditor extends React.Component< } } -function LabeledSwitch(props: { - label: string; - checked: boolean; - onChange: (value: boolean) => unknown; - disabled: boolean; -}) { - const {label, disabled, ...switchProps} = props; - const id = useId(); - return ( - <> - - - - - ); -} - export default WidgetEditor; diff --git a/packages/perseus-editor/src/styles/perseus-editor.css b/packages/perseus-editor/src/styles/perseus-editor.css index 39c3c0791f0..d2f901fdb53 100644 --- a/packages/perseus-editor/src/styles/perseus-editor.css +++ b/packages/perseus-editor/src/styles/perseus-editor.css @@ -108,8 +108,6 @@ code { } .perseus-widget-editor .perseus-widget-editor-title { background-color: #eee; - border: 1px solid #ddd; - border-bottom: 0; font-size: 1.25em; padding: 4px 10px; border-radius: 3px 3px 0 0; @@ -138,8 +136,7 @@ code { } .perseus-widget-editor .perseus-widget-editor-content { border-radius: 0 0 3px 3px; - border-top: 1px solid #ddd; - padding: 10px; + padding: 0 var(--wb-sizing-size_100) var(--wb-sizing-size_100); transition: all 0s; } .perseus-widget-editor .perseus-widget-editor-content.leave { diff --git a/packages/perseus-editor/src/testing/feature-flags-util.ts b/packages/perseus-editor/src/testing/feature-flags-util.ts index 689a1d0649c..5c89469de85 100644 --- a/packages/perseus-editor/src/testing/feature-flags-util.ts +++ b/packages/perseus-editor/src/testing/feature-flags-util.ts @@ -11,6 +11,7 @@ const DEFAULT_FEATURE_FLAGS = { "interactive-graph-logarithm": false, "interactive-graph-exponent": false, "interactive-graph-vector": false, + "interactive-graph-not-scored": false, // ...add new flags here }; diff --git a/packages/perseus-editor/src/testing/test-dependencies.tsx b/packages/perseus-editor/src/testing/test-dependencies.tsx index ab7bd04b764..08f5d1fa315 100644 --- a/packages/perseus-editor/src/testing/test-dependencies.tsx +++ b/packages/perseus-editor/src/testing/test-dependencies.tsx @@ -119,7 +119,7 @@ export const storybookDependenciesV2: PerseusDependenciesV2 = { ...testDependenciesV2, analytics: { onAnalyticsEvent: async (event) => { - console.info("⚡️ Sending analytics event:", event); + console.debug("⚡️ Sending analytics event:", event); }, }, }; diff --git a/packages/perseus-editor/src/widgets/__tests__/interactive-graph-editor.test.tsx b/packages/perseus-editor/src/widgets/__tests__/interactive-graph-editor.test.tsx index 45931f58d17..8c3fe1702ff 100644 --- a/packages/perseus-editor/src/widgets/__tests__/interactive-graph-editor.test.tsx +++ b/packages/perseus-editor/src/widgets/__tests__/interactive-graph-editor.test.tsx @@ -72,7 +72,7 @@ describe("InteractiveGraphEditor", () => { ); // Act - const dropdown = await screen.findByLabelText("Answer type:"); + const dropdown = await screen.findByLabelText("Answer type"); await userEvent.click(dropdown); await userEvent.click(screen.getByRole("option", {name: "Polygon"})); @@ -810,7 +810,7 @@ describe("InteractiveGraphEditor", () => { }, ); - const dropdown = await screen.findByLabelText("Answer type:"); + const dropdown = await screen.findByLabelText("Answer type"); await userEvent.click(dropdown); expect(screen.getByRole("option", {name: "None"})).toBeInTheDocument(); }); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/components/graph-type-selector.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/components/graph-type-selector.tsx index 5fd06c82e8e..02cb8e67347 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/components/graph-type-selector.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/components/graph-type-selector.tsx @@ -1,5 +1,6 @@ import {isFeatureOn} from "@khanacademy/perseus-core"; import {OptionItem, SingleSelect} from "@khanacademy/wonder-blocks-dropdown"; +import {sizing} from "@khanacademy/wonder-blocks-tokens"; import {StyleSheet} from "aphrodite"; import * as React from "react"; @@ -76,9 +77,7 @@ const GraphTypeSelector = (props: GraphTypeSelectorProps) => { const styles = StyleSheet.create({ singleSelectShort: { - // Non-standard spacing, but it's the smallest we can go - // without running into styling issues with the dropdown. - height: 26, + height: sizing.size_260, }, }); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/interactive-graph-editor.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/interactive-graph-editor.tsx index 400257df854..c0b62863d83 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/interactive-graph-editor.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/interactive-graph-editor.tsx @@ -146,6 +146,10 @@ export type Props = { // Graphs in static mode are not interactive, and their coords are // set to those of the "correct" graph in the editor. static?: boolean; + /** + * Whether this widget is graded. + */ + graded?: boolean; }; // JSDoc will be shown in Storybook widget editor description @@ -157,6 +161,12 @@ export type Props = { */ class InteractiveGraphEditor extends React.Component { static widgetName = "interactive-graph"; + static bestPractices = { + // TODO: replace with real best practices + // see: https://github.com/Khan/perseus/pull/3466#discussion_r3157121327 + url: "https://khanacademy.atlassian.net/wiki/spaces/LC/pages/3295281289/Interactive+Graph+Widget#Adding-a-new-Interactive-Graph-Widget", + label: "Interactive Graph best practices", + }; displayName = "InteractiveGraphEditor"; className = "perseus-widget-interactive-graph"; @@ -402,7 +412,7 @@ class InteractiveGraphEditor extends React.Component { {(graphId) => ( - + { onChange={this.changeStartCoords} /> )} + { - const {children, label, labelSide = "left", style} = props; + const { + children, + label, + labelSide = "left", + labelSize = "small", + style, + } = props; return (