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

Remove onInputError #1661

Merged
merged 2 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/tough-snails-double.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": major
"@khanacademy/perseus-editor": patch
---

Remove unused onInputError from APIOptions
3 changes: 1 addition & 2 deletions packages/perseus-editor/src/iframe-content-renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,7 @@ class IframeContentRenderer extends React.Component<Props> {
this._lastData = data;

// We can't use JSON.stringify/parse for this because the apiOptions
// includes the functions GroupMetadataEditor, groupAnnotator,
// onFocusChange, and onInputError.
// includes the functions GroupMetadataEditor, groupAnnotator, and onFocusChange.
// @ts-expect-error - TS2339 - Property 'iframeDataStore' does not exist on type 'Window & typeof globalThis'.
window.iframeDataStore[this.iframeID] = data;
frame.contentWindow.postMessage(this.iframeID, "*");
Expand Down
14 changes: 0 additions & 14 deletions packages/perseus/src/__tests__/renderer-api.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -126,20 +126,6 @@ describe("Perseus API", function () {
});
});

describe("onInputError", function () {
it("should call a callback when grading an empty input-number", function () {
let wasCalled;
const {renderer} = renderQuestion(inputNumber1Item.question, {
onInputError: function (widgetId) {
wasCalled = true;
},
});

expect(renderer).toHaveInvalidInput();
expect(wasCalled).toBe(true);
});
});

describe("CSS ClassNames", function () {
describe("perseus-focused", function () {
it("should be on an input-number exactly when focused", async function () {
Expand Down
4 changes: 0 additions & 4 deletions packages/perseus/src/perseus-api.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
* * nothing if it is purely a bug fix.
*
* Callbacks passed to Renderer/ItemRenderer:
* * onInputError:
* Called when there is an error grading a widget
* * onFocusChange: (newFocusPath, oldFocusPath, keypadDOMNode)
* Called when the user focus changes. The first two parameters are `path`
* arrays uniquely identifying the respect inputs. The third parameter,
Expand Down Expand Up @@ -40,7 +38,6 @@ export const ApiOptions = {
propTypes: PropTypes.shape({
isArticle: PropTypes.bool.isRequired,

onInputError: PropTypes.func.isRequired,
onFocusChange: PropTypes.func.isRequired,
GroupMetadataEditor: PropTypes.func.isRequired,
showAlignmentOptions: PropTypes.bool.isRequired,
Expand Down Expand Up @@ -139,7 +136,6 @@ export const ApiOptions = {
defaults: {
isArticle: false,
isMobile: false,
onInputError: function () {},
onFocusChange: function () {},
GroupMetadataEditor: StubTagEditor,
showAlignmentOptions: false,
Expand Down
9 changes: 4 additions & 5 deletions packages/perseus/src/renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1809,7 +1809,6 @@ class Renderer extends React.Component<Props, State> {
[widgetId: string]: PerseusScore;
} = () => {
const widgetProps = this.state.widgetInfo;
const onInputError = this.getApiOptions().onInputError;

const gradedWidgetIds = _.filter(this.widgetIds, (id) => {
const props = widgetProps[id];
Expand All @@ -1826,10 +1825,10 @@ class Renderer extends React.Component<Props, State> {
const widget = this.getWidgetInstance(id);
// widget can be undefined if it hasn't yet been rendered
if (widget && widget.simpleValidate) {
widgetScores[id] = widget.simpleValidate(
{...props?.options, scoring: true},
onInputError,
);
widgetScores[id] = widget.simpleValidate({
...props?.options,
scoring: true,
});
}
});

Expand Down
6 changes: 0 additions & 6 deletions packages/perseus/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,6 @@ export const InteractiveGraphLockedFeaturesFlags = [
*/
export type APIOptions = Readonly<{
isArticle?: boolean;
onInputError?: (
widgetId: any,
value: string,
message?: string | null | undefined,
) => unknown;
onFocusChange?: (
newFocusPath: FocusPath,
oldFocusPath: FocusPath,
Expand Down Expand Up @@ -447,7 +442,6 @@ export type APIOptionsWithDefaults = Readonly<
isArticle: NonNullable<APIOptions["isArticle"]>;
isMobile: NonNullable<APIOptions["isMobile"]>;
onFocusChange: NonNullable<APIOptions["onFocusChange"]>;
onInputError: NonNullable<APIOptions["onInputError"]>;
readOnly: NonNullable<APIOptions["readOnly"]>;
setDrawingAreaAvailable: NonNullable<
APIOptions["setDrawingAreaAvailable"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,19 @@ import {expressionItem3Options} from "./expression.testdata";

describe("expression-validator", () => {
it("should handle defined ungraded answer case with no error callback", function () {
const err = validate(
"x+1",
expressionItem3Options,
undefined,
mockStrings,
"en",
);
const err = validate("x+1", expressionItem3Options, mockStrings, "en");
expect(err).toHaveInvalidInput();
});

it("should handle invalid expression answer with no error callback", function () {
const err = validate(
"x+^1",
expressionItem3Options,
undefined,
mockStrings,
"en",
);
const err = validate("x+^1", expressionItem3Options, mockStrings, "en");
expect(err).toHaveInvalidInput();
});

it("should handle listed incorrect answers as wrong", function () {
const result = validate(
"y+1",
expressionItem3Options,
undefined,
mockStrings,
"en",
);
Expand All @@ -41,7 +28,6 @@ describe("expression-validator", () => {
const result = validate(
"2+2",
expressionItem3Options,
undefined,
mockStrings,
"en",
);
Expand All @@ -52,7 +38,6 @@ describe("expression-validator", () => {
const result = validate(
"z+1",
expressionItem3Options,
undefined,
mockStrings,
"en",
);
Expand All @@ -64,7 +49,6 @@ describe("expression-validator", () => {
const result1 = validate(
"z+1",
expressionItem3Options,
undefined,
mockStrings,
"en",
);
Expand All @@ -74,7 +58,6 @@ describe("expression-validator", () => {
const result2 = validate(
"a+1",
expressionItem3Options,
undefined,
mockStrings,
"en",
);
Expand All @@ -85,7 +68,6 @@ describe("expression-validator", () => {
const result = validate(
"z+1.0",
expressionItem3Options,
undefined,
mockStrings,
"en",
);
Expand All @@ -96,7 +78,6 @@ describe("expression-validator", () => {
const result = validate(
"z+1,0",
expressionItem3Options,
undefined,
mockStrings,
"fr",
);
Expand All @@ -107,7 +88,6 @@ describe("expression-validator", () => {
const result = validate(
"z+1,0",
expressionItem3Options,
undefined,
mockStrings,
"en",
);
Expand Down
13 changes: 2 additions & 11 deletions packages/perseus/src/widgets/expression/expression-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import KhanAnswerTypes from "../../util/answer-types";

import getDecimalSeparator from "./get-decimal-separator";

import type {Rubric, OnInputErrorFunctionType} from "./expression.types";
import type {Rubric} from "./expression.types";
import type {PerseusExpressionAnswerForm} from "../../perseus-types";
import type {PerseusStrings} from "../../strings";
import type {PerseusScore} from "../../types";
Expand All @@ -34,8 +34,6 @@ import type {Score} from "../../util/answer-types";
function expressionValidator(
userInput: string,
rubric: Rubric,
// @ts-expect-error - TS2322 - Type '() => void' is not assignable to type 'OnInputErrorFunctionType'.
onInputError: OnInputErrorFunctionType = function () {},
strings: PerseusStrings,
locale: string,
): PerseusScore {
Expand Down Expand Up @@ -150,16 +148,9 @@ function expressionValidator(
};
}
if (matchingAnswerForm.considered === "ungraded") {
// We matched an ungraded answer form - return "invalid", which
// will let the user try again with no penalty
const apiResult = onInputError(
null, // Reserved for some widget identifier
userInput,
matchMessage,
);
return {
type: "invalid",
message: apiResult === false ? null : matchMessage,
message: matchMessage,
};
}
// We matched a graded answer form, so we can now tell the user
Expand Down
34 changes: 9 additions & 25 deletions packages/perseus/src/widgets/expression/expression.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import a11y from "../../util/a11y";
import expressionValidator from "./expression-validator";
import getDecimalSeparator from "./get-decimal-separator";

import type {Rubric, OnInputErrorFunctionType} from "./expression.types";
import type {Rubric} from "./expression.types";
import type {DependenciesContext} from "../../dependencies";
import type {PerseusExpressionWidgetOptions} from "../../perseus-types";
import type {PerseusStrings} from "../../strings";
Expand Down Expand Up @@ -125,18 +125,10 @@ export class Expression extends React.Component<Props, ExpressionState> {
static validate(
userInput: string,
rubric: Rubric,
// @ts-expect-error - TS2322 - Type '() => void' is not assignable to type 'OnInputErrorFunctionType'.
onInputError: OnInputErrorFunctionType = function () {},
strings: PerseusStrings,
locale: string,
): PerseusScore {
return expressionValidator(
userInput,
rubric,
onInputError,
strings,
locale,
);
return expressionValidator(userInput, rubric, strings, locale);
}

static getUserInputFromProps(props: Props): string {
Expand Down Expand Up @@ -209,16 +201,9 @@ export class Expression extends React.Component<Props, ExpressionState> {
showErrorStyle: false,
});
if (!this.parse(this.props.value, this.props).parsed) {
const apiResult = this.props.apiOptions.onInputError(
null, // reserved for some widget identifier
this.props.value,
this.context.strings.ERROR_TITLE,
);
if (apiResult !== false) {
this.setState({
invalid: true,
});
}
this.setState({
invalid: true,
});
}
}
};
Expand All @@ -235,14 +220,13 @@ export class Expression extends React.Component<Props, ExpressionState> {
}
};

simpleValidate: (
rubric: Rubric & {scoring?: boolean},
onInputError: OnInputErrorFunctionType,
) => PerseusScore = ({scoring, ...rubric}, onInputError) => {
simpleValidate: (rubric: Rubric & {scoring?: boolean}) => PerseusScore = ({
scoring,
...rubric
}) => {
const score = expressionValidator(
this.getUserInput(),
rubric,
onInputError || function () {},
this.context.strings,
this.context.locale,
);
Expand Down
6 changes: 0 additions & 6 deletions packages/perseus/src/widgets/expression/expression.types.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
import type {PerseusExpressionWidgetOptions} from "../../perseus-types";

export type Rubric = PerseusExpressionWidgetOptions;

export type OnInputErrorFunctionType = (
arg1?: any,
arg2?: any,
arg3?: any,
) => boolean | null | undefined;
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import KhanAnswerTypes from "../../util/answer-types";

import type {Rubric} from "./input-number.types";
import type {PerseusStrings} from "../../strings";
import type {APIOptions, PerseusScore} from "@khanacademy/perseus";
import type {PerseusScore} from "@khanacademy/perseus";

const ParseTex = TexWrangler.parseTex;

Expand Down Expand Up @@ -48,7 +48,6 @@ function inputNumberValidator(
},
rubric: Rubric,
strings: PerseusStrings,
onInputError: APIOptions["onInputError"] = () => {},
): PerseusScore {
if (rubric.answerType == null) {
rubric.answerType = "number";
Expand Down Expand Up @@ -78,15 +77,9 @@ function inputNumberValidator(
// TODO(eater): Seems silly to translate result to this invalid/points
// thing and immediately translate it back in ItemRenderer.scoreInput()
if (result.empty) {
// TODO(FEI-3867): remove null-check once we have APIOptionsInternal
const apiResult = onInputError?.(
null, // reserved for some widget identifier
state.currentValue,
result.message,
);
return {
type: "invalid",
message: apiResult === false ? null : result.message,
message: result.message,
};
}
return {
Expand Down
18 changes: 3 additions & 15 deletions packages/perseus/src/widgets/input-number/input-number.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,7 @@ import inputNumberValidator, {answerTypes} from "./input-number-validator";
import type {Rubric} from "./input-number.types";
import type {PerseusInputNumberWidgetOptions} from "../../perseus-types";
import type {PerseusStrings} from "../../strings";
import type {
APIOptions,
Path,
PerseusScore,
WidgetExports,
WidgetProps,
} from "../../types";
import type {Path, PerseusScore, WidgetExports, WidgetProps} from "../../types";

const formExamples = {
integer: function (options, strings: PerseusStrings) {
Expand Down Expand Up @@ -104,9 +98,8 @@ class InputNumber extends React.Component<Props> {
},
rubric: Rubric,
strings: PerseusStrings,
onInputError: APIOptions["onInputError"] = () => {},
): PerseusScore {
return inputNumberValidator(state, rubric, strings, onInputError);
return inputNumberValidator(state, rubric, strings);
}

static getUserInputFromProps(props: Props): {
Expand Down Expand Up @@ -200,16 +193,11 @@ class InputNumber extends React.Component<Props> {
return InputNumber.getUserInputFromProps(this.props);
};

simpleValidate: (
rubric: Rubric,
onInputError?: APIOptions["onInputError"],
) => PerseusScore = (rubric, onInputError) => {
onInputError = onInputError || function () {};
simpleValidate: (rubric: Rubric) => PerseusScore = (rubric) => {
return inputNumberValidator(
this.getUserInput(),
rubric,
this.context.strings,
onInputError,
);
};

Expand Down