feat(stage-tamagotchi): add option for chat area send key#851
feat(stage-tamagotchi): add option for chat area send key#851cheesemori wants to merge 5 commits intomoeru-ai:mainfrom
Conversation
Summary of ChangesHello @cheesemori, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a user-friendly enhancement to the chat functionality, enabling users to customize their message sending shortcut. By offering 'Enter', 'Ctrl + Enter', and 'Double Enter' options, the feature aims to improve the typing experience and reduce unintended message dispatches, particularly for multi-line inputs. The chosen preference is automatically saved and recalled for future sessions. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a useful feature for toggling the send key behavior in the chat. The implementation using @vueuse/core's useLocalStorage is appropriate for persisting the user's preference. The code is generally well-structured. My review includes suggestions to improve maintainability and readability by using constants for send modes, labels, and other magic numbers, refactoring the keydown handler for better clarity, and cleaning up the Vue template by moving logic to the script section. Additionally, there are some temporary comments like // --- new feature --- that should be removed before merging.
apps/stage-tamagotchi/src/renderer/components/InteractiveArea.vue
Outdated
Show resolved
Hide resolved
| const sendMode = useLocalStorage<SendMode>('chat-send-mode', 'enter') // use useLocalStorage to make the browser remember the user's choice. The default is 'enter' | ||
| const showSendModeMenu = ref(false) // whether the menu is displayed | ||
|
|
||
| let lastEnterPressTime = 0 // timer of double-enter |
There was a problem hiding this comment.
Extract the magic number 300 (used in the double-enter logic) into a named constant like DOUBLE_PRESS_INTERVAL to improve readability and make it easier to change the value in the future. The comment // timer of double-enter can be removed as the variable name lastEnterPressTime is descriptive enough.
const DOUBLE_PRESS_INTERVAL = 300;
let lastEnterPressTime = 0;
| function handleKeydown(e: KeyboardEvent) { // new key processing function | ||
|
|
||
| if (isComposing.value) return | ||
|
|
||
| const isEnter = e.key === 'Enter' | ||
| const isCtrl = e.ctrlKey || e.metaKey | ||
|
|
||
| if (!isEnter) return | ||
|
|
||
| // 1. Enter mode(default) | ||
| if (sendMode.value === 'enter') { | ||
| if (!e.shiftKey && !isCtrl) { | ||
| e.preventDefault() // | ||
| handleSend() | ||
| return | ||
| } | ||
| } | ||
|
|
||
| // 2. Ctrl + Enter mode | ||
| if (sendMode.value === 'ctrl-enter') { | ||
| if (isCtrl) { | ||
| e.preventDefault() | ||
| handleSend() | ||
| return | ||
| } | ||
| } | ||
|
|
||
| // 3. Double Enter mode | ||
| if (sendMode.value === 'double-enter') { | ||
| if (!e.shiftKey && !isCtrl) { | ||
| const now = Date.now() | ||
| if (now - lastEnterPressTime < 300) { | ||
| e.preventDefault() // prevent line break caused by the second press | ||
| handleSend() | ||
| lastEnterPressTime = 0 | ||
| } else { | ||
| lastEnterPressTime = now | ||
| // for the first time, don't stop it | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The handleKeydown function can be refactored to use a switch statement, which is cleaner and more readable than a series of if statements for handling the different sendMode values. The initial checks for isComposing and e.key can also be combined. This also removes redundant comments and cleans up the code.
function handleKeydown(e: KeyboardEvent) {
if (isComposing.value || e.key !== 'Enter') return
const isCtrl = e.ctrlKey || e.metaKey
switch (sendMode.value) {
case 'enter':
if (!e.shiftKey && !isCtrl) {
e.preventDefault()
handleSend()
}
break
case 'ctrl-enter':
if (isCtrl) {
e.preventDefault()
handleSend()
}
break
case 'double-enter':
if (!e.shiftKey && !isCtrl) {
const now = Date.now()
if (now - lastEnterPressTime < DOUBLE_PRESS_INTERVAL) {
e.preventDefault() // prevent line break caused by the second press
handleSend()
lastEnterPressTime = 0
}
else {
lastEnterPressTime = now
}
}
break
}
}
| <button | ||
| v-for="mode in ['enter', 'ctrl-enter', 'double-enter']" | ||
| :key="mode" | ||
| class="px-3 py-2 text-left text-xs transition-colors hover:bg-primary-100 rounded-md dark:hover:bg-primary-900/50" | ||
| :class="sendMode === mode ? 'text-primary-600 font-bold bg-primary-50 dark:bg-primary-900/20' : 'text-neutral-500'" | ||
| @click="sendMode = mode as any; showSendModeMenu = false" | ||
| > | ||
| <span class="mr-1">{{ sendMode === mode ? '✓' : ' ' }}</span> | ||
| {{ mode === 'enter' ? 'Enter' : mode === 'ctrl-enter' ? 'Ctrl + Enter' : 'Double-click Enter' }} | ||
| </button> |
There was a problem hiding this comment.
By using the SEND_MODES and SEND_MODE_LABELS constants (suggested in other comments), you can remove hardcoded values from the template. This improves maintainability and separates logic from presentation. It also provides better type inference, allowing the removal of the unsafe as any type assertion.
<button
v-for="mode in SEND_MODES"
:key="mode"
class="px-3 py-2 text-left text-xs transition-colors hover:bg-primary-100 rounded-md dark:hover:bg-primary-900/50"
:class="sendMode === mode ? 'text-primary-600 font-bold bg-primary-50 dark:bg-primary-900/20' : 'text-neutral-500'"
@click="sendMode = mode; showSendModeMenu = false"
>
<span class="mr-1">{{ sendMode === mode ? '✓' : ' ' }}</span>
{{ SEND_MODE_LABELS[mode] }}
</button>
| function handleKeydown(e: KeyboardEvent) { // new key processing function | ||
|
|
||
| if (isComposing.value) return | ||
|
|
||
| const isEnter = e.key === 'Enter' | ||
| const isCtrl = e.ctrlKey || e.metaKey | ||
|
|
||
| if (!isEnter) return | ||
|
|
||
| // 1. Enter mode(default) | ||
| if (sendMode.value === 'enter') { | ||
| if (!e.shiftKey && !isCtrl) { | ||
| e.preventDefault() // | ||
| handleSend() | ||
| return | ||
| } | ||
| } | ||
|
|
||
| // 2. Ctrl + Enter mode | ||
| if (sendMode.value === 'ctrl-enter') { | ||
| if (isCtrl) { | ||
| e.preventDefault() | ||
| handleSend() | ||
| return | ||
| } | ||
| } | ||
|
|
||
| // 3. Double Enter mode | ||
| if (sendMode.value === 'double-enter') { | ||
| if (!e.shiftKey && !isCtrl) { | ||
| const now = Date.now() | ||
| if (now - lastEnterPressTime < 300) { | ||
| e.preventDefault() // prevent line break caused by the second press | ||
| handleSend() | ||
| lastEnterPressTime = 0 | ||
| } else { | ||
| lastEnterPressTime = now | ||
| // for the first time, don't stop it | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Use useMagicKeys (https://vueuse.org/core/useMagicKeys/#usemagickeys) to replace your implementation.
| "@tresjs/cientos": "^5.2.1", | ||
| "@tresjs/core": "^5.2.1", | ||
| "@vueuse/core": "^14.1.0", | ||
| "@vueuse/core": "catalog:", |
| const { activeModel, activeProvider } = storeToRefs(useConsciousnessStore()) | ||
| const isComposing = ref(false) | ||
|
|
||
| // --- new feature --- |
apps/stage-tamagotchi/src/renderer/components/InteractiveArea.vue
Outdated
Show resolved
Hide resolved
| :class="sendMode === mode ? 'text-primary-600 font-bold bg-primary-50 dark:bg-primary-900/20' : 'text-neutral-500'" | ||
| @click="sendMode = mode as any; showSendModeMenu = false" | ||
| > | ||
| <span class="mr-1">{{ sendMode === mode ? '✓' : ' ' }}</span> |
There was a problem hiding this comment.
Use icons from https://icones.js.org/collection/ph?s=check&icon=ph:check-bold.
| <button | ||
| v-for="mode in ['enter', 'ctrl-enter', 'double-enter']" | ||
| :key="mode" | ||
| class="px-3 py-2 text-left text-xs transition-colors hover:bg-primary-100 rounded-md dark:hover:bg-primary-900/50" | ||
| :class="sendMode === mode ? 'text-primary-600 font-bold bg-primary-50 dark:bg-primary-900/20' : 'text-neutral-500'" | ||
| @click="sendMode = mode as any; showSendModeMenu = false" | ||
| > |
There was a problem hiding this comment.
Ask AI Agent to use existing
- packages/stage-ui/src/components
- packages/ui/src/components
components instead of crafting one from nowhere.
|
Rebase needed, you may ask AI to learn how to rebase. |
- Replace custom button implementation with `DropdownMenu` components from reka-ui. - Refactor key binding logic using `useMagicKeys` and `whenever`. - Update selected state icon to `ph:check-bold` as requested.
…emori/airi into feat/configurable-send-key
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature for selecting the chat message send key, enhancing user experience by preventing accidental sends. The implementation using @vueuse/core for local storage is well-chosen. My review focuses on refactoring opportunities within InteractiveArea.vue to improve code clarity, reduce duplication, and better separate logic from the template. These changes will make the new feature more maintainable.
| whenever(enter, () => { | ||
| if (isComposing.value || activeElement.value?.tagName !== 'TEXTAREA') | ||
| return | ||
|
|
||
| const isCtrl = control.value || meta.value | ||
| const isShift = shift.value | ||
|
|
||
| if (sendMode.value === 'enter') { | ||
| if (!isShift && !isCtrl) { | ||
| setTimeout(() => { | ||
| messageInput.value = messageInput.value.replace(/[\r\n]+$/, '') | ||
| handleSend() | ||
| }, 0) | ||
| } | ||
| } | ||
|
|
||
| else if (sendMode.value === 'ctrl-enter') { | ||
| if (isCtrl) { | ||
| setTimeout(() => { | ||
| messageInput.value = messageInput.value.replace(/[\r\n]+$/, '') | ||
| handleSend() | ||
| }, 0) | ||
| } | ||
| } | ||
|
|
||
| else if (sendMode.value === 'double-enter') { | ||
| if (!isShift && !isCtrl) { | ||
| const now = Date.now() | ||
|
|
||
| if (now - lastEnterTime.value < 300) { | ||
| setTimeout(() => { | ||
| messageInput.value = messageInput.value.replace(/[\r\n]+$/, '') | ||
| handleSend() | ||
| }, 0) | ||
| lastEnterTime.value = 0 | ||
| } | ||
| else { | ||
| lastEnterTime.value = now | ||
| } | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
The key handling logic in whenever(enter, ...) can be refactored to improve readability and reduce code duplication.
I recommend the following improvements:
- Extract the repeated send logic into a
triggerSendhelper function. - Use a
switchstatement for the differentsendModevalues to make the logic clearer. - The
300ms timeout for double-enter should be a named constant for better readability.
To apply this, please add the following helpers to your <script setup>:
const DOUBLE_ENTER_TIMEOUT_MS = 300;
function triggerSend() {
setTimeout(() => {
messageInput.value = messageInput.value.replace(/[\r\n]+$/, '');
handleSend();
}, 0);
}Then, you can replace the whenever block with the cleaner version in the suggestion.
whenever(enter, () => {
if (isComposing.value || activeElement.value?.tagName !== 'TEXTAREA')
return
const isCtrl = control.value || meta.value
const isShift = shift.value
switch (sendMode.value) {
case 'enter':
if (!isShift && !isCtrl)
triggerSend()
break
case 'ctrl-enter':
if (isCtrl)
triggerSend()
break
case 'double-enter':
if (!isShift && !isCtrl) {
const now = Date.now()
if (now - lastEnterTime.value < DOUBLE_ENTER_TIMEOUT_MS) {
triggerSend()
lastEnterTime.value = 0
}
else {
lastEnterTime.value = now
}
}
break
}
})
| v-for="mode in ['enter', 'ctrl-enter', 'double-enter']" | ||
| :key="mode" | ||
| class="w-full flex cursor-pointer items-center rounded-md px-3 py-2 text-left text-xs outline-none transition-colors hover:bg-primary-100 dark:hover:bg-primary-900/50" | ||
| :class="sendMode === mode ? 'text-primary-600 font-bold bg-primary-50 dark:bg-primary-900/20' : 'text-neutral-500'" | ||
| @select="sendMode = mode as any" | ||
| > | ||
| <div class="mr-2 w-4 flex shrink-0 items-center justify-center"> | ||
| <div v-if="sendMode === mode" class="i-ph:check-bold text-base" /> | ||
| </div> | ||
| <span> | ||
| {{ mode === 'enter' ? 'Enter' : mode === 'ctrl-enter' ? 'Ctrl + Enter' : 'Double-click Enter' }} | ||
| </span> | ||
| </DropdownMenuItem> |
There was a problem hiding this comment.
The template currently contains logic for iterating over send modes and generating their labels. To improve separation of concerns, type safety, and readability, this logic should be moved to the <script setup> block.
- Define a typed array for the send modes to use in
v-for. This will also eliminate the need for theas anytype cast on@select. - Create a map to store the display labels for each mode, removing the complex ternary expression from the template.
Please add the following code to your <script setup> section:
const sendModes: SendMode[] = ['enter', 'ctrl-enter', 'double-enter']
const sendModeLabels: Record<SendMode, string> = {
'enter': 'Enter',
'ctrl-enter': 'Ctrl + Enter',
'double-enter': 'Double-click Enter',
}Then, update the <DropdownMenuItem> as suggested below.
v-for="mode in sendModes"
:key="mode"
class="w-full flex cursor-pointer items-center rounded-md px-3 py-2 text-left text-xs outline-none transition-colors hover:bg-primary-100 dark:hover:bg-primary-900/50"
:class="sendMode === mode ? 'text-primary-600 font-bold bg-primary-50 dark:bg-primary-900/20' : 'text-neutral-500'"
@select="sendMode = mode"
>
<div class="mr-2 w-4 flex shrink-0 items-center justify-center">
<div v-if="sendMode === mode" class="i-ph:check-bold text-base" />
</div>
<span>
{{ sendModeLabels[mode] }}
</span>
</DropdownMenuItem>
|
Just a quick bump to prevent this from getting stale since I saw lots of updates recently. Let me know if any changes are needed! 🍵 |
Description
Added a feature to toggle the message sending shortcut in the chat area.
Users can now choose between:
This helps prevent accidental sends when typing multi-line messages.
Implementation Details
@vueuse/coreto handleuseLocalStorage, ensuring the user's send key preference persists across sessions.package.jsonandpnpm-lock.yaml.UI Screenshot