Skip to content

CSProgram: renderer user input state #2584

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: renderer-user-input-core
Choose a base branch
from

Conversation

handeyeco
Copy link
Contributor

@handeyeco handeyeco commented Jun 12, 2025

Summary:

See the parent PR for more context. Part of LEMS-3208.

In #2566 I add additional APIs to let widgets consume userInput from a parent and to update that parent state using handleUserInput, while also supporting the legacy way of storing user input (which was a combination of internal widget state and stashing state in a random blob in Renderer). This PR is part of a series of PRs that go widget-by-widget to use the new API.

This is a generic summary I'm putting on each PR and not every point will be applicable to every widget, but the common themes are:

  • I added a test for serialization to make sure we're still backwards compatible (see LEMS-3185), then implemented the helpers getSerializedState and getUserInputFromSerializedState to keep supporting the expected results.
  • I moved user input for the component into the new userInput state in Renderer. This means consuming userInput in the component and calling handleUserInput on change.
  • When transform did something to initialize user input state, I moved the logic to getStartUserInput.
  • When staticTransform did something to get correct state in static widgets, I moved the logic to getCorrectUserInput.
  • Editors that use a widget to collect answer data are changed to support the new API.

Please see the parent PR for more information.


CSProgramEditor doesn't use CSProgram, so this shouldn't affect the editing experience

Copy link
Contributor

github-actions bot commented Jun 12, 2025

Size Change: +26 B (+0.01%)

Total Size: 473 kB

Filename Size Change
packages/perseus/dist/es/index.js 201 kB +26 B (+0.01%)
ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 20.7 kB
packages/keypad-context/dist/es/index.js 1 kB
packages/kmath/dist/es/index.js 5.98 kB
packages/math-input/dist/es/index.js 98.6 kB
packages/math-input/dist/es/strings.js 1.61 kB
packages/perseus-core/dist/es/index.js 20.5 kB
packages/perseus-editor/dist/es/index.js 91.1 kB
packages/perseus-linter/dist/es/index.js 7.14 kB
packages/perseus-score/dist/es/index.js 9.23 kB
packages/perseus-utils/dist/es/index.js 403 B
packages/perseus/dist/es/strings.js 7.56 kB
packages/pure-markdown/dist/es/index.js 1.22 kB
packages/simple-markdown/dist/es/index.js 6.71 kB

compressed-size-action

Copy link
Contributor

github-actions bot commented Jun 12, 2025

npm Snapshot: Published

Good news!! We've packaged up the latest commit from this PR (df5f178) and published it to npm. You
can install it using the tag PR2584.

Example:

pnpm add @khanacademy/perseus@PR2584

If you are working in Khan Academy's frontend, you can run the below command.

./tools/bump_perseus_version.js -t PR2584

If you are working in Khan Academy's webapp, you can run the below command.

./dev/tools/bump_perseus_version.js -s n -t PR2584

@handeyeco handeyeco marked this pull request as ready for review June 16, 2025 15:16
@handeyeco handeyeco requested review from SonicScrewdriver, Myranae and a team June 16, 2025 15:17
Copy link
Contributor

@SonicScrewdriver SonicScrewdriver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More great improvements. I'm really happy to see another usage of changeable get removed - gosh was it adding a needless mess.

@@ -8,7 +8,6 @@ import * as React from "react";
import _ from "underscore";

import {getDependencies} from "../../dependencies";
import * as Changeable from "../../mixins/changeable";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YES THANK YOU. Little by little, we're finally getting rid of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants