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.

Sign up for GitHub

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

Jump to bottom

fix: Handle Enter key event for IME inputs #3272

Merged
merged 2 commits into from
Jan 23, 2025
Merged

fix: Handle Enter key event for IME inputs #3272

merged 2 commits into from
Jan 23, 2025

Conversation

ryotatake
Copy link
Contributor

@ryotatake ryotatake commented Jan 13, 2025 •
edited
Loading

Types of changes

Description

“Create or edit Task" Modal input, there was a conflict between the Enter key event used to confirm IME conversions and the Enter key event used to create tasks, causing unintended behavior.

This has been corrected so that the Enter key is not used to create a task when the IME conversion is confirmed.

Motivation and Context

How has this been tested?

Manually tested the following

  • When using IME, the task is not created when the conversion is confirmed with the Enter key. When the Enter key is pressed again, the task is created.
  • When IME is not used, the task is created when the Enter key is pressed.

Screenshots (if appropriate)

before fix

IME_input.mov

after fix

IME_input_after_fix.mov

Checklist

  • My code follows the code style of this project and passes yarn run lint.
  • [-] My change requires a change to the documentation.
  • [-] I have updated the documentation accordingly.
  • My change has adequate Unit Test coverage.
    • I was not sure if testing was needed for this change, so please let me know if it is needed.

Terms

@claremacrae claremacrae self-requested a review January 20, 2025 12:59
@claremacrae
Copy link
Collaborator

For my own reference, this is the documentation of the code added e.isComposing

https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/isComposing

claremacrae
claremacrae approved these changes Jan 20, 2025
View reviewed changes
Copy link
Collaborator

@claremacrae claremacrae left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thank you very much for doing this - I would have had no idea this was the solution.

I looked for other uses of the same pattern elsewhere in Tasks, and found these:

obsidian-tasks/src/Suggestor/EditorSuggestorPopup.ts

Lines 107 to 116 in d26fd2e

if (value.suggestionType === 'empty') {
// Close the suggestion dialog and simulate an Enter press to the editor
this.close();
const eventClone = new KeyboardEvent('keydown', {
code: 'Enter',
key: 'Enter',
});
(editor as any)?.cm?.contentDOM?.dispatchEvent(eventClone);
return;
}

obsidian-tasks/src/ui/Dependency.svelte

Lines 57 to 66 in 4e8c961

case 'Enter':
if (searchIndex !== null) {
e.preventDefault();
addTask(searchResults[searchIndex]);
searchIndex = null;
inputFocused = false;
} else {
_onDescriptionKeyDown(e);
}
break;

Are they things you would be interested in looking at too?

@claremacrae
Copy link
Collaborator

(I've edited the comment about to increase the number of lines of code shown, for readability)

@claremacrae claremacrae added the scope: edit task The Create or edit task modal/dialog label Jan 20, 2025
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@ryotatake
Copy link
Contributor Author

@claremacrae

Thank you for your review!

Thank you also for letting me know what other codes might be relevant.
The first one did not affect IME input.
The second one was a code that caused a problem with IME input! So I have made an additional correction.

I would be happy to review it when you have time.

@claremacrae claremacrae merged commit 245b38e into obsidian-tasks-group:main Jan 23, 2025
3 checks passed
@claremacrae
Copy link
Collaborator

@ryotatake Many thanks for doing this!

@claremacrae claremacrae mentioned this pull request Jan 23, 2025
Closed
4 tasks
@ryotatake
Copy link
Contributor Author

Thank you for merging!

I use this plugin every day. Many thanks for your maintenance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@claremacrae claremacrae claremacrae approved these changes

Assignees
No one assigned
Labels
scope: edit task The Create or edit task modal/dialog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ryotatake @claremacrae