refactor: share keyboard shortcuts and generated feature documentation - #803
Open
jabrailkhalil wants to merge 1 commit into
Open
refactor: share keyboard shortcuts and generated feature documentation#803jabrailkhalil wants to merge 1 commit into
jabrailkhalil wants to merge 1 commit into
Conversation
Refs libredb#802. Use physical keys and one registry for DOM matching, Monaco bindings, UI labels and the FEATURES shortcut list.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
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.
Closes #802.
The tab bar, command palette and sidebar each matched keys differently, while Monaco bindings, UI hints and the FEATURES list repeated the same chords. This adds one internal registry for matching, display labels, Monaco keybindings and generated documentation, following the suggestion in the review of #782.
All three DOM handlers now match physical
event.codewith explicit modifiers. Caps Lock and non-Latin layouts work consistently; undocumented extra Alt/Shift combinations are rejected. Listener ownership, the new-tab rename guard and textarea support stay in the components. Monaco keeps its native command registration. UI hints show Cmd/Ctrl, andshortcuts:syncupdates FEATURES from the registry; the unit suite fails if the list drifts. The sidebar entry explicitly describes the optional SidebarProvider context.Testing
bun run test:shortcuts: 173 passing tests (5 unit, 168 component), including shifted/non-Latin keys, Ctrl/Meta, incorrect modifiers, listener cleanup, rename/textarea behavior and documentation regeneration/drift.keyboard-shortcuts.ts58/58 lines, CommandPalette 140/140 and StudioTabBar 125/125. Temporarily replacing the physical-key comparison withevent.keymakes two unit tests fail; source restored afterward.bun run format,bun run lint,bun run typecheck,bun run knip,bun run shortcuts:check,bun run readme:check,bun run chart:check,bun run channels:showcase:check,bun run security:check,bun run build,bun run build:lib,bun run attwandgit diff --check.bun run test:components: 33/35 groups pass. The same two groups fail on cleanmain(ce157a1): SQLite fixture cleanup gets WindowsEBUSY, and OperationsTab expects a different number separator. The new sidebar test is registered in the component runner and passes.bun run testalso fails locally, including missing Helm, Windows path expectations and shared-process mock interference; the clean-main run fails too. The complete merged coverage gate requires CI; the targeted report above is not a claim of full-project coverage.Local toolchain: Windows, Bun 1.4.0 and Node 22.21.1.
CI on
45508e9has now passedLint, Typecheck and BuildandUnit & Integration Tests, including the full merged coverage gate, in the repository-pinned environment. Security, Helm, Node 24/26 engine smoke, channel E2E, PostgreSQL functional smoke and browser E2E checks also passed.