Ensure that keypad backspaces don't delete entire parentheses nodes at once. #2292
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…eses nodes at once. When performing a backspace, our Math Input keypad appears to be deleting the entire contents of a parentheses node, which has been frustrating for our users. For example: hitting backspace on x+(9*9) would result in only x+ remaining. When reviewing the code it appears that this functionality was set up deliberately, but I have been unable to find the justification or history why. Given there's no clear reason for this feature, this PR is removing this special logic so that backspaces are handled with the default logic. For example: hitting backspace on x+(9*9) would result in x+(9*9 Issue: MOB-5576 Test plan: Manual testing
…te entire parentheses nodes at once.
|
Size Change: -189 B (-0.02%) Total Size: 875 kB
ℹ️ View Unchanged
|
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (419bb9d) and published it to npm. You Example: pnpm add @khanacademy/perseus@PR2292If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR2292 |
| @@ -302,64 +302,6 @@ describe("MathQuill", () => { | |||
| expect(mathField.getContent()).toEqual("\\left(\\right)"); | |||
| }); | |||
|
|
|||
| it("should select an expression when deleting from outside (1)", () => { | |||
There was a problem hiding this comment.
I simply deleted these tests as we no longer want this unique behaviour. I didn't think there'd be much value in adding new tests to test the default behaviour, but I'm happy to take feedback on this!
There was a problem hiding this comment.
Maybe it would be worth adding one test to make sure we don't use the old behavior? Something like it("it doesn't delete whole expressions when using backspace (MOB-5576)", () => {
Were these tests failing after you made your changes?
There was a problem hiding this comment.
They were (failing)!
I like including the ticket number in the test - great idea! I'll get that added.
There was a problem hiding this comment.
@handeyeco , I've added the test! I added some comments as it's a little confusing with how it works. :)
While we may show the user that we've "deleted" the parens, they both still exist in the code until the expression is fully empty and the backspace is hit one more time to remove the opening parens.
handeyeco
approved these changes
Mar 11, 2025
Merged
catandthemachines
added a commit
that referenced
this pull request
Mar 12, 2025
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @khanacademy/[email protected] ### Minor Changes - [#2293](#2293) [`91e30c02c`](91e30c0) Thanks [@catandthemachines](https://github.com/catandthemachines)! - Deprecate usage of PerseusErrorOccurred event logging in favor of PerseusErrorOccurredTI events. ### Patch Changes - [#2294](#2294) [`254fa3605`](254fa36) Thanks [@nishasy](https://github.com/nishasy)! - [Polygon] Show right angle markers when showAngles is on - Updated dependencies \[[`91e30c02c`](91e30c0), [`59b932619`](59b9326)]: - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Minor Changes - [#2293](#2293) [`91e30c02c`](91e30c0) Thanks [@catandthemachines](https://github.com/catandthemachines)! - Deprecate usage of PerseusErrorOccurred event logging in favor of PerseusErrorOccurredTI events. ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`91e30c02c`](91e30c0)]: - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`91e30c02c`](91e30c0)]: - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - [#2292](#2292) [`59b932619`](59b9326) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Ensure that keypad backspaces don't delete entire parentheses nodes at once. - Updated dependencies \[[`91e30c02c`](91e30c0)]: - @khanacademy/[email protected] - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`91e30c02c`](91e30c0), [`254fa3605`](254fa36), [`59b932619`](59b9326)]: - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`91e30c02c`](91e30c0)]: - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`91e30c02c`](91e30c0)]: - @khanacademy/[email protected] - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`91e30c02c`](91e30c0), [`59b932619`](59b9326)]: - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
When performing a backspace, our Math Input keypad appears to be deleting the entire contents of a parentheses node, which has been frustrating for our users. This functionality also occurs for logarithmic or trigonometric functions.
For example: hitting backspace on
x+(9*9)would result in onlyx+remaining.When reviewing the code it appears that this functionality was set up deliberately, but I have been unable to find the justification or history why.
Given there's no clear reason for this feature, this PR is removing this special logic so that backspaces are handled with the default logic.
For example: hitting backspace on
x+(9*9)would result inx+(9*9Issue: MOB-5576
Test plan:
Manual testing