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 The Freeze Hinter #3252 #3299

Merged
merged 9 commits into from
Feb 12, 2025
Merged

Conversation

Jatin24062005
Copy link
Contributor

@Jatin24062005 Jatin24062005 commented Dec 9, 2024

Making runnig all the tests and
Working When Press the Esc key to dismiss the Hinter.

1.Start typing something with the Hinter enabled.
2.The Hinter window pops up.
3.Press the Esc key to dismiss the Hinter.

Fixes #3252

Changes:
Added a keydown event listener in the Completion constructor to monitor key presses on the editor and trigger the handleKeyPress method for custom key handling.

Implemented Escape key functionality in the handleKeyPress method to close the completion widget by calling this.close() and preventing default behavior.
I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

Jatin24062005 and others added 2 commits December 9, 2024 11:19
Making runnig all the tests and
Working When Press the Esc key to dismiss the Hinter.

1.Start typing something with the Hinter enabled.
2.The Hinter window pops up.
3.Press the Esc key to dismiss the Hinter.
Copy link
Collaborator

@raclim raclim left a comment

Choose a reason for hiding this comment

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

Thanks so much for your work on this and sorry that it took a while!

I think this is a pretty good start so far and addresses the issue of the autocomplete hinter freezing when the escape key is pressed! One additional change I would make is besides my other comment is ensuring that the cursor is visible on the editor after the escape key as pressed. I think at the moment, although the window is closed, the user cannot continue typing within the text editor.

Thanks so much again and let me know if you have any questions!

package.json Outdated
@@ -125,7 +125,7 @@
"eslint-plugin-storybook": "^0.6.15",
"file-loader": "^6.2.0",
"husky": "^4.3.8",
"jest": "^27.3.1",
"jest": "^27.5.1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this change is not related to the issue, I would remove the changes to package.json and package-lock.json for this PR!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@raclim Thankyou for your response should i make any Changes/Remove package.json and package-lock.json or you can do from your end ??

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please give it a try! If you end up facing any issues with it, I'll jump in and see if I can support!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i have Fixed Package.json but struggling to fix package-lock.json . else everything is good to go from my side.

@raclim
Copy link
Collaborator

raclim commented Feb 12, 2025

Thanks so much for giving this a shot!

Removing package.json is a great start, but can sometimes cause issues for the package-lock.json file. I reset the package-lock.json file to match the one on the develop branch, which I think should fix it for now. I believe you can also remove changes from these files by running npm uninstall [name-of-pkg].

During this process, I remembered that there were a few areas between the Editor component and show-hint.js files that already referenced the Escape key, and ended up doing some extra digging. I think these multiple references were what caused this issue. I'm sorry I ended up going forward with implementing a modified solution within this PR 😅 but wanted to keep the history of the work you provided!

I'll do a quick break down of the changes I made, which ended up being tied to an earlier issue. When looking at show-hint.js, I realized that there was already a reference to the Escape key in the buildKeyMap() function on line 211, which establish binding the keys and hinter actions. Line 233 was dedicated to the escape key, which ideally closes the hinter.

There's another area that handles the Escape key within the Editor component on lines 212 - 223, which was added in #3114 to address an issue where the Editor was a focus trap. This solution did work but seemed to be interfering with closing the autocomplete hinter. This issue finds another way to address the focus trap by setting an extra key-biding for Shift-Tab to false. I added in this change, which seems to be working for me and I believe addresses the issue.

I'm going to merge this in with these updates, but please feel free to make any further modifications to this pull request or open a new one with any further suggestions or changes you would like to make! Thank you again for your work on this!

@raclim raclim merged commit 5e51841 into processing:develop Feb 12, 2025
1 check passed
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.

Autocomplete Hinter "freezes"
2 participants