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

Refactor detection logic of keyboard shortcuts in ChatPage #476

Conversation

Yukinobu-Mine
Copy link
Contributor

@Yukinobu-Mine Yukinobu-Mine commented Jul 27, 2024

Issue #, if available:
Closes #368

Description of changes:

  • Refactor detection logic of Cmd+Shift+O in ChatPage.
  • Move detection logic of Shift+ESC from Textarea to ChatPage

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@Yukinobu-Mine Yukinobu-Mine linked an issue Jul 27, 2024 that may be closed by this pull request
2 tasks
@Yukinobu-Mine
Copy link
Contributor Author

Yukinobu-Mine commented Jul 27, 2024

@k70suK3-k06a7ash1 Cmd-Shift-O でのページ遷移について、意図しないと思われる挙動を確認したので、その修正となります。

本来は Cmd+Shift+O を同時に押した時点で遷移するのが期待動作だと思われますが、チャット入力中のキー入力シーケンスで Cmd と Shift と O の押下がそれぞれ独立して判定され、例えば以下のような入力をすると、「で」を入力するために日本語入力に切り替えようとした瞬間にページ遷移が発火します。

(Shift) COBOL (Cmd + `) で開発

また、カスタムボット利用時は、遷移先を / ではなく /bot/{botId} にする修正も行いました。
上記修正内容に問題ございませんでしょうか?

@Yukinobu-Mine Yukinobu-Mine self-assigned this Jul 27, 2024
@k70suK3-k06a7ash1
Copy link
Contributor

@Yukinobu-Mine
修正対応ありがとうございます!
問題ございません!

@Yukinobu-Mine Yukinobu-Mine force-pushed the 368-feature-requesttransition-to-initial-chat-page-with-keyboard-commands branch from 7773cc6 to daca168 Compare July 28, 2024 00:27
@Yukinobu-Mine
Copy link
Contributor Author

@k70suK3-k06a7ash1 もう少し検討を進めたところ、以下のように KeyboardEvent の shiftKeymetaKey といったプロパティを使うことで activeCodes で修飾キーを記憶する必要がなくなり、keyup イベントを監視する必要もなくなるのではないかと考え、試したところ問題なく動作しているようでした。(修飾キーの左右どちらでも正しく検出されました)

const isNewConversationCommand = (() => {
  if (event.code !== 'KeyO') {
    return false;
  }
  if (isWindows) {
    return event.ctrlKey && event.shiftKey;
  } else {
    return event.metaKey && event.shiftKey;
  }
})();

上記の判定ロジックについて、何か問題があればご教示いただけますと幸いです 🙇‍♂️

@k70suK3-k06a7ash1
Copy link
Contributor

@Yukinobu-Mine
なるほど!直接Eventから参照するのですね!
上記問題ないかと思います!
ご対応いただき、ありがとうございます!!!

@Yukinobu-Mine Yukinobu-Mine changed the title Fix: Screen transition occurs due to unintended key input Refactor detection logic of keyboard shortcuts in ChatPage Jul 29, 2024
@Yukinobu-Mine Yukinobu-Mine merged commit f4ce2ac into v1 Jul 29, 2024
6 checks passed
@k70suK3-k06a7ash1
Copy link
Contributor

@Yukinobu-Mine
ご対応いただきありがとうございました!!

@Yukinobu-Mine Yukinobu-Mine deleted the 368-feature-requesttransition-to-initial-chat-page-with-keyboard-commands branch July 30, 2024 10:46
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.

[Feature Request]Transition to initial Chat page with keyboard commands
2 participants