From 740cc7bb7b622e25f607f6e03c4c5d5b7883ab77 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 1 Jun 2026 18:36:48 +0200 Subject: [PATCH] actually reveal the editor split on file open (T-197) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first T-197 fix flipped the wrong lever: it called activateTab(Slots.workspace, 'editor.active'), but _WorkspaceSlot renders its editor split off arrangement.editorOpen — not the active tab — so clicking a file still showed nothing. Call arrangement .openEditor() on editor.opened / active-changed(non-null), and closeEditor() on active-changed(null) so the split collapses when the last buffer closes. Test now asserts arrangement.editorOpen, the lever the UI actually reads. Co-Authored-By: Claude Opus 4.8 (1M context) --- .pql/changelog/ticket_history/2026-06.sql | 3 + .pql/changelog/tickets/2026-06.sql | 3 + lib/builtin/editor/src/extension.dart | 26 ++++++--- .../builtin/editor/editor_extension_test.dart | 58 +++++++++---------- 4 files changed, 51 insertions(+), 39 deletions(-) diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql index cfceab0b..6f93cd0e 100644 --- a/.pql/changelog/ticket_history/2026-06.sql +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -8,3 +8,6 @@ INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, 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-200', 'status', 'backlog', 'done', NULL, '2026-06-01 14:04:24', '2026-06-01 14:04:24', '2026-06-01 14:04:24', NULL, 'e77f8d64e4f9723af700dd46f962dea6', 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-201', 'status', 'backlog', 'in_progress', NULL, '2026-06-01 14:27:48', '2026-06-01 14:27:48', '2026-06-01 14:27:48', NULL, '98ad7183de73e87fe2192e3bcc12680b', 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-201', 'status', 'in_progress', 'done', NULL, '2026-06-01 14:45:39', '2026-06-01 14:45:39', '2026-06-01 14:45:39', NULL, '3073ba674f5d929f9d226e5351811433', 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-197', 'description', 'Opening a file in the editor (the reader''s edit pencil, a non-.md file-tree click, a decision''s edit) calls editor.open and the daemon opens the buffer, but the editor pane never appears in the workspace slot above the Claude pane. Root cause: EditorExtension contributes a workspace tab (editor.active, priority 80) but has no activate() that reveals/activates it. editor.opened is emitted on the DaemonBus, but the EditorController that handles it only exists once EditorView is mounted — and nothing ever activates the editor tab to mount it. Fix: add EditorExtension.activate() subscribing to the editor.opened / editor.active-changed DaemonEvents and calling panels.activateTab(Slots.workspace, ''editor.active''). EditorView.hydrate() already pulls the active buffer on mount, so reveal-then-hydrate avoids any publish/subscribe race.', 'Opening a file in the editor (the reader''s edit pencil, a non-.md file-tree click, a decision''s edit) calls editor.open and the daemon opens the buffer, but the editor pane never appears in the workspace slot above the Claude pane. Root cause: EditorExtension contributes a workspace tab (editor.active, priority 80) but has no activate() that reveals/activates it. editor.opened is emitted on the DaemonBus, but the EditorController that handles it only exists once EditorView is mounted — and nothing ever activates the editor tab to mount it. Fix: add EditorExtension.activate() subscribing to the editor.opened / editor.active-changed DaemonEvents and calling panels.activateTab(Slots.workspace, ''editor.active''). EditorView.hydrate() already pulls the active buffer on mount, so reveal-then-hydrate avoids any publish/subscribe race. + +Reopened fix (2026-06-01): the original fix called panels.activateTab(Slots.workspace, ''editor.active''), but _WorkspaceSlot renders its editor split off arrangement.editorOpen, NOT the active tab — so the editor never appeared. Real fix: EditorExtension.activate now calls arrangement.openEditor() on editor.opened / active-changed(non-null) and closeEditor() on active-changed(null). Test asserts arrangement.editorOpen.', NULL, '2026-06-01 16:36:38', '2026-06-01 16:36:38', '2026-06-01 16:36:38', NULL, '62283198922795b643960d5beab56dba', 1) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql index 21117dd8..8aa4ac65 100644 --- a/.pql/changelog/tickets/2026-06.sql +++ b/.pql/changelog/tickets/2026-06.sql @@ -6,3 +6,6 @@ INSERT INTO tickets (id, type, parent_id, title, description, status, priority, 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-199', 'task', 'T-186', 'Tickets detail: adopt retained ReaderNav (back/forward + pin) + static tab', 'Bring the tickets detail reader in line with markdown/decisions (T-196/D-81). Adopt the retained ReaderNav(''builtin.tickets'', dataKey ''id''): the nav records selections + emits ''load''; the controller loads on ''load'' (not ''selection''); the view grabs nav.current on mount and wraps in ClidePaneChrome with a ReaderActionBar (pin toggle left; back/forward + jump-to-pin right; NO edit pencil since tickets are pql records, not files). Also drop the per-click uncontribute/contribute churn in TicketsExtension (the T-188 anti-pattern) — use the static tickets.detail tab + reveal on selection. TicketDetailController loses its panels dependency (reveal moves to the extension).', 'done', 'medium', NULL, NULL, NULL, '2026-06-01 11:38:32', '2026-06-01 12:35:04', NULL, '82fe1aefd26d3f53872cc6983d1ae52d', 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-200', 'bug', NULL, 'Sidebar icon rail overflows (RenderFlex) when tab count exceeds width', 'Adding the Search sidebar tab (T-52) pushed the bottom icon rail past its width: ''A RenderFlex overflowed by 54 pixels'' from clide_icon_rail.dart — a fixed Row(center, max) with one _RailButton per tab. Fix: ClideIconRail centers when the icons fit and scrolls horizontally when they don''t (LayoutBuilder + SingleChildScrollView + ConstrainedBox(minWidth: maxWidth)). Robust to any tab count. (Related: T-201 will reduce tab count by merging the pql + find-in-files search surfaces.)', 'done', 'high', NULL, NULL, NULL, '2026-06-01 14:04:00', '2026-06-01 14:04:24', NULL, '59e5ac635342f5994afcc76c0fb3cd1e', 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-201', 'task', 'T-186', 'Merge pql search into the find-in-files Search tab (one search surface)', 'The pql sidebar panel (ranked text search + PQL DSL query + synced markdown-file listing) and the find-in-files Search tab (content grep) are duplicate search surfaces. Consolidate into ONE Search tab with a mode switch (option B): Find (content grep, existing), Vault (pql ranked search), Query (PQL DSL), Markdown (synced md-file listing, keeps focus-highlight + live refresh). Remove the builtin.pql SIDEBAR tab; keep the pql extension''s Backlinks context-panel tab + PqlController. Implementation: SearchPanelView gains a 4-mode switcher and holds both FindInFilesController + PqlController; extract the pql body/result rows into a reusable widget (they''re private in pql_panel_view.dart today); move the md-listing focus-sync + files.changed refresh into the merged panel; delete PqlPanelView. Test/coverage note: pql_panel_view + pql_controller are currently untested (not in the coverage denominator); folding them into the tested Search panel pulls them in, so the merge must cover the Vault/Query/Markdown modes too (same dynamic that bit T-199). Also reduces sidebar tab count (helps the rail, T-200). No D-79 conflict: grep vs ranked-search remain distinct backends; this is UI consolidation only.', 'done', 'medium', NULL, NULL, NULL, '2026-06-01 14:25:59', '2026-06-01 14:45:39', NULL, '4101a253d4893d6ea9b994332645dfdb', 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-197', 'bug', NULL, 'Editor pane never reveals on editor.open (edit button, file-tree open)', 'Opening a file in the editor (the reader''s edit pencil, a non-.md file-tree click, a decision''s edit) calls editor.open and the daemon opens the buffer, but the editor pane never appears in the workspace slot above the Claude pane. Root cause: EditorExtension contributes a workspace tab (editor.active, priority 80) but has no activate() that reveals/activates it. editor.opened is emitted on the DaemonBus, but the EditorController that handles it only exists once EditorView is mounted — and nothing ever activates the editor tab to mount it. Fix: add EditorExtension.activate() subscribing to the editor.opened / editor.active-changed DaemonEvents and calling panels.activateTab(Slots.workspace, ''editor.active''). EditorView.hydrate() already pulls the active buffer on mount, so reveal-then-hydrate avoids any publish/subscribe race. + +Reopened fix (2026-06-01): the original fix called panels.activateTab(Slots.workspace, ''editor.active''), but _WorkspaceSlot renders its editor split off arrangement.editorOpen, NOT the active tab — so the editor never appeared. Real fix: EditorExtension.activate now calls arrangement.openEditor() on editor.opened / active-changed(non-null) and closeEditor() on active-changed(null). Test asserts arrangement.editorOpen.', 'done', 'high', NULL, NULL, NULL, '2026-06-01 08:58:19', '2026-06-01 16:36:38', NULL, '01039a9e83c3112ca93e33c228763b50', 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/lib/builtin/editor/src/extension.dart b/lib/builtin/editor/src/extension.dart index 35b6f6f1..c8ef386e 100644 --- a/lib/builtin/editor/src/extension.dart +++ b/lib/builtin/editor/src/extension.dart @@ -19,17 +19,29 @@ class EditorExtension extends ClideExtension { StreamSubscription? _sub; - /// Reveal the editor tab when a buffer opens or becomes active. - /// `editor.open` opens the buffer daemon-side and emits the event, - /// but nothing else brings the workspace tab to front — without this - /// the editor never appears over the Claude pane (T-197). The view's - /// `hydrate()` pulls the active buffer once it mounts. + /// Reveal the editor split when a buffer opens, hide it when the last + /// one closes. `editor.open` opens the buffer daemon-side and emits the + /// event, but the workspace renders its editor split off + /// `arrangement.editorOpen` (not the active tab) — so without flipping + /// that flag the editor never appears over the Claude pane (T-197). The + /// view's `hydrate()` pulls the active buffer once it mounts. @override Future activate(ClideExtensionContext ctx) async { _sub = ctx.events.on().listen((e) { if (e.subsystem != 'editor') return; - if (e.kind != 'editor.opened' && e.kind != 'editor.active-changed') return; - ctx.panels.activateTab(Slots.workspace, 'editor.active'); + switch (e.kind) { + case 'editor.opened': + ctx.arrangement.openEditor(); + ctx.panels.activateTab(Slots.workspace, 'editor.active'); + case 'editor.active-changed': + // A null id means the last buffer closed — collapse the split. + if (e.data['id'] == null) { + ctx.arrangement.closeEditor(); + } else { + ctx.arrangement.openEditor(); + ctx.panels.activateTab(Slots.workspace, 'editor.active'); + } + } }); } diff --git a/test/builtin/editor/editor_extension_test.dart b/test/builtin/editor/editor_extension_test.dart index 459a8fc2..b94e26c3 100644 --- a/test/builtin/editor/editor_extension_test.dart +++ b/test/builtin/editor/editor_extension_test.dart @@ -1,15 +1,14 @@ -/// T-197: EditorExtension reveals its workspace tab when a buffer opens. +/// T-197: EditorExtension opens the workspace editor split when a buffer +/// opens and collapses it when the last one closes. /// -/// `editor.open` opens the buffer daemon-side and emits `editor.opened`, -/// but nothing else brings the editor tab to front over the Claude -/// pane. The extension's activate() listens for the editor lifecycle -/// events and activates the workspace tab. +/// The workspace renders its editor split off `arrangement.editorOpen` +/// (not the active tab), so the extension must flip that flag — otherwise +/// `editor.open` opens the buffer daemon-side but nothing appears over +/// the Claude pane. library; import 'package:clide/builtin/editor/src/extension.dart'; -import 'package:clide/extension/extension.dart' show TabContribution; import 'package:clide/kernel/kernel.dart'; -import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; import '../../helpers/kernel_fixture.dart'; @@ -19,15 +18,6 @@ void main() { setUp(() async { f = await KernelFixture.create(); - f.services.panels.registerSlot(const SlotDefinition(id: Slots.workspace, position: SlotPosition.center)); - // A pre-existing workspace tab so 'editor.active' is NOT the default - // active tab — the reveal must switch to it explicitly. - f.services.panels.contribute(TabContribution( - id: 'claude.primary', - slot: Slots.workspace, - title: 'Claude', - build: (_) => const SizedBox(), - )); f.services.extensions.register(EditorExtension()); await f.services.extensions.activate('builtin.editor'); }); @@ -37,39 +27,43 @@ void main() { f.services.events.emit(DaemonEvent(subsystem: 'editor', kind: kind, data: {'id': id}, ts: DateTime.now().toUtc())); } - test('contributes editor.active but leaves Claude active by default', () { + test('contributes the editor.active workspace tab', () { expect(f.services.panels.tabsFor(Slots.workspace).any((t) => t.id == 'editor.active'), isTrue); - expect(f.services.panels.activeTabIn(Slots.workspace), 'claude.primary'); }); - test('editor.opened reveals (activates) the editor tab', () async { + test('editor.opened opens the editor split', () async { + expect(f.services.arrangement.editorOpen, isFalse); emitEditor('editor.opened', id: 'b_1'); await Future.delayed(Duration.zero); - expect(f.services.panels.activeTabIn(Slots.workspace), 'editor.active'); + expect(f.services.arrangement.editorOpen, isTrue); }); - test('editor.active-changed also reveals the editor tab', () async { + test('editor.active-changed with a buffer keeps the split open', () async { emitEditor('editor.active-changed', id: 'b_2'); await Future.delayed(Duration.zero); - expect(f.services.panels.activeTabIn(Slots.workspace), 'editor.active'); + expect(f.services.arrangement.editorOpen, isTrue); }); - test('a non-editor event leaves the active tab unchanged', () async { + test('editor.active-changed with a null id collapses the split', () async { + emitEditor('editor.opened', id: 'b_1'); + await Future.delayed(Duration.zero); + expect(f.services.arrangement.editorOpen, isTrue); + + emitEditor('editor.active-changed', id: null); + await Future.delayed(Duration.zero); + expect(f.services.arrangement.editorOpen, isFalse); + }); + + test('a non-editor event does not open the split', () async { f.services.events.emit(DaemonEvent(subsystem: 'git', kind: 'changed', data: const {}, ts: DateTime.now().toUtc())); await Future.delayed(Duration.zero); - expect(f.services.panels.activeTabIn(Slots.workspace), 'claude.primary'); + expect(f.services.arrangement.editorOpen, isFalse); }); - test('an unrelated editor event kind does not reveal', () async { - emitEditor('editor.saved', id: 'b_1'); - await Future.delayed(Duration.zero); - expect(f.services.panels.activeTabIn(Slots.workspace), 'claude.primary'); - }); - - test('after deactivate, editor events no longer reveal', () async { + test('after deactivate, editor events no longer open the split', () async { await f.services.extensions.deactivate('builtin.editor'); emitEditor('editor.opened', id: 'b_9'); await Future.delayed(Duration.zero); - expect(f.services.panels.activeTabIn(Slots.workspace), isNot('editor.active')); + expect(f.services.arrangement.editorOpen, isFalse); }); }