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 deprecated/unused support for examples in Renderer (keeping for numeric-input though) #1961

Merged
merged 4 commits into from
Dec 6, 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
5 changes: 5 additions & 0 deletions .changeset/orange-wombats-destroy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": major
---

Remove deprecated/unused `examples()` function from `Renderer`
68 changes: 0 additions & 68 deletions packages/perseus/src/__tests__/renderer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1793,72 +1793,4 @@ describe("renderer", () => {
expect(Object.keys(json.widgets)).toEqual(widgetKeys);
});
});

describe("examples", () => {
it("should return examples if all widgets return the same examples (or null)", () => {
// Arrange
const {renderer} = renderQuestion({
content:
"Input widget: [[\u2603 input-number 1]]\n\n" +
"Dropdown widget: [[\u2603 dropdown 1]]\n\n" +
"Image widget (won't have user input): [[\u2603 image 1]]\n\n" +
"Another input widget: [[\u2603 input-number 2]]",
widgets: {
"image 1": imageWidget,
"input-number 1": inputNumberWidget,
"input-number 2": inputNumberWidget,
"dropdown 1": dropdownWidget,
},
images: {},
});

// Act
const examples = renderer.examples();

// Assert
expect(examples).toMatchInlineSnapshot(`
[
"**Your answer should be** ",
"an integer, like $6$",
"a *proper* fraction, like $1/2$ or $6/10$",
"an *improper* fraction, like $10/7$ or $14/8$",
"a mixed number, like $1\\ 3/4$",
]
`);
});

it("should return nothing if widgets return the different examples", () => {
// NOTE(jeremy): I'm unsure why we don't return examples if the
// examples aren't the same, but this is current functionality so
// I'm adding this test to verify the current behaviour.

// Arrange
const {renderer} = renderQuestion({
content:
"Input widget: [[\u2603 input-number 1]]\n\n" +
"Dropdown widget: [[\u2603 dropdown 1]]\n\n" +
"Image widget (won't have user input): [[\u2603 image 1]]\n\n" +
"Another input widget: [[\u2603 input-number 2]]",
widgets: {
"image 1": imageWidget,
"input-number 1": inputNumberWidget,
"input-number 2": {
...inputNumberWidget,
options: {
...inputNumberWidget.options,
answerType: "percent",
},
},
"dropdown 1": dropdownWidget,
},
images: {},
});

// Act
const examples = renderer.examples();

// Assert
expect(examples).toBeNull();
});
});
});
29 changes: 0 additions & 29 deletions packages/perseus/src/renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1751,35 +1751,6 @@ class Renderer
return [totalGuess, totalScore];
};

examples: () => ReadonlyArray<string> | null | undefined = () => {
const widgetIds = this.widgetIds;
const examples = widgetIds
.map((widgetId) => {
const widget = this.getWidgetInstance(widgetId);
return widget != null && widget.examples
? widget.examples()
: null;
})
.filter(Boolean);

// no widgets with examples
if (!examples.length) {
return null;
}

const allEqual = _.all(examples, function (example) {
return _.isEqual(examples[0], example);
});

// some widgets have different examples
// TODO(alex): handle this better
if (!allEqual) {
return null;
}

return examples[0];
};

// TranslationLinter callback
handletranslationLintErrors: (lintErrors: ReadonlyArray<string>) => void = (
lintErrors: ReadonlyArray<string>,
Expand Down
1 change: 0 additions & 1 deletion packages/perseus/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ export interface Widget {
getUserInput?: () => UserInputArray | UserInput | undefined;

showRationalesForCurrentlySelectedChoices?: (options?: any) => void;
examples?: () => ReadonlyArray<string>;
getPromptJSON?: () => WidgetPromptJSON;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/perseus/src/widgets/input-number/input-number.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ class InputNumber extends React.Component<Props> implements Widget {
return _getPromptJSON(this.props, this.getUserInput());
}

examples: () => ReadonlyArray<string> = () => {
examples(): ReadonlyArray<string> {
const {strings} = this.context;
const type = this.props.answerType;
const forms = answerTypes[type].forms.split(/\s*,\s*/);
Expand All @@ -185,7 +185,7 @@ class InputNumber extends React.Component<Props> implements Widget {
);

return [strings.yourAnswer].concat(examples);
};
}

render(): React.ReactNode {
if (this.props.apiOptions.customKeypad) {
Expand Down
9 changes: 6 additions & 3 deletions packages/perseus/src/widgets/numeric-input/numeric-input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,11 @@ export class NumericInput
isFocused: false,
};

// TODO(Nicole, Jeremy): This is maybe never used and should be removed
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is used, but only internal to the widget.

examples: () => ReadonlyArray<string> = () => {
/**
* Generates a string that demonstrates how to input the various supported
* answer forms.
*/
examples(): ReadonlyArray<string> {
// if the set of specified forms are empty, allow all forms
const forms =
this.props.answerForms?.length !== 0
Expand All @@ -144,7 +147,7 @@ export class NumericInput
examples = _.uniq(examples);

return [this.context.strings.yourAnswer].concat(examples);
};
}

shouldShowExamples: () => boolean = () => {
const noFormsAccepted = this.props.answerForms?.length === 0;
Expand Down
Loading