Skip to content

Commit

Permalink
Move Numeric Input's UI related code to a functional component (#2108)
Browse files Browse the repository at this point in the history
## Summary:
We have a lot of old patterns in Perseus, and we would like to start to updating to more modern methods. This PR updates the Numeric Input logic in the following ways:

1. Moves all UI Related Numeric Input logic to a functional component, and creates a new `numeric-input.class.tsx` file for housing the static / class methods. 
2. Removes all string refs related to Numeric Input in both the InputWithExamples, SimpleKeypadInput, and Tooltip components
3. Adds a little more specificity to method parameters 

Issue: LEMS-2443

## Test plan:
- Manual Testing 
- Automated Tests 
- Landing onto a feature branch for future QA Regression pass

Author: SonicScrewdriver

Reviewers: SonicScrewdriver, mark-fitzgerald

Required Reviewers:

Approved By: mark-fitzgerald

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x)

Pull Request URL: #2108
  • Loading branch information
SonicScrewdriver authored Jan 16, 2025
1 parent 41ffd4a commit 2d5d63d
Show file tree
Hide file tree
Showing 17 changed files with 592 additions and 440 deletions.
6 changes: 6 additions & 0 deletions .changeset/smart-countries-hunt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/math-input": minor
"@khanacademy/perseus": minor
---

Refactoring Numeric Input to move UI-logic to functional component.
3 changes: 3 additions & 0 deletions packages/math-input/src/components/input/math-input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,9 @@ class MathInput extends React.Component<Props, State> {
});
};

// [Jan 2025] Third: While testing, I've discovered that we likely don't
// need to be passing setKeypadActive here at all. Removing the parameter
// still results in the same behavior.
focus: (setKeypadActive: KeypadContextType["setKeypadActive"]) => void = (
setKeypadActive,
) => {
Expand Down
17 changes: 10 additions & 7 deletions packages/perseus/src/components/input-with-examples.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ class InputWithExamples extends React.Component<Props, State> {
static contextType = PerseusI18nContext;
declare context: React.ContextType<typeof PerseusI18nContext>;

inputRef: React.RefObject<TextInput>;

static defaultProps: DefaultProps = {
shouldShowExamples: true,
onFocus: function () {},
Expand All @@ -65,6 +67,11 @@ class InputWithExamples extends React.Component<Props, State> {
showExamples: false,
};

constructor(props: Props) {
super(props);
this.inputRef = React.createRef<TextInput>();
}

_getUniqueId: () => string = () => {
return `input-with-examples-${btoa(this.props.id).replace(/=/g, "")}`;
};
Expand Down Expand Up @@ -98,7 +105,7 @@ class InputWithExamples extends React.Component<Props, State> {
const inputProps = {
id: id,
"aria-describedby": ariaId,
ref: "input",
ref: this.inputRef,
className: this._getInputClassName(),
labelText: this.props.labelText,
value: this.props.value,
Expand Down Expand Up @@ -148,15 +155,11 @@ class InputWithExamples extends React.Component<Props, State> {
};

focus: () => void = () => {
// eslint-disable-next-line react/no-string-refs
// @ts-expect-error - TS2339 - Property 'focus' does not exist on type 'ReactInstance'.
this.refs.input.focus();
this.inputRef.current?.focus();
};

blur: () => void = () => {
// eslint-disable-next-line react/no-string-refs
// @ts-expect-error - TS2339 - Property 'blur' does not exist on type 'ReactInstance'.
this.refs.input.blur();
this.inputRef.current?.blur();
};

handleChange: (arg1: any) => void = (e) => {
Expand Down
25 changes: 14 additions & 11 deletions packages/perseus/src/components/simple-keypad-input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* interface to `math-input`'s MathInput component.
*/

import {KeypadContext} from "@khanacademy/keypad-context";
import {
KeypadInput,
KeypadType,
Expand All @@ -17,7 +18,15 @@ import PropTypes from "prop-types";
import * as React from "react";

export default class SimpleKeypadInput extends React.Component<any> {
static contextType = KeypadContext;
declare context: React.ContextType<typeof KeypadContext>;
_isMounted = false;
inputRef: React.RefObject<KeypadInput>;

constructor(props: any) {
super(props);
this.inputRef = React.createRef<KeypadInput>();
}

componentDidMount() {
// TODO(scottgrant): This is a hack to remove the deprecated call to
Expand All @@ -30,18 +39,13 @@ export default class SimpleKeypadInput extends React.Component<any> {
}

focus() {
// @ts-expect-error - TS2339 - Property 'focus' does not exist on type 'ReactInstance'.
this.refs.input.focus(); // eslint-disable-line react/no-string-refs
// The inputRef is a ref to a MathInput, which
// also controls the keypad state during focus events.
this.inputRef.current?.focus(this.context.setKeypadActive);
}

blur() {
// eslint-disable-next-line react/no-string-refs
// @ts-expect-error - TS2339 - Property 'blur' does not exist on type 'ReactInstance'.
if (typeof this.refs.input?.blur === "function") {
// eslint-disable-next-line react/no-string-refs
// @ts-expect-error - TS2339 - Property 'blur' does not exist on type 'ReactInstance'.
this.refs.input?.blur();
}
this.inputRef.current?.blur();
}

getValue(): string | number {
Expand All @@ -59,8 +63,7 @@ export default class SimpleKeypadInput extends React.Component<any> {
return (
// @ts-expect-error - TS2769 - No overload matches this call.
<KeypadInput
// eslint-disable-next-line react/no-string-refs
ref="input"
ref={this.inputRef}
keypadElement={keypadElement}
onFocus={() => {
if (keypadElement) {
Expand Down
2 changes: 0 additions & 2 deletions packages/perseus/src/components/tooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -356,8 +356,6 @@ class Tooltip extends React.Component<Props, State> {
{/* The contents of the tooltip */}
<div
className={this.props.className}
// eslint-disable-next-line react/no-string-refs
ref="tooltipContent"
style={{
position: "relative",
top: settings["top"],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type {PerseusNumericInputUserInput} from "../../validation.types";
import type numericInput from "../../widgets/numeric-input/numeric-input";
import type numericInput from "../../widgets/numeric-input/numeric-input.class";
import type React from "react";

export type NumericInputPromptJSON = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ exports[`graded-group-set should render all graded groups 1`] = `
autocapitalize="off"
autocomplete="off"
autocorrect="off"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-input_1y6ajxo"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-inputWithExamples_1y6ajxo"
id="input-with-examples-bnVtZXJpYy1pbnB1dCAx"
type="text"
value=""
Expand Down Expand Up @@ -311,7 +311,7 @@ exports[`graded-group-set should render all graded groups 1`] = `
autocapitalize="off"
autocomplete="off"
autocorrect="off"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-input_1y6ajxo"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-inputWithExamples_1y6ajxo"
id="input-with-examples-bnVtZXJpYy1pbnB1dCAx"
type="text"
value=""
Expand Down Expand Up @@ -553,7 +553,7 @@ exports[`graded-group-set should render all graded groups 1`] = `
autocapitalize="off"
autocomplete="off"
autocorrect="off"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-input_1y6ajxo"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-inputWithExamples_1y6ajxo"
id="input-with-examples-bnVtZXJpYy1pbnB1dCAx"
type="text"
value=""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ exports[`graded group widget should snapshot 1`] = `
autocapitalize="off"
autocomplete="off"
autocorrect="off"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-input_1y6ajxo"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-inputWithExamples_1y6ajxo"
id="input-with-examples-bnVtZXJpYy1pbnB1dCAx"
type="text"
value=""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ exports[`group widget should snapshot: initial render 1`] = `
autocapitalize="off"
autocomplete="off"
autocorrect="off"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-input_1y6ajxo"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-inputWithExamples_1y6ajxo"
id="input-with-examples-bnVtZXJpYy1pbnB1dCAx"
type="text"
value=""
Expand Down Expand Up @@ -982,7 +982,7 @@ exports[`group widget should snapshot: initial render 1`] = `
autocapitalize="off"
autocomplete="off"
autocorrect="off"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-input_1y6ajxo"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-inputWithExamples_1y6ajxo"
id="input-with-examples-bnVtZXJpYy1pbnB1dCAy"
type="text"
value=""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,31 +27,33 @@ exports[`Numeric input widget styles differently on mobile: mobile render 1`] =
<div
class="perseus-widget-container widget-nohighlight widget-inline-block"
>
<div
aria-label="Math input box Tap with one or two fingers to open keyboard"
class="initial_4qg14c-o_O-input_1n1c032"
role="textbox"
>
<div>
<div
class="keypad-input"
tabindex="0"
aria-label="Math input box Tap with one or two fingers to open keyboard"
class="initial_4qg14c-o_O-input_1n1c032"
role="textbox"
>
<div
class="mq-editable-field mq-math-mode"
style="background-color: white; min-height: 44px; min-width: 64px; max-width: 128px; box-sizing: border-box; position: relative; border-style: solid; border-color: rgba(33,36,44,0.50); border-radius: 4px; color: rgb(33, 36, 44); border-width: 1px;"
class="keypad-input"
tabindex="0"
>
<span
class="mq-textarea"
<div
class="mq-editable-field mq-math-mode"
style="background-color: white; min-height: 44px; min-width: 64px; max-width: 128px; box-sizing: border-box; position: relative; border-style: solid; border-color: rgba(33,36,44,0.50); border-radius: 4px; color: rgb(33, 36, 44); border-width: 1px;"
>
<span
aria-label="Math Input:"
class="mq-textarea"
>
<span
aria-label="Math Input:"
/>
</span>
<span
aria-hidden="true"
class="mq-root-block mq-empty"
style="padding: 10px 11px 8px 11px; font-size: 18pt;"
/>
</span>
<span
aria-hidden="true"
class="mq-root-block mq-empty"
style="padding: 10px 11px 8px 11px; font-size: 18pt;"
/>
</div>
</div>
</div>
</div>
Expand Down Expand Up @@ -100,7 +102,7 @@ exports[`numeric-input widget Should render predictably: after interaction 1`] =
autocapitalize="off"
autocomplete="off"
autocorrect="off"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-input_ylhnsi"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-inputWithExamples_ylhnsi"
id="input-with-examples-bnVtZXJpYy1pbnB1dCAx"
type="text"
value="1252"
Expand Down Expand Up @@ -317,7 +319,7 @@ exports[`numeric-input widget Should render predictably: first render 1`] = `
autocapitalize="off"
autocomplete="off"
autocorrect="off"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-input_1y6ajxo"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-inputWithExamples_1y6ajxo"
id="input-with-examples-bnVtZXJpYy1pbnB1dCAx"
type="text"
value=""
Expand Down Expand Up @@ -534,7 +536,7 @@ exports[`numeric-input widget Should render tooltip as list when multiple format
autocapitalize="off"
autocomplete="off"
autocorrect="off"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-input_1y6ajxo"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-inputWithExamples_1y6ajxo"
id="input-with-examples-bnVtZXJpYy1pbnB1dCAx"
type="text"
value=""
Expand Down Expand Up @@ -698,7 +700,7 @@ exports[`numeric-input widget Should render tooltip when format option is given:
autocapitalize="off"
autocomplete="off"
autocorrect="off"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-input_1y6ajxo"
class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-inputWithExamples_1y6ajxo"
id="input-with-examples-bnVtZXJpYy1pbnB1dCAx"
type="text"
value=""
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus/src/widgets/numeric-input/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export {default} from "./numeric-input";
export {default} from "./numeric-input.class";
Loading

0 comments on commit 2d5d63d

Please sign in to comment.