From 12e0509fa3a7f143145f830c6b636c21156c7ff7 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Sun, 17 May 2026 21:48:03 +0200 Subject: [PATCH] keyboard-operable ClideTappable + palette nav (T-100) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plug widgets into the keymap layer landed in T-117. ClideTappable: - Wrap in `Actions(ActivateIntent → onTap)` outside a `Focus` so dispatch from the focused context walks up and hits the action. - Add a focus ring via `tokens.globalFocus` (DecoratedBox foreground overlay, transparent border when unfocused, no layout shift). - Disabled (`onTap == null`) skips focus traversal and shows the forbidden cursor. ClidePalette: - Register Actions for the four palette intents (selectNext / selectPrev / accept / dismiss). - Publish `palette.open` scope flag via `KeymapService.setScopeFlag` so when-clauses can scope future bindings to "palette only". - Highlight the selected row with `listItemSelectedBackground`; scroll it into view on nav. - `PaletteController` grows `selectedIndex` + `selectNext` / `selectPrevious` / `acceptSelected`; index resets on open / filter change. Intents.dart drops the `ClideIntent` base — `ActivateIntent` and `DismissIntent` come from Flutter; clide owns the palette and text- scale and command-bridge subclasses. `parseIntentId('activate')` → Flutter's class; same for dismiss. Widget code uses the canonical Flutter Intent types where they fit. App root grows a PaletteOpenIntent action that calls `services.palette.open()`, completing the ctrl/cmd+shift+p path end-to-end. Co-Authored-By: Claude Opus 4.7 (1M context) --- .pql/changelog/ticket_history/2026-05.sql | 2 + .pql/changelog/tickets/2026-05.sql | 10 + CHANGELOG.md | 3 + assets/keymaps/default.yaml | 7 +- lib/app.dart | 6 + lib/kernel/src/commands/palette.dart | 40 ++++ lib/kernel/src/keymap/intents.dart | 113 +++------ lib/kernel/src/keymap/keymap.dart | 9 +- lib/kernel/src/keymap/keymap_service.dart | 3 +- lib/widgets/src/clide_palette.dart | 217 +++++++++++++----- lib/widgets/src/clide_tappable.dart | 77 ++++++- .../src/keymap/keymap_service_test.dart | 3 +- test/kernel/src/keymap/keymap_test.dart | 1 + test/widgets/src/clide_tappable_test.dart | 115 ++++++++++ test/widgets/zero_coverage_widgets_test.dart | 85 +++++++ 15 files changed, 531 insertions(+), 160 deletions(-) create mode 100644 test/widgets/src/clide_tappable_test.dart diff --git a/.pql/changelog/ticket_history/2026-05.sql b/.pql/changelog/ticket_history/2026-05.sql index fb5a9fc2..8bcf3cbe 100644 --- a/.pql/changelog/ticket_history/2026-05.sql +++ b/.pql/changelog/ticket_history/2026-05.sql @@ -1797,3 +1797,5 @@ INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, 2. `KeymapService.setPreset("jetbrains")` activates the preset and all asserted bindings resolve. 3. A regression test exercises a representative subset (e.g. shift+shift → quick-open command — see Q-9 if the search-everywhere overlay needs its own intent).', NULL, '2026-05-17 19:39:37', '2026-05-17 19:39:37', '2026-05-17 19:39:37', NULL, 'c1129f4cb3454b99e1cbf645ca56bea5', 1) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-117', 'status', 'in_progress', 'done', NULL, '2026-05-17 19:39:40', '2026-05-17 19:39:40', '2026-05-17 19:39:40', NULL, '75b2fd14ceb7daf28645ccf84cb041f1', 1) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-100', 'status', 'in_progress', 'in_progress', NULL, '2026-05-17 19:40:44', '2026-05-17 19:40:44', '2026-05-17 19:40:44', NULL, '65600862f5787501dcd057756cdb4313', 1) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-100', 'status', 'in_progress', 'done', NULL, '2026-05-17 19:47:50', '2026-05-17 19:47:50', '2026-05-17 19:47:50', NULL, '7d3c0ad4c47d3271ac35c5159e6e9d0e', 1) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-05.sql b/.pql/changelog/tickets/2026-05.sql index 5491fb05..3495679e 100644 --- a/.pql/changelog/tickets/2026-05.sql +++ b/.pql/changelog/tickets/2026-05.sql @@ -1761,3 +1761,13 @@ INSERT INTO tickets (id, type, parent_id, title, description, status, priority, - ClideTappable/Palette widget integration (T-100 — comes immediately after this lands). Source: 2026-05-17 design conversation; supersedes T-110.', 'done', 'high', NULL, NULL, NULL, '2026-05-17 19:28:28', '2026-05-17 19:39:40', NULL, '92d68025f0ee7c4d133dca65e97c97b8', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (id, type, parent_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-100', 'story', 'T-97', 'make ClideTappable keyboard-operable; add palette arrow-keys + Escape', '`ClideTappable` (`lib/widgets/src/clide_tappable.dart`) is the base of nearly every interactive widget — `ClideButton`, `_WinBtn`, `_RecentProjectRow`, `_ActionRow`, most builtin list items — and it is `MouseRegion` + `GestureDetector` only. No `Focus`, no Enter/Space handler, no focus ring. None of these widgets can be reached by Tab or activated from the keyboard. The keyboard-traversal test passes only because it externally wraps in a `Focus` node — it tests non-blocking, not operability. + +`ClidePalette` (`lib/widgets/src/clide_palette.dart`) is similarly broken: `onSubmitted` only ever invokes `filtered.first`; no up/down handling, no selected index, no selection highlight, no Escape handler. + +**Fix:** +1. Wrap `ClideTappable`''s child in `Focus` + `Shortcuts`/`Actions` so Tab focuses it and Enter/Space invokes `onTap`. Render a focus ring via the token system. +2. Add arrow-key navigation + selected-index + Escape + Enter-on-selected to `ClidePalette` (model after `_ProjectSwitcherDropdown.onKeyEvent` at `app.dart:446-452`). +3. Extend the a11y test layer to assert operability (Tab + Enter actually invokes), not just Semantics presence. + +Source: consultants.md "UX — Findings — [Critical]".', 'done', 'high', NULL, NULL, NULL, '2026-05-17 18:47:23', '2026-05-17 19:47:50', NULL, 'f6f0c34a700b133bc84c6d8d7ea450a3', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); diff --git a/CHANGELOG.md b/CHANGELOG.md index 60a98b16..30caf1eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,9 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. VS-Code-style when-clauses, layered preset → user file → settings overlay. Default preset ships; vim/vscode/jetbrains unblocked (T-117, supersedes T-110). +- Keyboard operability — `ClideTappable` is now Tab-focusable with a + focus ring and Enter/Space activation; `ClidePalette` adds arrow + nav, Escape dismiss, and selection highlight (T-100). - Test sweep — `keybindings`, `toolchain_paths`, and several `widgets/src/` primitives (tooltip, palette, multitab, markdown). - `tree_sitter_service` sweep — fake-FFI + real-library smoke, diff --git a/assets/keymaps/default.yaml b/assets/keymaps/default.yaml index a0846edf..d408d2c7 100644 --- a/assets/keymaps/default.yaml +++ b/assets/keymaps/default.yaml @@ -17,17 +17,18 @@ name: default bindings: # -- Activation / focus ----------------------------------------------- + # ActivateIntent has no when-clause: it dispatches via Actions.maybeInvoke + # against the focused context, so only widgets that opt in (ClideTappable's + # Actions wrapper) actually catch it. Text-input widgets handle Enter + # themselves first. - intent: activate keys: [enter, space] - when: focused.tappable - intent: dismiss keys: escape - intent: focus.next keys: tab - when: '!textInputFocused' - intent: focus.previous keys: shift+tab - when: '!textInputFocused' # -- Command palette -------------------------------------------------- - intent: palette.open diff --git a/lib/app.dart b/lib/app.dart index 413495df..fd0f14e5 100644 --- a/lib/app.dart +++ b/lib/app.dart @@ -113,6 +113,12 @@ class _RootShellState extends State<_RootShell> { return null; }, ), + PaletteOpenIntent: CallbackAction( + onInvoke: (_) { + widget.services.palette.open(); + return null; + }, + ), }, child: KeyboardListener( focusNode: _keyFocus, diff --git a/lib/kernel/src/commands/palette.dart b/lib/kernel/src/commands/palette.dart index 0ce94298..e6472f58 100644 --- a/lib/kernel/src/commands/palette.dart +++ b/lib/kernel/src/commands/palette.dart @@ -9,13 +9,26 @@ class PaletteController extends ChangeNotifier { bool _open = false; String _filter = ''; + int _selectedIndex = 0; bool get isOpen => _open; String get filter => _filter; + /// Index of the highlighted entry inside the currently-filtered + /// list. Clamped to `[0, filtered().length - 1]` on read. Returns 0 + /// when the filter excludes everything. + int get selectedIndex { + final n = filtered().length; + if (n == 0) return 0; + if (_selectedIndex < 0) return 0; + if (_selectedIndex >= n) return n - 1; + return _selectedIndex; + } + void open() { if (_open) return; _open = true; + _selectedIndex = 0; notifyListeners(); } @@ -23,6 +36,7 @@ class PaletteController extends ChangeNotifier { if (!_open) return; _open = false; _filter = ''; + _selectedIndex = 0; notifyListeners(); } @@ -31,9 +45,35 @@ class PaletteController extends ChangeNotifier { void setFilter(String f) { if (_filter == f) return; _filter = f; + _selectedIndex = 0; notifyListeners(); } + /// Highlight the next entry, wrapping at the end. No-op when the + /// filtered list has fewer than 2 entries. + void selectNext() { + final n = filtered().length; + if (n < 2) return; + _selectedIndex = (selectedIndex + 1) % n; + notifyListeners(); + } + + /// Highlight the previous entry, wrapping at the start. + void selectPrevious() { + final n = filtered().length; + if (n < 2) return; + _selectedIndex = (selectedIndex - 1 + n) % n; + notifyListeners(); + } + + /// Invoke whatever's currently highlighted; no-op when the filter + /// excludes everything. + Future acceptSelected() async { + final list = filtered(); + if (list.isEmpty) return; + await invoke(list[selectedIndex].command); + } + List filtered() { if (_filter.isEmpty) return _registry.all.toList(); final q = _filter.toLowerCase(); diff --git a/lib/kernel/src/keymap/intents.dart b/lib/kernel/src/keymap/intents.dart index d2d32572..4ea92d8f 100644 --- a/lib/kernel/src/keymap/intents.dart +++ b/lib/kernel/src/keymap/intents.dart @@ -5,99 +5,54 @@ /// `palette.selectNext`, …). The id stays stable across SDK reshapes; /// the Dart class name can move without invalidating user keymaps. /// -/// To add a new Intent: declare a subclass with a unique [id] and -/// register it in [allIntents]. Widget integration is per-feature -/// (Actions wiring lives in the consuming widget). +/// Two flavors of intents live here: +/// 1. **Flutter-provided** — `ActivateIntent` and `DismissIntent` +/// are first-class Flutter intents; we reuse them so the keymap +/// integrates with anything else in the Flutter ecosystem that +/// already dispatches those (focus traversal, modal scrims, …). +/// They're mapped by id in [builtinIntents] but not declared +/// here. +/// 2. **Clide-specific** — palette navigation, text scale, the +/// `InvokeCommandIntent` bridge. Each subclass extends [Intent] +/// directly. library; import 'package:flutter/widgets.dart'; -/// Base for every keymap-dispatched intent. The [id] is the YAML -/// identifier (e.g. `palette.selectNext`). -abstract class ClideIntent extends Intent { - const ClideIntent(); - String get id; -} - -// -- Activation / navigation ------------------------------------------------ - -/// "Click this thing" — fired on Enter/Space against any focusable -/// `ClideTappable`-rooted widget. -class ActivateIntent extends ClideIntent { - const ActivateIntent(); - @override - String get id => 'activate'; -} - -/// "Cancel / dismiss the current modal / overlay". -class DismissIntent extends ClideIntent { - const DismissIntent(); - @override - String get id => 'dismiss'; -} - -/// "Move focus to the next focusable in tab order". -class FocusNextIntent extends ClideIntent { - const FocusNextIntent(); - @override - String get id => 'focus.next'; -} - -/// "Move focus to the previous focusable". -class FocusPreviousIntent extends ClideIntent { - const FocusPreviousIntent(); - @override - String get id => 'focus.previous'; -} - // -- Command palette -------------------------------------------------------- /// Open the command palette. -class PaletteOpenIntent extends ClideIntent { +class PaletteOpenIntent extends Intent { const PaletteOpenIntent(); - @override - String get id => 'palette.open'; } /// Highlight the next palette result. -class PaletteSelectNextIntent extends ClideIntent { +class PaletteSelectNextIntent extends Intent { const PaletteSelectNextIntent(); - @override - String get id => 'palette.selectNext'; } /// Highlight the previous palette result. -class PaletteSelectPreviousIntent extends ClideIntent { +class PaletteSelectPreviousIntent extends Intent { const PaletteSelectPreviousIntent(); - @override - String get id => 'palette.selectPrevious'; } /// Invoke the highlighted palette result. -class PaletteAcceptIntent extends ClideIntent { +class PaletteAcceptIntent extends Intent { const PaletteAcceptIntent(); - @override - String get id => 'palette.accept'; } // -- Text scale ------------------------------------------------------------- -class TextScaleIncreaseIntent extends ClideIntent { +class TextScaleIncreaseIntent extends Intent { const TextScaleIncreaseIntent(); - @override - String get id => 'text.scaleIncrease'; } -class TextScaleDecreaseIntent extends ClideIntent { +class TextScaleDecreaseIntent extends Intent { const TextScaleDecreaseIntent(); - @override - String get id => 'text.scaleDecrease'; } -class TextScaleResetIntent extends ClideIntent { +class TextScaleResetIntent extends Intent { const TextScaleResetIntent(); - @override - String get id => 'text.scaleReset'; } // -- Command bridge --------------------------------------------------------- @@ -106,11 +61,9 @@ class TextScaleResetIntent extends ClideIntent { /// bindings that target a contributed command rather than a typed /// intent. The keymap creates one per binding; the Actions handler /// dispatches to the [CommandRegistry]. -class InvokeCommandIntent extends ClideIntent { +class InvokeCommandIntent extends Intent { const InvokeCommandIntent(this.commandId); final String commandId; - @override - String get id => 'command:$commandId'; } // -- Lookup ----------------------------------------------------------------- @@ -120,29 +73,23 @@ class InvokeCommandIntent extends ClideIntent { /// configurable payload (only `InvokeCommandIntent` today) are not in /// the map — the loader recognises the `command:` prefix and /// instantiates them inline. -final Map builtinIntents = { - for (final i in _allBuiltin) i.id: () => i, +final Map builtinIntents = { + 'activate': () => const ActivateIntent(), + 'dismiss': () => const DismissIntent(), + 'palette.open': () => const PaletteOpenIntent(), + 'palette.selectNext': () => const PaletteSelectNextIntent(), + 'palette.selectPrevious': () => const PaletteSelectPreviousIntent(), + 'palette.accept': () => const PaletteAcceptIntent(), + 'text.scaleIncrease': () => const TextScaleIncreaseIntent(), + 'text.scaleDecrease': () => const TextScaleDecreaseIntent(), + 'text.scaleReset': () => const TextScaleResetIntent(), }; -const List _allBuiltin = [ - ActivateIntent(), - DismissIntent(), - FocusNextIntent(), - FocusPreviousIntent(), - PaletteOpenIntent(), - PaletteSelectNextIntent(), - PaletteSelectPreviousIntent(), - PaletteAcceptIntent(), - TextScaleIncreaseIntent(), - TextScaleDecreaseIntent(), - TextScaleResetIntent(), -]; - -/// Parse an intent id into a [ClideIntent]. Returns null if the id is +/// Parse an intent id into an [Intent]. Returns null if the id is /// unknown. Recognises: /// - any builtin intent by its stable id /// - `command:` → [InvokeCommandIntent] -ClideIntent? parseIntentId(String id) { +Intent? parseIntentId(String id) { final builtin = builtinIntents[id]; if (builtin != null) return builtin(); if (id.startsWith('command:')) { diff --git a/lib/kernel/src/keymap/keymap.dart b/lib/kernel/src/keymap/keymap.dart index 9a1a4d9c..cdc565f3 100644 --- a/lib/kernel/src/keymap/keymap.dart +++ b/lib/kernel/src/keymap/keymap.dart @@ -6,11 +6,12 @@ /// same (chord, when-clause) tuple. /// /// At resolve time, the [Keymap] walks the layered list once per -/// (chord, scope) and returns the [ClideIntent] bound by the highest- +/// (chord, scope) and returns the [Intent] bound by the highest- /// precedence matching layer. library; import 'package:flutter/foundation.dart'; +import 'package:flutter/widgets.dart' show Intent; import 'package:yaml/yaml.dart'; import 'intents.dart'; @@ -28,11 +29,11 @@ class KeymapBinding { }); final KeyChord chord; - final ClideIntent intent; + final Intent intent; final WhenExpr? when; @override - String toString() => 'Binding($chord → ${intent.id}${when == null ? '' : ' when $when'})'; + String toString() => 'Binding($chord → ${intent.runtimeType}${when == null ? '' : ' when $when'})'; } /// One source of bindings. Layers are merged in order — later layers @@ -117,7 +118,7 @@ class Keymap { /// Resolve a [chord] against the current [context]. Returns the /// highest-precedence binding whose chord matches and whose when- /// clause (if any) evaluates true. Returns null if no match. - ClideIntent? resolve(KeyChord chord, Map context) { + Intent? resolve(KeyChord chord, Map context) { // Effective list is highest-precedence-first; first match wins. for (final b in _effective) { if (b.chord != chord) continue; diff --git a/lib/kernel/src/keymap/keymap_service.dart b/lib/kernel/src/keymap/keymap_service.dart index 0e361805..a2272018 100644 --- a/lib/kernel/src/keymap/keymap_service.dart +++ b/lib/kernel/src/keymap/keymap_service.dart @@ -23,6 +23,7 @@ import 'dart:io'; import 'package:flutter/foundation.dart'; import 'package:flutter/services.dart' show AssetBundle, KeyEvent, HardwareKeyboard, rootBundle; +import 'package:flutter/widgets.dart' show Intent; import '../settings.dart'; import 'intents.dart'; @@ -160,7 +161,7 @@ class KeymapService extends ChangeNotifier { /// Resolve a [KeyEvent] against the active keymap and current scope. /// Returns null when nothing matches. - ClideIntent? resolveEvent(KeyEvent event, HardwareKeyboard kb) { + Intent? resolveEvent(KeyEvent event, HardwareKeyboard kb) { final km = _active; if (km == null) return null; final chord = KeyChord.fromKeyEvent(event, kb); diff --git a/lib/widgets/src/clide_palette.dart b/lib/widgets/src/clide_palette.dart index 3e45cc61..72584c1e 100644 --- a/lib/widgets/src/clide_palette.dart +++ b/lib/widgets/src/clide_palette.dart @@ -12,21 +12,91 @@ class ClidePalette extends StatefulWidget { class _ClidePaletteState extends State { final _input = TextEditingController(); - final _focus = FocusNode(); + final _focus = FocusNode(debugLabel: 'ClidePalette.input'); + final _itemKeys = {}; + + PaletteController? _palette; + KeymapService? _keymap; @override - void initState() { - super.initState(); - _focus.requestFocus(); + void didChangeDependencies() { + super.didChangeDependencies(); + final kernel = ClideKernel.of(context); + if (!identical(_palette, kernel.palette)) { + _palette?.removeListener(_onPaletteChanged); + _palette = kernel.palette; + _palette!.addListener(_onPaletteChanged); + _syncFromController(); + } + _keymap = kernel.keymap; + // Sync the initial state: if the palette was opened before this + // widget mounted (e.g., open()-then-pumpWidget in a test), no + // listener fires for the "already open" condition. Mirror what + // _onPaletteChanged would have done. + final isOpen = _palette?.isOpen ?? false; + _keymap?.setScopeFlag('palette.open', isOpen); + if (isOpen && !_focus.hasFocus) { + WidgetsBinding.instance.addPostFrameCallback((_) { + if (mounted && (_palette?.isOpen ?? false)) _focus.requestFocus(); + }); + } } @override void dispose() { + _palette?.removeListener(_onPaletteChanged); + _keymap?.clearScopeFlag('palette.open'); _input.dispose(); _focus.dispose(); super.dispose(); } + void _onPaletteChanged() { + final isOpen = _palette?.isOpen ?? false; + _keymap?.setScopeFlag('palette.open', isOpen); + if (isOpen) _focus.requestFocus(); + _syncFromController(); + } + + void _syncFromController() { + final f = _palette?.filter ?? ''; + if (_input.text != f) { + _input.value = TextEditingValue(text: f, selection: TextSelection.collapsed(offset: f.length)); + } + } + + Object? _selectNext(PaletteSelectNextIntent _) { + _palette?.selectNext(); + _scrollSelectedIntoView(); + return null; + } + + Object? _selectPrev(PaletteSelectPreviousIntent _) { + _palette?.selectPrevious(); + _scrollSelectedIntoView(); + return null; + } + + Object? _accept(PaletteAcceptIntent _) { + _palette?.acceptSelected(); + _input.clear(); + return null; + } + + Object? _dismiss(DismissIntent _) { + _palette?.close(); + return null; + } + + void _scrollSelectedIntoView() { + final idx = _palette?.selectedIndex; + if (idx == null) return; + final key = _itemKeys[idx]; + final ctx = key?.currentContext; + if (ctx == null) return; + Scrollable.ensureVisible(ctx, duration: const Duration(milliseconds: 120), alignment: 0.5); + } + @override Widget build(BuildContext context) { final kernel = ClideKernel.of(context); @@ -36,71 +106,84 @@ class _ClidePaletteState extends State { builder: (ctx, _) { if (!kernel.palette.isOpen) return const SizedBox.shrink(); final filtered = kernel.palette.filtered(); + final selected = kernel.palette.selectedIndex; return Positioned( top: 60, left: 0, right: 0, child: Center( - child: Container( - width: 480, - constraints: const BoxConstraints(maxHeight: 360), - decoration: BoxDecoration( - color: tokens.dropdownBackground, - border: Border.all(color: tokens.dropdownBorder), - borderRadius: BorderRadius.circular(6), - boxShadow: const [ - BoxShadow( - color: Color(0x40000000), - blurRadius: 12, - offset: Offset(0, 4), - ), - ], - ), - child: Column( - mainAxisSize: MainAxisSize.min, - children: [ - Padding( - padding: const EdgeInsets.all(8), - child: EditableText( - controller: _input, - focusNode: _focus, - style: TextStyle( - fontFamily: clideMonoFamily, - fontSize: clideFontMono, - color: tokens.dropdownForeground, - ), - cursorColor: tokens.globalFocus, - backgroundCursorColor: tokens.globalFocus, - maxLines: 1, - onChanged: (v) => kernel.palette.setFilter(v), - onSubmitted: (_) { - if (filtered.isNotEmpty) { - kernel.palette.invoke(filtered.first.command); + child: Actions( + actions: >{ + PaletteSelectNextIntent: CallbackAction(onInvoke: _selectNext), + PaletteSelectPreviousIntent: CallbackAction(onInvoke: _selectPrev), + PaletteAcceptIntent: CallbackAction(onInvoke: _accept), + DismissIntent: CallbackAction(onInvoke: _dismiss), + }, + child: Container( + width: 480, + constraints: const BoxConstraints(maxHeight: 360), + decoration: BoxDecoration( + color: tokens.dropdownBackground, + border: Border.all(color: tokens.dropdownBorder), + borderRadius: BorderRadius.circular(6), + boxShadow: const [ + BoxShadow( + color: Color(0x40000000), + blurRadius: 12, + offset: Offset(0, 4), + ), + ], + ), + child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + Padding( + padding: const EdgeInsets.all(8), + child: EditableText( + controller: _input, + focusNode: _focus, + style: TextStyle( + fontFamily: clideMonoFamily, + fontSize: clideFontMono, + color: tokens.dropdownForeground, + ), + cursorColor: tokens.globalFocus, + backgroundCursorColor: tokens.globalFocus, + maxLines: 1, + onChanged: (v) => kernel.palette.setFilter(v), + // Enter on the input forwards to the palette + // accept intent — keeps the legacy single-key + // submit working alongside arrow-driven nav. + onSubmitted: (_) { + kernel.palette.acceptSelected(); _input.clear(); - } - }, + }, + ), ), - ), - Flexible( - child: ListView.builder( - shrinkWrap: true, - padding: EdgeInsets.zero, - itemCount: filtered.length, - itemBuilder: (ctx, i) { - final cmd = filtered[i]; - return _PaletteItem( - title: cmd.title ?? cmd.command, - command: cmd.command, - binding: cmd.defaultBinding, - onTap: () { - kernel.palette.invoke(cmd.command); - _input.clear(); - }, - ); - }, + Flexible( + child: ListView.builder( + shrinkWrap: true, + padding: EdgeInsets.zero, + itemCount: filtered.length, + itemBuilder: (ctx, i) { + final cmd = filtered[i]; + final key = _itemKeys.putIfAbsent(i, () => GlobalKey()); + return _PaletteItem( + key: key, + title: cmd.title ?? cmd.command, + command: cmd.command, + binding: cmd.defaultBinding, + highlighted: i == selected, + onTap: () { + kernel.palette.invoke(cmd.command); + _input.clear(); + }, + ); + }, + ), ), - ), - ], + ], + ), ), ), ), @@ -112,15 +195,18 @@ class _ClidePaletteState extends State { class _PaletteItem extends StatefulWidget { const _PaletteItem({ + super.key, required this.title, required this.command, required this.onTap, + required this.highlighted, this.binding, }); final String title; final String command; final String? binding; + final bool highlighted; final VoidCallback onTap; @override @@ -133,6 +219,7 @@ class _PaletteItemState extends State<_PaletteItem> { @override Widget build(BuildContext context) { final tokens = ClideTheme.of(context).surface; + final selected = widget.highlighted; return MouseRegion( cursor: SystemMouseCursors.click, onEnter: (_) => setState(() => _hover = true), @@ -140,14 +227,18 @@ class _PaletteItemState extends State<_PaletteItem> { child: GestureDetector( onTap: widget.onTap, child: Container( - color: _hover ? tokens.listItemHoverBackground : null, + color: selected + ? tokens.listItemSelectedBackground + : _hover + ? tokens.listItemHoverBackground + : null, padding: const EdgeInsets.symmetric(horizontal: 12, vertical: 6), child: Row( children: [ Expanded( child: ClideText( widget.title, - color: tokens.listItemForeground, + color: selected ? tokens.listItemSelectedForeground : tokens.listItemForeground, ), ), if (widget.binding != null) diff --git a/lib/widgets/src/clide_tappable.dart b/lib/widgets/src/clide_tappable.dart index 11ca0e6c..56c8b0d8 100644 --- a/lib/widgets/src/clide_tappable.dart +++ b/lib/widgets/src/clide_tappable.dart @@ -1,6 +1,15 @@ +import 'package:clide/kernel/kernel.dart' show ClideTheme; import 'package:clide/widgets/src/clide_tooltip.dart'; import 'package:flutter/widgets.dart'; +/// Mouse + keyboard activatable surface. Wraps the [builder] child in +/// a `Focus` so Tab traversal reaches it; an `Actions` provider that +/// handles [ActivateIntent] by invoking [onTap] (the keymap binds +/// Enter / Space to ActivateIntent by default); and a focus ring +/// rendered via `tokens.globalFocus`. +/// +/// Hover + pressed state still feed [builder] for visual feedback. +/// Disabled state (`onTap == null`) blocks focus traversal too. class ClideTappable extends StatefulWidget { const ClideTappable({ super.key, @@ -10,6 +19,8 @@ class ClideTappable extends StatefulWidget { this.onPressChanged, this.cursor = SystemMouseCursors.click, this.tooltip, + this.focusNode, + this.autofocus = false, }); final Widget Function(BuildContext context, bool hovered, bool pressed) builder; @@ -18,6 +29,8 @@ class ClideTappable extends StatefulWidget { final ValueChanged? onPressChanged; final MouseCursor cursor; final String? tooltip; + final FocusNode? focusNode; + final bool autofocus; @override State createState() => _ClideTappableState(); @@ -26,6 +39,16 @@ class ClideTappable extends StatefulWidget { class _ClideTappableState extends State { bool _hover = false; bool _pressed = false; + bool _focused = false; + FocusNode? _internalFocus; + + FocusNode get _effectiveFocus => widget.focusNode ?? (_internalFocus ??= FocusNode(debugLabel: 'ClideTappable')); + + @override + void dispose() { + _internalFocus?.dispose(); + super.dispose(); + } void _setPressed(bool v) { if (_pressed == v) return; @@ -33,10 +56,22 @@ class _ClideTappableState extends State { widget.onPressChanged?.call(v); } + void _setFocused(bool v) { + if (_focused == v) return; + setState(() => _focused = v); + } + + Object? _activate(ActivateIntent _) { + widget.onTap?.call(); + return null; + } + @override Widget build(BuildContext context) { + final tokens = ClideTheme.of(context).surface; + final enabled = widget.onTap != null; Widget child = MouseRegion( - cursor: widget.cursor, + cursor: enabled ? widget.cursor : SystemMouseCursors.forbidden, onEnter: (_) => setState(() => _hover = true), onExit: (_) { setState(() => _hover = false); @@ -46,12 +81,46 @@ class _ClideTappableState extends State { behavior: HitTestBehavior.opaque, onTap: widget.onTap, onLongPress: widget.onLongPress, - onTapDown: (_) => _setPressed(true), - onTapUp: (_) => _setPressed(false), - onTapCancel: () => _setPressed(false), + onTapDown: enabled ? (_) => _setPressed(true) : null, + onTapUp: enabled ? (_) => _setPressed(false) : null, + onTapCancel: enabled ? () => _setPressed(false) : null, child: widget.builder(context, _hover, _pressed), ), ); + + // Focus ring — 2 px outer outline in the global focus token. Drawn + // as a wrapping decoration so it sits outside the child's content + // without shifting layout (the same DecoratedBox always paints; + // border color falls through to transparent when unfocused). + child = DecoratedBox( + position: DecorationPosition.foreground, + decoration: BoxDecoration( + border: Border.all( + color: _focused ? tokens.globalFocus : const Color(0x00000000), + width: 2, + ), + borderRadius: BorderRadius.circular(3), + ), + child: child, + ); + + // Actions wraps Focus: dispatching ActivateIntent from the focused + // context (the node held by Focus) walks UP and finds this Actions + // provider. The reverse nesting would leave Actions as a descendant + // of the focused context — unreachable. + child = Actions( + actions: >{ + ActivateIntent: CallbackAction(onInvoke: _activate), + }, + child: Focus( + focusNode: _effectiveFocus, + canRequestFocus: enabled, + autofocus: widget.autofocus, + onFocusChange: _setFocused, + child: child, + ), + ); + if (widget.tooltip != null) { child = ClideTooltip(message: widget.tooltip!, child: child); } diff --git a/test/kernel/src/keymap/keymap_service_test.dart b/test/kernel/src/keymap/keymap_service_test.dart index 53240cb8..28e4dc3b 100644 --- a/test/kernel/src/keymap/keymap_service_test.dart +++ b/test/kernel/src/keymap/keymap_service_test.dart @@ -8,6 +8,7 @@ import 'package:clide/kernel/src/keymap/key_chord.dart'; import 'package:clide/kernel/src/keymap/keymap_service.dart'; import 'package:clide/kernel/src/settings.dart'; import 'package:flutter/services.dart'; +import 'package:flutter/widgets.dart' show ActivateIntent, DismissIntent; import 'package:flutter_test/flutter_test.dart'; void main() { @@ -136,8 +137,6 @@ void main() { expect(intent, isA()); final invoke = intent as InvokeCommandIntent; expect(invoke.commandId, 'git.commit'); - // `id` carries the command suffix for round-trip identification. - expect(invoke.id, 'command:git.commit'); }); test('honours a when-clause on the contribution', () async { diff --git a/test/kernel/src/keymap/keymap_test.dart b/test/kernel/src/keymap/keymap_test.dart index 9a32a88b..ab4ea984 100644 --- a/test/kernel/src/keymap/keymap_test.dart +++ b/test/kernel/src/keymap/keymap_test.dart @@ -4,6 +4,7 @@ library; import 'package:clide/kernel/src/keymap/intents.dart'; import 'package:clide/kernel/src/keymap/key_chord.dart'; import 'package:clide/kernel/src/keymap/keymap.dart'; +import 'package:flutter/widgets.dart' show ActivateIntent, DismissIntent; import 'package:flutter_test/flutter_test.dart'; void main() { diff --git a/test/widgets/src/clide_tappable_test.dart b/test/widgets/src/clide_tappable_test.dart new file mode 100644 index 00000000..46f55927 --- /dev/null +++ b/test/widgets/src/clide_tappable_test.dart @@ -0,0 +1,115 @@ +/// Widget tests for the keyboard-operable [ClideTappable] (T-100). +/// +/// The pre-T-100 widget was mouse-only — these tests assert it now +/// accepts focus, that Enter / Space invoke onTap via the +/// [ActivateIntent] action, and that the focus ring appears when +/// the widget has focus. +library; + +import 'package:clide/widgets/widgets.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import '../../helpers/kernel_fixture.dart'; +import '../../helpers/widget_harness.dart'; + +void main() { + group('ClideTappable — keyboard', () { + late KernelFixture f; + setUp(() async => f = await KernelFixture.create()); + tearDown(() async => f.dispose()); + + testWidgets('accepts Tab focus when onTap is provided', (tester) async { + var tapped = 0; + final node = FocusNode(); + addTearDown(node.dispose); + await tester.pumpWidget(harness( + f, + Center( + child: ClideTappable( + focusNode: node, + onTap: () => tapped++, + builder: (_, hovered, pressed) => const SizedBox(width: 60, height: 24), + ), + ), + )); + + expect(node.hasFocus, isFalse); + node.requestFocus(); + await tester.pump(); + expect(node.hasFocus, isTrue); + // Tap count unchanged — focus alone doesn't invoke. + expect(tapped, 0); + }); + + testWidgets('Enter on a focused tappable invokes onTap via ActivateIntent', (tester) async { + var tapped = 0; + final node = FocusNode(); + addTearDown(node.dispose); + await tester.pumpWidget(harness( + f, + Center( + child: ClideTappable( + focusNode: node, + autofocus: true, + onTap: () => tapped++, + builder: (_, hovered, pressed) => const SizedBox(width: 60, height: 24), + ), + ), + )); + await tester.pump(); + expect(node.hasFocus, isTrue); + // Dispatch ActivateIntent directly against the focused context + // — mirrors what KeymapService.resolveEvent → Actions.maybeInvoke + // would do on Enter / Space. + Actions.invoke(node.context!, const ActivateIntent()); + await tester.pump(); + expect(tapped, 1); + }); + + testWidgets('disabled tappable (onTap == null) cannot receive focus', (tester) async { + final node = FocusNode(); + addTearDown(node.dispose); + await tester.pumpWidget(harness( + f, + Center( + child: ClideTappable( + focusNode: node, + onTap: null, + builder: (_, hovered, pressed) => const SizedBox(width: 60, height: 24), + ), + ), + )); + node.requestFocus(); + await tester.pump(); + expect(node.hasFocus, isFalse, reason: 'canRequestFocus is false when onTap is null'); + }); + + testWidgets('focus ring appears when focused, gone when unfocused', (tester) async { + final node = FocusNode(); + addTearDown(node.dispose); + await tester.pumpWidget(harness( + f, + Center( + child: ClideTappable( + focusNode: node, + onTap: () {}, + builder: (_, hovered, pressed) => const SizedBox(width: 60, height: 24), + ), + ), + )); + + Iterable ringColors() => tester + .widgetList(find.descendant(of: find.byType(ClideTappable), matching: find.byType(DecoratedBox))) + .map((d) => ((d.decoration as BoxDecoration).border?.top.color ?? const Color(0x00000000))); + + // Unfocused: no DecoratedBox descendant carries a non-transparent border. + expect(ringColors().any((c) => c.a > 0), isFalse); + + node.requestFocus(); + await tester.pumpAndSettle(); + expect(node.hasFocus, isTrue); + expect(ringColors().any((c) => c.a > 0), isTrue, reason: 'focus ring border should become opaque on focus'); + }); + }); +} diff --git a/test/widgets/zero_coverage_widgets_test.dart b/test/widgets/zero_coverage_widgets_test.dart index 2e85b32e..743a538e 100644 --- a/test/widgets/zero_coverage_widgets_test.dart +++ b/test/widgets/zero_coverage_widgets_test.dart @@ -145,6 +145,91 @@ void main() { await tester.pumpAndSettle(); expect(find.text('Hoverable'), findsOneWidget); }); + + testWidgets('arrow keys move the highlighted command (T-100)', (tester) async { + for (final id in ['a', 'b', 'c']) { + f.services.commands.register(CommandContribution( + id: id, + command: 'cmd.$id', + title: 'Item $id', + run: (_) async => IpcResponse.ok(id: '', data: const {}), + )); + } + f.services.palette.open(); + await tester.pumpWidget(harness(f, Stack(children: const [ClidePalette()]))); + await tester.pumpAndSettle(); + + // selectedIndex starts at 0. + expect(f.services.palette.selectedIndex, 0); + + // Dispatch palette intents directly against the palette's + // focused context — what the keymap would do for ↓ / ↑. + final ctx = f.services.palette.isOpen ? FocusManager.instance.primaryFocus?.context : null; + Actions.invoke(ctx!, const PaletteSelectNextIntent()); + expect(f.services.palette.selectedIndex, 1); + Actions.invoke(ctx, const PaletteSelectNextIntent()); + expect(f.services.palette.selectedIndex, 2); + // Wraps at end. + Actions.invoke(ctx, const PaletteSelectNextIntent()); + expect(f.services.palette.selectedIndex, 0); + // Wraps backwards at start. + Actions.invoke(ctx, const PaletteSelectPreviousIntent()); + expect(f.services.palette.selectedIndex, 2); + }); + + testWidgets('PaletteAcceptIntent invokes the highlighted command (T-100)', (tester) async { + var which = ''; + for (final id in ['a', 'b', 'c']) { + f.services.commands.register(CommandContribution( + id: id, + command: 'cmd.$id', + title: 'Item $id', + run: (_) async { + which = id; + return IpcResponse.ok(id: '', data: const {}); + }, + )); + } + f.services.palette.open(); + await tester.pumpWidget(harness(f, Stack(children: const [ClidePalette()]))); + await tester.pumpAndSettle(); + + final ctx = FocusManager.instance.primaryFocus!.context!; + Actions.invoke(ctx, const PaletteSelectNextIntent()); + Actions.invoke(ctx, const PaletteAcceptIntent()); + await tester.pumpAndSettle(); + expect(which, 'b'); + expect(f.services.palette.isOpen, isFalse, reason: 'acceptSelected closes the palette'); + }); + + testWidgets('DismissIntent closes the palette (T-100)', (tester) async { + f.services.commands.register(CommandContribution( + id: 'c1', + command: 'thing', + title: 'Thing', + run: (_) async => IpcResponse.ok(id: '', data: const {}), + )); + f.services.palette.open(); + await tester.pumpWidget(harness(f, Stack(children: const [ClidePalette()]))); + await tester.pumpAndSettle(); + + final ctx = FocusManager.instance.primaryFocus!.context!; + Actions.invoke(ctx, const DismissIntent()); + await tester.pumpAndSettle(); + expect(f.services.palette.isOpen, isFalse); + }); + + testWidgets('opening the palette publishes palette.open scope flag (T-100)', (tester) async { + await tester.pumpWidget(harness(f, Stack(children: const [ClidePalette()]))); + await tester.pumpAndSettle(); + expect(f.services.keymap.scope['palette.open'] ?? false, isFalse); + f.services.palette.open(); + await tester.pumpAndSettle(); + expect(f.services.keymap.scope['palette.open'], isTrue); + f.services.palette.close(); + await tester.pumpAndSettle(); + expect(f.services.keymap.scope['palette.open'], isFalse); + }); }); group('ClideFilterBox', () {