Skip to content
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

Fix cursor jumping bug in tree and comment renaming #7390

Merged
merged 5 commits into from
Oct 16, 2023

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented Oct 16, 2023

#7136 removed a workaround for cursor jumping, because I thought that react v16 would have fixed this. During testing, I assumed that all inputs would suffer from this bug, but it turns out that only controlled inputs that store their state in the store (and not in a local state) are affected. I think, this is only true for the tree name and comment inputs which is likely why I missed this back then.
I re-introduced the bug fix and removed the isFocused handling, since that assumed that the input's content should not update when the input is changed from the outside while it's focussed. However, the dataset position view supports changing the input via the arrow-keys now which is not compatible with the isFocused check. I think the change should not cause any issues ™️

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • create a new tree and try to rename the tree
  • create a new node and try to add a comment for it and rename it
  • in both cases, do some typical text editing (e.g., also insert something in the middle of the string). the cursor should behave normally (no jumping)
  • focus the position input in the navbar and use the arrow keys to increment/decrement the coordinates -> should also work as expected

Issues:


(Please delete unneeded items, merge only when none are left open)

@philippotto philippotto self-assigned this Oct 16, 2023
Copy link
Member

@daniel-wer daniel-wer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@philippotto philippotto enabled auto-merge (squash) October 16, 2023 12:14
@philippotto philippotto merged commit dec2382 into master Oct 16, 2023
1 check passed
@philippotto philippotto deleted the fix-cursor-jumping branch October 16, 2023 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cursor jumps while typing comments for node
2 participants