Modernization and Migration of InputWithExamples to NumericInput folder #2121
Conversation
SonicScrewdriver
changed the title
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (054c29c) and published it to npm. You Example: yarn add @khanacademy/perseus@PR2121If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR2121 |
|
Size Change: +74 B (+0.01%) Total Size: 1.47 MB
ℹ️ View Unchanged
|
| const [state, setState] = React.useState<State>({ | ||
| focused: false, | ||
| showExamples: false, | ||
| }); |
There was a problem hiding this comment.
These two variables are always set together, so I've opted to keep this pairing after the shift to the functional component. I wasn't sure of a good name to explain both of them so I went "generic", but I could easily see an argument to rename this to something like "setFocusStates".
There was a problem hiding this comment.
...Wait. If these are always set the exact same thing in multiple places ... why do we even need both? I bet we could just use "focused".
There was a problem hiding this comment.
I've updated this to just use a single inputFocused state.
There was a problem hiding this comment.
I opted to keep this a separate file as it seemed like too long a component to append to the numeric-input file.
| focus: () => { | ||
| if (inputRef.current) { | ||
| inputRef.current.focus(); | ||
| inputRef.current?.focus(); |
There was a problem hiding this comment.
Is the optional (?) needed when we have the code within a conditional that checks for its existence? I thought that Typescript was able to understand that.
There was a problem hiding this comment.
Oooo I actually meant to replace the if conditional! Thank you for catching this
…NumericInput folder
SonicScrewdriver
force-pushed
the
input-examples-to-numeric
branch
from
64d86ca to
054c29c
Compare
SonicScrewdriver
merged commit e603970
into
feature/numeric-dx-refactor
…er (#2121) ## Summary: This PR is part of the Numeric Input Project work. It is being landed onto the `feature/numeric-dx-refactor` branch. This PR contains the following changes: 1. Moves the InputWithExamples component to the NumericInput folder 2. Modernizes InputWithExamples to be a functional component 3. Addition of some comments Video Example of Snapshot Testing: https://github.com/user-attachments/assets/ca917778-50b0-46d2-89d8-dad95d1dadf2 Issue: LEMS-2785 ## Test plan: - Ensure all tests pass - Manual testing with PR Snapshot in upstream consumer - Landing onto feature branch that will see full QA regression pass before deployment Author: SonicScrewdriver Reviewers: SonicScrewdriver, mark-fitzgerald Required Reviewers: Approved By: mark-fitzgerald Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x) Pull Request URL: #2121
Summary:
This PR is part of the Numeric Input Project work. It is being landed onto the
feature/numeric-dx-refactorbranch.This PR contains the following changes:
Video Example of Snapshot Testing:
Screen.Recording.2025-01-16.at.2.45.52.PM.mov
Issue: LEMS-2785
Test plan: