Skip to content

Conversation

apollo13
Copy link
Contributor

@apollo13 apollo13 commented Aug 28, 2025

I don't really know vue so I am not sure this is correct :)

@fiftin fiftin requested a review from Copilot August 30, 2025 14:18
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds the ability to submit edit dialogs by pressing the Enter key, in addition to the existing Escape key functionality for closing dialogs.

  • Renamed the keyboard event handler from handleEscape to handleControlKeys to reflect its expanded functionality
  • Added Enter key handling to trigger the save action by setting needSave = true

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +231 to +232
if (ev.key === 'Enter' && this.dialog !== false) {
this.needSave = true;
Copy link
Preview

Copilot AI Aug 30, 2025

Choose a reason for hiding this comment

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

The Enter key handler should prevent the default behavior to avoid potential form submission conflicts. Add ev.preventDefault() after setting needSave = true.

Suggested change
if (ev.key === 'Enter' && this.dialog !== false) {
this.needSave = true;
this.needSave = true;
ev.preventDefault();

Copilot uses AI. Check for mistakes.

Comment on lines +231 to +233
if (ev.key === 'Enter' && this.dialog !== false) {
this.needSave = true;
}
Copy link
Preview

Copilot AI Aug 30, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider adding a check similar to the Escape handler to ensure Enter doesn't trigger in inappropriate contexts. You may want to add conditions like !this.noEnter or validate that input fields are focused to prevent accidental saves.

Copilot uses AI. Check for mistakes.

@fiftin
Copy link
Collaborator

fiftin commented Aug 30, 2025

@apollo13 Some dialogs have multiline text inputs, so I don't think this behavior works well for all dialogs.

@apollo13
Copy link
Contributor Author

apollo13 commented Aug 30, 2025 via email

@fiftin fiftin closed this Sep 14, 2025
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.

2 participants