Skip to content

Commit

Permalink
HintEditor: migrate to React refs (#1558)
Browse files Browse the repository at this point in the history
## Summary:

This PR migrates the HintEditor to use React refs instead of legacy string refs. This improves safety.

Issue: LEMS-1809

## Test plan:

`yarn tsc`
`yarn test`

Author: jeremywiebe

Reviewers: benchristel, nishasy, jeremywiebe

Required Reviewers:

Approved By: nishasy, benchristel

Checks: ✅ codecov/project, ❌ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1558
  • Loading branch information
jeremywiebe authored Aug 29, 2024
1 parent 9e18d81 commit b4b9430
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 28 deletions.
5 changes: 5 additions & 0 deletions .changeset/big-plums-confess.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus-editor": patch
---

HintEditor: migrate to React refs
44 changes: 16 additions & 28 deletions packages/perseus-editor/src/hint-editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ type CombinedHintsEditorProps = {
*/
class CombinedHintsEditor extends React.Component<CombinedHintsEditorProps> {
static HintEditor: typeof HintEditor = HintEditor;
hintEditors: Record<number, CombinedHintEditor | null> = {};

static defaultProps: {
highlightLint: boolean;
Expand Down Expand Up @@ -353,9 +354,7 @@ class CombinedHintsEditor extends React.Component<CombinedHintsEditorProps> {
const hint = hints.splice(i, 1)[0];
hints.splice(i + dir, 0, hint);
this.props.onChange({hints: hints}, () => {
// eslint-disable-next-line react/no-string-refs
// @ts-expect-error - TS2339 - Property 'focus' does not exist on type 'ReactInstance'.
this.refs["hintEditor" + (i + dir)].focus();
this.hintEditors[i + dir]?.focus();
});
};

Expand All @@ -365,42 +364,29 @@ class CombinedHintsEditor extends React.Component<CombinedHintsEditorProps> {
]);
this.props.onChange({hints: hints}, () => {
const i = hints.length - 1;
// eslint-disable-next-line react/no-string-refs
// @ts-expect-error - TS2339 - Property 'focus' does not exist on type 'ReactInstance'.
this.refs["hintEditor" + i].focus();
this.hintEditors[i]?.focus();
});
};

getSaveWarnings: () => any = () => {
return _.chain(this.props.hints)
return this.props.hints
.map((hint, i) => {
return _.map(
// eslint-disable-next-line react/no-string-refs
// @ts-expect-error - TS2339 - Property 'getSaveWarnings' does not exist on type 'ReactInstance'.
this.refs["hintEditor" + i].getSaveWarnings(),
(issue) => "Hint " + (i + 1) + ": " + issue,
);
return this.hintEditors[i]
?.getSaveWarnings()
.map((issue) => "Hint " + (i + 1) + ": " + issue);
})
.flatten(true)
.value();
.flat();
};

serialize: (options?: any) => ReadonlyArray<PerseusRenderer> = (
options: any,
) => {
serialize(options?: any): ReadonlyArray<PerseusRenderer> {
return this.props.hints.map((hint, i) => {
return this.serializeHint(i, options);
});
};
}

serializeHint: (index: number, options?: any) => PerseusRenderer = (
index,
options,
) => {
// eslint-disable-next-line react/no-string-refs
// @ts-expect-error - TS2339 - Property 'serialize' does not exist on type 'ReactInstance'.
return this.refs["hintEditor" + index].serialize(options);
};
serializeHint(index: number, options?: any): PerseusRenderer {
return this.hintEditors[index]!.serialize(options)!;
}

render(): React.ReactNode {
const {itemId, hints} = this.props;
Expand All @@ -409,7 +395,9 @@ class CombinedHintsEditor extends React.Component<CombinedHintsEditorProps> {
(hint, i) => {
return (
<CombinedHintEditor
ref={"hintEditor" + i}
ref={(editor) => {
this.hintEditors[i] = editor;
}}
key={"hintEditor" + i}
isFirst={i === 0}
isLast={i + 1 === hints.length}
Expand Down

0 comments on commit b4b9430

Please sign in to comment.