feat(keyboard): add keyboard shortcuts for log viewer#10
Merged
Conversation
Spec artifacts: - research.md: feasibility analysis and codebase exploration - requirements.md: user stories and acceptance criteria - design.md: architecture and technical decisions - tasks.md: POC-first implementation plan (22 tasks) Ready for implementation.
- Add selectedIndex state initialized to -1 (no selection) - Add svelte:document onkeydown handler for global shortcuts - Implement handleKeyboardShortcut with j/k key handling - Add scrollSelectedIntoView() to scroll selected log into view - Guard shortcuts when modal open or in form elements
- Add selectedIndex prop to LogTable component
- Pass isSelected={i === selectedIndex} to LogRow and LogCard
- Add isSelected prop to LogRow and LogCard with data-selected attribute
- Pass selectedIndex from page to LogTable
The data-selected attribute enables scrollSelectedIntoView() to find
selected elements. Visual styling will be added in subsequent tasks.
- Added conditional bg-primary/10 and ring-1 ring-primary/50 classes when isSelected is true - Added aria-selected attribute for accessibility - Note: svelte-check warns about aria-selected with role="button" but task requires it for screen reader support
- Fixed a11y warnings: aria-selected is not valid for role="button" - Changed to aria-current="true" for selected state on LogRow and LogCard - All lint and type checks now pass with 0 errors and 0 warnings
Verified existing LogDetailModal implementation: - Escape handling via <svelte:document onkeydown> - Focus restoration via previouslyFocusedElement - Selection preserved after modal close (selectedIndex independent)
- bun run lint: passed (254 files checked, no issues) - bun run check: passed (0 errors, 0 warnings) - No fixes needed, all POC shortcuts working correctly
- Import announceToScreenReader from focus-trap utility - Add position announcements on j/k navigation (e.g., "Log 3 of 10") - Only announce when selection actually changes (check prevIndex)
- Remove unused ShortcutDefinition export (knip compliance) - Add loading state guards to j/k navigation - Ensure empty log list check for navigation shortcuts - keyboard.ts already has proper JSDoc comments - No console.log statements found
- Test shouldBlockShortcut function: - Blocks shortcuts for form elements (INPUT, TEXTAREA, SELECT) - Blocks shortcuts during IME composition - Blocks shortcuts when modifier keys are pressed (Ctrl, Alt, Meta) - Allows shortcuts for regular elements (DIV, TABLE, BUTTON, BODY) - Handles null target gracefully - Test FORM_ELEMENTS constant contains all expected elements - Test SHORTCUTS array contains all expected shortcuts with properties
Add test coverage for keyboard shortcut related functionality: - Test ref bindable prop exposes input element - Test programmatic focus via ref works - Test onEscape callback when Escape is pressed - Test input blurs on Escape key - Test onEscape not called for other keys - Test graceful handling when onEscape not provided
Comprehensive test coverage for the keyboard shortcuts help modal: - Test rendering when open/closed - Test all shortcuts from SHORTCUTS array are displayed - Test group headers (Navigation, Search & Filters, Other) - Test onClose callback on Escape key press - Test backdrop click closes modal - Test close button functionality - Test accessibility: dialog role, aria-modal, aria-labelledby - Test kbd elements have aria-labels for screen readers
Fixes E2E test failure where Enter key was being intercepted by the page-level keyboard handler even when no log was selected, preventing buttons like the filter-toggle from working properly. The fix moves preventDefault() inside the condition that checks for a valid selection, allowing buttons to work normally when no log is selected.
Divkix
changed the title
There was a problem hiding this comment.
Pull request overview
This PR implements Gmail/GitHub-style keyboard shortcuts for the log viewer to enable power users to navigate and interact with logs without using the mouse. The implementation adds comprehensive keyboard navigation (j/k for up/down, Enter to open details, / for search, l for live toggle, ? for help) with visual selection highlighting and accessibility features.
Changes:
- Added keyboard shortcut handler at page level using
<svelte:document>pattern - Implemented visual selection state with
selectedIndextracking and highlighting on LogRow/LogCard - Created keyboard utilities module with shortcut guards and definitions
- Built KeyboardHelpModal component to display available shortcuts
- Extended SearchInput component to support programmatic focus and Escape handling
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/routes/(app)/projects/[id]/+page.svelte | Main keyboard handler with j/k/Enter/Escape///l/? shortcuts and selection state management |
| src/lib/utils/keyboard.ts | Keyboard guard utilities and shortcut definitions for help modal |
| src/lib/utils/keyboard.unit.test.ts | Unit tests for shouldBlockShortcut function and SHORTCUTS array |
| src/lib/components/keyboard-help-modal.svelte | New modal component displaying available keyboard shortcuts |
| src/lib/components/keyboard-help-modal.component.test.ts | Component tests for KeyboardHelpModal |
| src/lib/components/search-input.svelte | Added ref bindable and onEscape callback for / shortcut |
| src/lib/components/tests/search-input.component.test.ts | Tests for ref exposure and onEscape handling |
| src/lib/components/log-table.svelte | Accepts selectedIndex prop and passes isSelected to children |
| src/lib/components/log-row.svelte | Added isSelected prop with visual selection styling |
| src/lib/components/tests/log-row.component.test.ts | Tests for isSelected visual styling |
| src/lib/components/log-card.svelte | Added isSelected prop with visual selection styling |
| src/lib/components/tests/log-card.component.test.ts | Tests for isSelected visual styling |
| specs/keyboard-shortcuts/*.md | Comprehensive spec documentation (research, requirements, design, tasks) |
| .gitignore | Added spec tracking files to gitignore |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- PR #10 created with full description - Fixed E2E test failure (Enter key preventDefault issue) - All CI checks passing
Use `isSelected || undefined` instead of ternary for cleaner code. Addresses Copilot review feedback on PR #10.
All 28 acceptance criteria verified and passing. Spec execution finished - 22/22 tasks complete.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Implements Gmail/GitHub-style keyboard shortcuts for the log viewer, enabling power users to navigate and interact with logs without using the mouse.
Implemented shortcuts:
j/k- Navigate down/up through logsEnter- Open detail modal for selected logEscape- Close modals, blur search/- Focus search inputl- Toggle live mode (when not paused)?- Open keyboard shortcuts help modalKey features:
Files Created
src/lib/utils/keyboard.ts- Keyboard utilities (shouldBlockShortcut, SHORTCUTS)src/lib/components/keyboard-help-modal.svelte- Help modal componentsrc/lib/utils/keyboard.unit.test.ts- Unit tests for keyboard utilitiessrc/lib/components/log-row.component.test.ts- Component tests for LogRowsrc/lib/components/log-card.component.test.ts- Component tests for LogCardsrc/lib/components/search-input.component.test.ts- Component tests for SearchInputsrc/lib/components/keyboard-help-modal.component.test.ts- Component tests for KeyboardHelpModalFiles Modified
src/routes/(app)/projects/[id]/+page.svelte- Main keyboard handlersrc/lib/components/log-table.svelte- Accept selectedIndex propsrc/lib/components/log-row.svelte- Accept isSelected, add visual stylessrc/lib/components/log-card.svelte- Accept isSelected, add visual stylessrc/lib/components/search-input.svelte- Expose ref, add onEscape callbackTest Coverage
Test Plan