Skip to content
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

[LEMS-2852] Answerless Expression #2226

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

handeyeco
Copy link
Contributor

@handeyeco handeyeco commented Feb 11, 2025

Summary:

So the main thing with the Expression widget is that it was using answer data to determine which extraKeys to show in the MathInput keypad. For instance if a possible answer was 42i, it would show i in the Extra tab on the keypad.

This PR sets us up for removing answers by adding extraKeys to the data schema so it can be determined at publish time rather than read time. We simulate this by upgrading the widget to provide extraKeys when not present on the data.

Issue: LEMS-2852

Test plan:

  • Go to an Expression widget that has a constant/variable in the answer
  • It should render the same, including having the constant/variable in the keypad
  • It should be answerable/scorable

@handeyeco handeyeco self-assigned this Feb 11, 2025
Copy link
Contributor

github-actions bot commented Feb 11, 2025

Size Change: +137 B (+0.02%)

Total Size: 871 kB

Filename Size Change
packages/perseus-core/dist/es/index.js 30.4 kB +831 B (+2.81%)
packages/perseus/dist/es/index.js 367 kB -694 B (-0.19%)
ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 39.5 kB
packages/keypad-context/dist/es/index.js 760 B
packages/kmath/dist/es/index.js 11.1 kB
packages/math-input/dist/es/index.js 78 kB
packages/math-input/dist/es/strings.js 1.79 kB
packages/perseus-editor/dist/es/index.js 276 kB
packages/perseus-linter/dist/es/index.js 22.8 kB
packages/perseus-score/dist/es/index.js 20.3 kB
packages/perseus/dist/es/strings.js 6.57 kB
packages/pure-markdown/dist/es/index.js 4.14 kB
packages/simple-markdown/dist/es/index.js 13.1 kB

compressed-size-action

@handeyeco handeyeco changed the title [LEMS-2852/expression-answerless] make transform work with both forms of data LEMS-2852 Answerless Expression Feb 11, 2025
@handeyeco handeyeco changed the title LEMS-2852 Answerless Expression [LEMS-2852] Answerless Expression Feb 11, 2025
Comment on lines 842 to 844
"extraKeys": [
"PI",
],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe instead of having deriveExtraKeys default to ["PI"] it should just be empty and the props can default to ["PI"] when rendering...

@@ -0,0 +1,77 @@
import * as KAS from "@khanacademy/kas";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just need to work with Jeremy to figure out this import stuff. There's a weird issue with a circular dependency that I wasn't able to figure out.

expect(score).toHaveBeenAnsweredCorrectly();
});

it("is interactive with answerless widget options", async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the important one. Without answer data we:

  • Render
  • Answer
  • Send the input off for scoring

* to be included as keys on the keypad. These are scraped from the answer
* forms.
*/
export const keypadConfigurationForProps = (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of this logic was for deriving extra keys, so I just transformed this into deriveExtraKeys.

buttonsVisible: v0options.buttonsVisible,
visibleLabel: v0options.visibleLabel,
ariaLabel: v0options.ariaLabel,
extraKeys: v0options.extraKeys,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove this

extraKeys: v1options.extraKeys || deriveExtraKeys(v1options),
};
},
"1": (v0options: any): PerseusExpressionWidgetOptions => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This probably shouldn't return PerseusExpressionWidgetOptions

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

Successfully merging this pull request may close these issues.

1 participant