Skip to content

Commit

Permalink
remove onInputError
Browse files Browse the repository at this point in the history
  • Loading branch information
handeyeco committed Sep 24, 2024
1 parent c04d275 commit 4cfad41
Show file tree
Hide file tree
Showing 11 changed files with 23 additions and 119 deletions.
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

0 comments on commit 4cfad41

Please sign in to comment.