fix: prevent keyboard shortcuts from triggering while typing in input fields #12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… fields Improved the input field detection in the global keyboard shortcut handler to comprehensively check if the user is currently typing. This prevents shortcuts like 'N', 'P', 'C', and 'S' from being triggered while editing text in any input field, textarea, or contenteditable element. The fix includes: - Checking both event target and active element - Detecting INPUT, TEXTAREA, and SELECT elements - Checking contentEditable elements - Using closest() to detect nested input elements Fixes the issue where shortcuts were making the app unusable during normal typing.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
components/ProBillSplitter.tsx
Outdated
Comment on lines
582
to
584
| activeElement?.contentEditable === 'true' || | ||
| activeElement?.isContentEditable || | ||
| target.closest('input, textarea, select, [contenteditable="true"]') !== null |
There was a problem hiding this comment.
The updated input detection now always calls target.closest(...), but the global keydown listener is attached to window (lines 775–778), where the event target is often the document when no input has focus. Document does not implement closest, so pressing any key while nothing focusable is active will throw a TypeError and halt all keyboard shortcuts before they run. Guard the call with an instanceof Element check or optional chaining to avoid crashing the handler when the target isn’t an element.
Useful? React with 👍 / 👎.
Prevent TypeError when keyboard events target non-element objects like document. The closest() method only exists on Element instances, so we must check with instanceof Element before calling it to avoid breaking all keyboard shortcuts when no focusable element is active.
Three quality improvements: 1. PersonChip keyboard accessibility - Add tabIndex, role="button", and aria attributes - Support Enter and Space key activation - Only apply when onToggle prop is provided 2. SSR-safe localStorage access in MobileSpreadsheetView - Guard localStorage calls with typeof window check - Prevents potential hydration mismatches - Safer for Next.js SSR/SSG environments 3. Remove console.log in production - Clean up debug statement in bill loading logic - Silent early return when request is cancelled
The test was failing in CI because Google API keys were present in the environment. Updated the test to explicitly clear all OCR-related API keys before testing the warning behavior, ensuring consistent results across different environments.
Change all single-key shortcuts to require Cmd/Ctrl modifiers to prevent interference with Excel-like grid navigation. This allows users to type directly in grid cells without triggering shortcuts. New shortcuts: - Cmd+Shift+N: Add new item (was: N) - Cmd+Shift+P: Add person (was: P) - Cmd+Shift+C: Copy summary (was: C) - Cmd+S: Share (was: S) - Cmd+N: New bill (unchanged) - Cmd+Z: Undo (unchanged) - Cmd+Shift+Z: Redo (unchanged) This enables true Excel-like behavior where users can tab to cells and immediately start typing without double-clicking.
Replace type="number" with type="text" and inputMode="decimal" for all numeric inputs (price, quantity, tax, tip, discount). This: - Removes ugly up/down spinner arrows on desktop - Still shows numeric keyboard on mobile devices - Provides cleaner, more professional appearance - Matches Excel-like spreadsheet UX expectations Users can now input numbers without the distracting spinners.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Improved the input field detection in the global keyboard shortcut handler
to comprehensively check if the user is currently typing. This prevents
shortcuts like 'N', 'P', 'C', and 'S' from being triggered while editing
text in any input field, textarea, or contenteditable element.
The fix includes:
Fixes the issue where shortcuts were making the app unusable during normal typing.