Keyboard architecture refactor: Key.handler, ContextualAction, focus decentralization#2198
Open
Negabinary wants to merge 15 commits intodevfrom
Open
Keyboard architecture refactor: Key.handler, ContextualAction, focus decentralization#2198Negabinary wants to merge 15 commits intodevfrom
Negabinary wants to merge 15 commits intodevfrom
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #2198 +/- ##
==========================================
- Coverage 49.89% 49.81% -0.08%
==========================================
Files 251 253 +2
Lines 28804 28842 +38
==========================================
- Hits 14371 14369 -2
- Misses 14433 14473 +40
🚀 New features to boost your workflow:
|
Member
|
patchwork detection??? claude may be lost in the multiverse again |
Contributor
Author
lol I was hoping you'd know what it meant by that |
Contributor
Author
Known issues (in progress)
Working on fixes. |
Negabinary
force-pushed
the
keyboard-architecture-refactor
branch
from
5fe9bad to
2bd21d5
Compare
Contributor
Author
Latest: Distribute contextual actions, wire EditMode, remove dead codeThis commit cleans up the dead code from the initial refactor and moves the architecture closer to the What changedContextual actions distributed to their layers (replaces
EditMode wired into CodeEditable:
Dead code removed (net -11 lines):
Architecture directionThis follows the |
Negabinary
force-pushed
the
keyboard-architecture-refactor
branch
from
a303b11 to
dd41fbb
Compare
Contributor
Author
Follow-up: Remove dead handle_key_event chainRemoved 305 lines of dead Removed from:
What's still live:
|
Adds a Key.handler(~f) function that bundles on_keydown + on_keyup + tabindex(0) into a single virtual-dom attribute. This replaces the duplicated inline event wiring pattern across Page.re, CellEditor.re, EditorView.re, and projector implementations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the inline on_keydown/on_keyup handlers with Key.handler(~f). Extract meta key tracking from the raw JS event to Key.t's meta field instead, removing the update_meta helper. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add ContextualAction.re as a unified type for actions that can serve as both keyboard shortcuts and command palette entries. The type uses Effect.t(unit) instead of Page.Update.t, decoupling actions from a specific update type. Refactor Shortcut.re to produce ContextualAction.t values via an inject function. Update NinjaKeys.re to consume ContextualAction.t directly with of_contextual_action. This means shortcuts and command palette entries are now defined in one place with one type. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extend cursor('update) with a contextual_actions field so components can
contribute context-sensitive actions to the command palette. Add
Cursor.with_actions helper for composing action lists.
Refactor Shortcut.re to keep its own Page.Update.t-based type with a
to_contextual_action converter. Add Shortcut.contextual_actions for
producing ContextualAction.t lists. Update NinjaKeys.initialize to
accept ContextualAction.t lists and execute effects via
Effect.Expert.handle_non_dom_event_exn.
Static shortcuts are initialized at startup from Main.re. The
contextual_actions field on cursor enables future per-component
dynamic actions (e.g. projector shortcuts only when applicable).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
EditMode.re defines the edit_mode type that bundles inject, escape, take_focus, and focus state for editor views. This replaces the pattern of passing separate ~inject, ~signal, ~selected parameters and adds escape callbacks for boundary navigation between cells/projectors. EditorView.Focus provides the new keyboard handling pattern where handlers return Effect.t(unit) directly (not option(Update.t)) and support arrow key escape at editor boundaries. It wraps the existing CodeEditable.Selection.handle_key_event chain and adds boundary detection using zipper caret/sibling state. Includes detailed documentation of non-obvious gotchas: - Escape direction semantics (direction = side to escape TO) - tabindex(0) scroll prevention (must Prevent_default on arrows) - Boundary detection must check caret == Outer specifically Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The inject_effect function was calling schedule_action eagerly during NinjaKeys initialization, meaning every action fired once at startup and the stored effect was Effect.Ignore. Use Bonsai.Effect.of_sync_fun to defer execution until the command palette item is actually clicked. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Each CodeEditable editor now has its own Key.handler that handles keyboard events directly via EditorView.Focus pattern: 1. Check arrow key escape at boundaries FIRST (before delegation, since Keyboard.handle_key_event always returns Some for arrows) 2. Delegate to existing handler chain (context menu -> projector handoff -> Keyboard.handle_key_event) 3. Stop_propagation on handled keys so Page doesn't also handle them 4. Unhandled keys bubble to Page for undo/redo/F7/meta Add Key.listener (no tabindex) for Page.re since #page should catch bubbled events but not be a focus target itself. This fixes the "WARNING: not combining attributes (name tabindex)" warning. Simplify Page.re handler to only handle page-level keys (undo/redo, F7 benchmark, meta key tracking). Remove Selection.handle_key_event delegation from Page. Fix cut handler to use ActiveEditor(Destruct(Right)) directly instead of routing a fake Delete key event through Selection.handle_key_event. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Three fixes for the keyboard architecture refactor: 1. First-click focus: Always add tabindex(0) to code-editor elements, even when not selected. This way the first click gives DOM focus immediately, and the MakeActive re-render adds the key handler while preserving the existing focus. 2. Selection highlighting (black boxes): Add outline:none to .code-editor in CSS. The browser default focus outline shows when tabindex(0) makes the element focusable, but Hazel has its own visual selection indicators. 3. Projector escape: After TextAreaProj blurs itself during escape, focus the parent .code-editor element using find_ancestor_with_class. Without this, focus goes to <body> after blur and the editor stops responding to keys. Also add Key.listener (no tabindex) so Page.re catches bubbled events without becoming a focus target. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The boundary escape check was triggering inside nested expressions (e.g. between "hello" ++ "world" in a let binding) because it only checked z.caret == Outer and no neighbor on one side. But those conditions are also true at the edges of nested groups, not just at the true editor boundary. Fix: add z.relatives.ancestors == [] check, matching the splices branch. This ensures escape only triggers at the top level of the editor, not inside delimited contexts like let/in, parens, etc. Also match splices by including ArrowUp/ArrowDown in the escape check (not just ArrowLeft/ArrowRight). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> autoformatter
Replace centralized Shortcut.re with per-layer contextual actions: - Page level: undo/redo, benchmark, settings, navigation, selection, projection - ScratchMode: export, encode, buffer management - ExercisesMode: export submission, instructor exports - NinjaKeys now reads from cursor.contextual_actions (dynamic per-render) Wire EditMode into CodeEditable.View.view, replacing separate ~inject/~selected/~escape params with a single ~edit_mode. Remove dead code: EditorView.re, Shortcut.re, Selection modules from History.re and Logged.re, Page.Selection.handle_key_event. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Editors now handle their own keys via CodeEditable.key_handler_attr, making the centralized Selection.handle_key_event routing chain dead. Removed from: StepInterface (both module types), StepperBase (StepKind + Stepper dispatchers), StepperView, Theorems, AxiomsBox, all 7 step implementations, Editors, ScratchMode, TutorialsMode, TutorialMode, ExercisesMode, ExerciseMode, TheoremExerciseMode, CellEditor, EvalResult, CodeSelectable, StepperEditor. Every removed function was pure routing (delegate + wrap in local update type) with no additional logic. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Negabinary
force-pushed
the
keyboard-architecture-refactor
branch
from
4e2eeaf to
ad986d4
Compare
Negabinary
force-pushed
the
keyboard-architecture-refactor
branch
from
ad986d4 to
d865a50
Compare
Member
|
@Negabinary is this a pure refactor or does it add functionality? the boundary stuff sort of implies you can use arrows to move between cells now? |
Contributor
Author
|
@disconcision pure refactor, you can't use keyboard to go between cells - though I guess that would be easier to implement now |
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
2 participants
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
What changed
Key.handler / Key.listener — Reusable keyboard event wiring in Key.re.
handleradds tabindex(0) for focusable components;listeneris for containers that catch bubbled events (like #page).ContextualAction.t — Unified type for actions that serve both keyboard shortcuts and command palette entries. Uses Effect.t(unit) so it's decoupled from Page.Update.t. Shortcut.re keeps its Page.Update.t type internally with a to_contextual_action converter. NinjaKeys.re consumes ContextualAction.t directly.
Editors handle their own keyboard events — Each CodeEditable editor now has its own Key.handler. Events are handled locally (context menu -> projector handoff -> Keyboard.handle_key_event) with Stop_propagation. Unhandled events bubble to Page.re for page-level shortcuts (undo/redo/F7/meta).
Boundary escape — Arrow keys at the true editor boundary (caret Outer, no neighbor, no ancestors) call
~escape(direction)so parents can navigate between cells/projectors. Direction means "side to escape TO" not "key pressed".Projector escape focus — TextAreaProj now focuses the parent .code-editor element after blur during escape, so the editor regains keyboard focus.
cursor.contextual_actions — The cursor type has a new contextual_actions field and with_actions helper, enabling future per-component dynamic actions.
EditMode.re — New type bundling inject, escape, take_focus, and focus for editor views.
Non-obvious gotchas (from splices trial-and-error)