Skip to content

feat: enhance keyboard shortcut visual feedback in footer navigation - #41

Open
m50S79sM6SRNp8Jn wants to merge 2 commits into
masterfrom
feature/keyboard-shortcut-visual-feedback
Open

feat: enhance keyboard shortcut visual feedback in footer navigation#41
m50S79sM6SRNp8Jn wants to merge 2 commits into
masterfrom
feature/keyboard-shortcut-visual-feedback

Conversation

@m50S79sM6SRNp8Jn

Copy link
Copy Markdown
Collaborator

Summary

  • Add enhanced visual feedback for keyboard shortcuts in footer navigation chip
  • Display Meta/Cmd keys as icons (⌘) while other keys show as text format
  • Implement 3-second auto-clear with configurable timeout constant
  • Synchronize keyboard shortcut state between store and device objects
  • Remove console warnings for physical keyboard key releases

Changes Made

  • AppFooterNavigation.vue: Enhanced chip display with icon support and auto-clear functionality
  • AppKeyboardShortcuts.vue: Added device keyboard state synchronization for visual feedback
  • useKeyboard-new.js: Removed console warning for normal physical keyboard behavior
  • en.json: Added localization support for keyboard shortcut tooltips

Visual Improvements

  • Meta and Cmd keys now display as command symbol (⌘) icons
  • Other keys display as text (e.g., "AltLeft+⌘+Escape")
  • Consistent green color matching regular key presses
  • Clean formatting without extra spaces

Test Plan

  • Test keyboard shortcut display on Windows shortcuts (Ctrl+C, Ctrl+V)
  • Test Meta key shortcuts on macOS (Meta+C shows as ⌘)
  • Verify 3-second auto-clear functionality works correctly
  • Confirm no console warnings appear for physical keyboard usage
  • Test various key combinations display properly formatted

The PR will show 4 files changed with 56 insertions and 5 deletions, focusing on the keyboard shortcut visual feedback
enhancement.

m50S79sM6SRNp8Jn and others added 2 commits November 9, 2025 20:05
- Add visual feedback for keyboard shortcuts in footer navigation chip
- Display Meta/Cmd keys as icons (⌘) while other keys show as text
- Auto-clear shortcut display after 3 seconds using configurable constant
- Synchronize keyboard shortcut state between store and device objects
- Remove console warning for physical keyboard key releases
- Add localization support for keyboard shortcut tooltips

The footer now provides clear visual indication when keyboard shortcuts
are pressed, improving user experience and system feedback.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 enhances the visual feedback for keyboard shortcuts in the footer navigation. It introduces icon-based display for Meta/Cmd keys (shown as ⌘), text display for other keys, and an auto-clear feature with a 3-second timeout. The changes also synchronize keyboard shortcut state between the store and device objects for consistent visual feedback, and remove console warnings for normal physical keyboard behavior.

  • Enhanced visual feedback with icon/text formatting for keyboard shortcuts
  • Auto-clear functionality with configurable 3-second timeout
  • State synchronization between store and device keyboard objects

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.

File Description
src/utils/locales/en.json Added new localization key keyboardShortcut (currently unused)
src/composables/useKeyboard-new.js Removed console warning for physical keyboard key releases
src/components/footer/AppFooterNavigation.vue Enhanced chip display with icon support, auto-clear functionality, and formatting inconsistencies
src/components/AppKeyboardShortcuts.vue Added device keyboard state synchronization for visual feedback
Comments suppressed due to low confidence (1)

src/components/footer/AppFooterNavigation.vue:106

  • Inconsistent indentation. Lines 104-116 use 2-space indentation while the rest of the script (lines 118-164) uses 0-space indentation. Remove the extra indentation for consistency.
  // Composables
  const { t } = useI18n();
  const { smAndUp } = useDisplay();

@click.stop
>
{{ device.hid.keyboard.keyPress }}
<template v-for="(part, index) in getFormattedKeyParts(device.hid.keyboard.keyPress)" :key="index"><v-icon v-if="part.isIcon" size="small">{{ part.icon }}</v-icon><span v-else>{{ part.text }}</span><span v-if="index < getFormattedKeyParts(device.hid.keyboard.keyPress).length - 1">+</span></template>

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

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

This template code is a single-line complex expression that's difficult to read and maintain. It also calls getFormattedKeyParts() multiple times unnecessarily (once in v-for and again in the conditional). Consider extracting to a computed property or breaking it into multiple lines for better readability and performance.

Copilot uses AI. Check for mistakes.

// Set timeout to clear the chip
keyPressTimeout.value = setTimeout(() => {
props.device.hid.keyboard.keyPress = '';

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

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

Directly mutating a prop (props.device.hid.keyboard.keyPress) violates Vue best practices. Props should be treated as read-only. The parent component should handle this state change through an event emission or by using v-model. Consider emitting an event like @update:keyPress to notify the parent to clear the value.

Copilot uses AI. Check for mistakes.
}, KEYPRESS_DISPLAY_DURATION);
}
});

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

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

The timeout is not cleared when the component is unmounted, which could lead to memory leaks and errors when trying to access props.device after unmounting. Add an onBeforeUnmount hook to clear the timeout: onBeforeUnmount(() => { if (keyPressTimeout.value) clearTimeout(keyPressTimeout.value); }).

Suggested change
// Clear keyPress timeout on unmount to prevent memory leaks
onBeforeUnmount(() => {
if (keyPressTimeout.value) {
clearTimeout(keyPressTimeout.value);
}
});

Copilot uses AI. Check for mistakes.
import { useI18n } from 'vue-i18n';
import { useDisplay } from 'vuetify';
import { computed, watch, ref } from 'vue';
import { useI18n } from "vue-i18n";

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

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

Inconsistent quote style. Line 73 uses double quotes while line 74 uses single quotes. According to the Prettier configuration (single quotes), this should use single quotes for consistency.

Suggested change
import { useI18n } from "vue-i18n";
import { useI18n } from 'vue-i18n';

Copilot uses AI. Check for mistakes.
Comment on lines 76 to 77
// Props
const props = defineProps({

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

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

Inconsistent indentation. Lines 76-102 use 2-space indentation while the rest of the script uses 0-space indentation at the top level. For consistency with the updated code style (lines 118-164), remove the extra indentation.

Copilot uses AI. Check for mistakes.
Comment thread src/utils/locales/en.json
"seconds": "seconds",
"local": "Local",
"keypress": "keypress",
"keyboardShortcut": "keyboard shortcut",

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

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

The new localization key keyboardShortcut is added but not used anywhere in the codebase. The tooltip at line 47 of AppFooterNavigation.vue still uses $t('common.keypress'). If this key is intended to replace it, update the tooltip reference; otherwise, remove this unused localization entry.

Suggested change
"keyboardShortcut": "keyboard shortcut",

Copilot uses AI. Check for mistakes.
Comment thread src/utils/locales/en.json
"seconds": "seconds",
"local": "Local",
"keypress": "keypress",
"keyboardShortcut": "keyboard shortcut",

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

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

The Chinese translation file (zh.json) is missing the corresponding keyboardShortcut entry. For complete internationalization support, add the Chinese translation alongside the English one to maintain consistency across locale files.

Copilot uses AI. Check for mistakes.

const store = useAppStore();
const { isProcessing, keyboard, settings } = storeToRefs(store);
const { device } = useDevice();

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

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

Variable 'useDevice' is used before its declaration.

Copilot uses AI. Check for mistakes.
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