Skip to content

refactor(caret): move active word element offset to test-state and remove .smoothScroller (@NadAlaba) #6541

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

NadAlaba
Copy link
Contributor

@NadAlaba NadAlaba commented May 7, 2025

Description

  1. Move activeWordElementOffset to test-state.ts to allow caret.ts to use it without causing circular dependency:

    • Motivation is to determine active word based on logical value, not DOM class, this started on #5834, but didn't continue for caret.ts because it'd cause circular dependency. This move of activeWordElementOffset to test-state.ts would solve the issue.
  2. Remove unneeded .smoothScroller:

    • .smoothScoller was only needed to transmit information from test-ui.ts to caret.ts in order to help with caret's vertical positioning during line jump. Now it is replaced by a variable lineScrollDistance in test-state.ts which gets set in test-ui.ts: lineJump() and used in caret.ts.
  3. Remove the unnecessary parameter activeWordEmpty from caret.ts: getTargetPositionLeft() and cleanup:

    • Combine the case of (zenMode + RTL + word beginning) with one of the cases of the conditional above it, making the parameter not needed anymore.
    • Also the variable activeWordEmpty in updatePosition() is not needed either, because in zen mode we set wordLen = inputLen, so activeWordEmpty is exactly the same as wordLen === 0.
  4. Change variable name from activeWordElementOffset to removedUIWordCount & add a helper function:

    • Also, add a helper function incrementRemovedUIWordCount() to shorten code changing the variable's value.

@monkeytypegeorge monkeytypegeorge added the frontend User interface or web stuff label May 7, 2025
Copy link

sentry-io bot commented May 7, 2025

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: frontend/src/ts/controllers/input-controller.ts

Function Unhandled Issue
backspaceToPrevious Error: activeWord is undefined - can't update active element ...
Event Count: 4 Affected Users: 4
handleChar Error: activeWord is undefined - can't update active element ...
Event Count: 3 Affected Users: 87
📄 File: frontend/src/ts/test/test-logic.ts (Click to Expand)
Function Unhandled Issue
init TypeError: Cannot read properties of null (reading 'style') ...
Event Count: 4 Affected Users: 3
init Error: activeWord is undefined - can't update active element ...
Event Count: 2 Affected Users: 9
init SyntaxError: Invalid left-hand side in assignment /
Event Count: 1 Affected Users: 1
📄 File: frontend/src/ts/test/test-ui.ts (Click to Expand)
Function Unhandled Issue
updateWordsInputPosition TypeError: Cannot read properties of null (reading 'style') ...
Event Count: 146 Affected Users: 121
updateActiveWordLetters TypeError: Cannot read properties of undefined (reading 'offsetLeft') ...
Event Count: 31 Affected Users: 92
updateActiveElement Error: activeWord is undefined - can't update active element ...
Event Count: 16 Affected Users: 87
---

Did you find this useful? React with a 👍 or 👎

@NadAlaba NadAlaba marked this pull request as draft May 7, 2025 18:59
@NadAlaba NadAlaba changed the title refactor(caret): move active word element offset to test-state and remove smooth line scroller (@NadAlaba) refactor(caret): move active word element offset to test-state and remove .smoothScroller (@NadAlaba) May 12, 2025
@NadAlaba NadAlaba marked this pull request as ready for review May 12, 2025 14:36
@github-actions github-actions bot added the waiting for review Pull requests that require a review before continuing label May 12, 2025
@NadAlaba
Copy link
Contributor Author

NadAlaba commented May 13, 2025

after merging #5748 and syncing master, checkout this pr head and:
git rebase --onto master f5f613b
then force push here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend User interface or web stuff waiting for review Pull requests that require a review before continuing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants