From 6f3c73a5214403918184de74663a9edd6de748ae Mon Sep 17 00:00:00 2001 From: burhankhanzada Date: Sun, 4 Oct 2026 20:22:11 +0200 Subject: [PATCH] =?UTF-8?q?Make=20File=20=E2=96=B8=20Close=20Editor=20(?= =?UTF-8?q?=E2=8C=98W)=20close=20the=20active=20tab?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Close Editor and Revert File read activeTabProvider, which switching tabs never updates, so it was usually empty and both did nothing. They now ask the layout which tab is on screen (AppLayoutController.activeCenterLeaf). Close Editor closes it as a tab's own × does — a file's editor state goes with it, and a circuit's canvas with its file — and closes non-file tabs too (a canvas, the welcome screen, settings), as ⌘W does in VS Code. Revert File acts on the active tab's file, and on nothing when that tab is not a file. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../controllers/app_layout_controller.dart | 9 ++ lib/shell/menus/file/actions/closed.dart | 8 +- lib/shell/menus/shared_menu_actions.dart | 29 ++++-- test/shell/close_editor_test.dart | 92 +++++++++++++++++++ 4 files changed, 127 insertions(+), 11 deletions(-) create mode 100644 test/shell/close_editor_test.dart diff --git a/lib/layout/controllers/app_layout_controller.dart b/lib/layout/controllers/app_layout_controller.dart index 669a357..4ce291f 100644 --- a/lib/layout/controllers/app_layout_controller.dart +++ b/lib/layout/controllers/app_layout_controller.dart @@ -158,6 +158,15 @@ class AppLayoutController( _platController.focus(AppTabs.releaseNotes); } + /// The tab on screen in the editor area, or null with none open. What + /// "the active editor" means to File ▸ Close Editor and Revert File. + LeafSnapshot? activeCenterLeaf() { + final group = _platController.snapshot('center_pane'); + if (group is! TabGroupSnapshot) return null; + final active = group.activeTab; + return active?.focusedLeaf ?? active?.firstLeaf; + } + /// Whether the settings tab is open *and* the one on screen — the activity /// bar's settings button lights up on exactly that. bool isSettingsTabSelected() { diff --git a/lib/shell/menus/file/actions/closed.dart b/lib/shell/menus/file/actions/closed.dart index be032b9..c6cc251 100644 --- a/lib/shell/menus/file/actions/closed.dart +++ b/lib/shell/menus/file/actions/closed.dart @@ -5,8 +5,8 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:multiview_desktop/multiview_desktop.dart'; import 'package:pinbench_ui/strings.dart'; -import '../../../../features/editor/providers/editor_provider.dart'; import '../../../../features/workspace/providers/workspace_files_provider.dart'; +import '../../../../layout/controllers/app_layout_controller.dart'; import '../../shared_menu_actions.dart'; PlatformMenuItemGroup closeActions({ @@ -20,9 +20,9 @@ PlatformMenuItemGroup closeActions({ onSelected: !hasEditorTab ? null : () async { - final activeTab = ref.read(activeTabProvider); - if (activeTab.isEmpty || activeTab == 'Welcome') return; - await ref.read(workspaceFilesProvider.notifier).revertFile(activeTab); + final path = activeEditorFile(ref.read(appLayoutControllerProvider)); + if (path == null) return; + await ref.read(workspaceFilesProvider.notifier).revertFile(path); }, ), PlatformMenuItem( diff --git a/lib/shell/menus/shared_menu_actions.dart b/lib/shell/menus/shared_menu_actions.dart index 6c649bc..768c429 100644 --- a/lib/shell/menus/shared_menu_actions.dart +++ b/lib/shell/menus/shared_menu_actions.dart @@ -5,9 +5,9 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:pinbench_ui/widgets/text_input_dialog.dart'; import 'package:pinbench_ui/strings.dart'; -import '../../features/editor/providers/editor_provider.dart'; import '../../features/workspace/providers/workspace_files_provider.dart'; import '../../features/workspace/providers/editor_state_provider.dart'; +import '../../core/chrome/chrome_commands.dart'; import '../../core/updates/update_providers.dart'; import '../../layout/controllers/app_layout_controller.dart'; import '../../layout/updates/update_dialog.dart'; @@ -61,16 +61,31 @@ Future loadPrecompiledHexFile(WorkspaceFiles files) async { /// Closes the active editor tab (and its canvas tab, if it's a `.cdl`). /// Backs "Close Editor" on both menus. No-op when nothing (or the welcome /// tab) is active. +/// +/// Asks the layout which tab that is. It used to read `activeTabProvider`, +/// which switching tabs never updates — so it was usually empty, and ⌘W did +/// nothing. void closeActiveEditorTab(WidgetRef ref) { - final activeTab = ref.read(activeTabProvider); - if (activeTab.isEmpty || activeTab == 'Welcome') return; - ref.read(appLayoutControllerProvider).closeTab(activeTab); - ref.read(editorStateControllerProvider).closeFile(activeTab); - if (activeTab.endsWith('.cdl')) { - ref.read(appLayoutControllerProvider).closeTab('canvas_$activeTab'); + final layout = ref.read(appLayoutControllerProvider); + final leaf = layout.activeCenterLeaf(); + if (leaf == null) return; + // A file tab carries its path; its editor state goes with it, and so does a + // `.cdl` file's canvas. Anything else — a canvas, the welcome screen, + // settings — is just a tab to close, as ⌘W closes any editor in VS Code. + if (activeEditorFile(layout) case final path?) { + ref.read(editorStateControllerProvider).closeFile(path); + if (path.endsWith('.cdl')) layout.closeTab(AppTabs.canvas(path)); } + layout.closeTab(leaf.id); } +/// The file open in the active editor tab, or null when that tab is not a +/// file (a canvas, the welcome screen, settings) or there is none. +String? activeEditorFile(AppLayoutController layout) => switch (layout.activeCenterLeaf()?.data) { + final String path when path != 'canvas_view' => path, + _ => null, +}; + /// Opens the update dialog, which starts a check as it appears. Backs "Check /// for Updates…" on both menus. /// diff --git a/test/shell/close_editor_test.dart b/test/shell/close_editor_test.dart new file mode 100644 index 0000000..0ac0f6b --- /dev/null +++ b/test/shell/close_editor_test.dart @@ -0,0 +1,92 @@ +import 'package:flutter/widgets.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:pinbench/core/chrome/chrome_commands.dart'; +import 'package:pinbench/features/workspace/providers/editor_state_provider.dart'; +import 'package:pinbench/layout/controllers/app_layout_controller.dart'; +import 'package:pinbench/shell/menus/shared_menu_actions.dart'; + +/// File ▸ Close Editor (⌘W) closes the tab on screen. It used to read +/// `activeTabProvider`, which switching tabs never updates, so it was usually +/// empty and the shortcut did nothing. +void main() { + late WidgetRef ref; + late ProviderContainer container; + + Future pump(WidgetTester tester) async { + container = ProviderContainer(); + addTearDown(container.dispose); + await tester.pumpWidget( + UncontrolledProviderScope( + container: container, + child: Consumer( + builder: (context, r, _) { + ref = r; + return const SizedBox(); + }, + ), + ), + ); + return container.read(appLayoutControllerProvider); + } + + testWidgets('closes the active file, and its editor state with it', (tester) async { + final layout = await pump(tester); + container.read(editorStateControllerProvider).openFile('/ws/a.ino', 'void setup() {}'); + layout.openEditorTab('/ws/a.ino'); + + closeActiveEditorTab(ref); + + expect(layout.platController.snapshot('/ws/a.ino'), isNull); + expect(container.read(editorStateControllerProvider).openFileControllers, isEmpty); + }); + + testWidgets('closes the tab that is on screen, not another', (tester) async { + final layout = await pump(tester); + final editors = container.read(editorStateControllerProvider) + ..openFile('/ws/a.ino', '') + ..openFile('/ws/b.ino', ''); + layout + ..openEditorTab('/ws/a.ino') + ..openEditorTab('/ws/b.ino') + ..focusTab('/ws/a.ino'); + + closeActiveEditorTab(ref); + + expect(layout.platController.snapshot('/ws/a.ino'), isNull); + expect(layout.platController.snapshot('/ws/b.ino'), isNotNull); + expect(editors.openFileControllers.keys, ['/ws/b.ino']); + }); + + testWidgets("a circuit's canvas goes with its file", (tester) async { + final layout = await pump(tester); + container.read(editorStateControllerProvider).openFile('/ws/c.cdl', ''); + layout + ..openCanvasTab('/ws/c.cdl') + ..openEditorTab('/ws/c.cdl'); + + closeActiveEditorTab(ref); + + expect(layout.platController.snapshot('/ws/c.cdl'), isNull); + expect(layout.platController.snapshot(AppTabs.canvas('/ws/c.cdl')), isNull); + }); + + testWidgets('closes a tab that is not a file, as VS Code does', (tester) async { + final layout = await pump(tester); + layout.openSettingsTab(); + + closeActiveEditorTab(ref); + + expect(layout.platController.snapshot(AppTabs.settings), isNull); + expect(layout.platController.snapshot(AppTabs.welcome), isNotNull); + }); + + testWidgets('does nothing with no tab open', (tester) async { + final layout = await pump(tester); + layout.closeTab(AppTabs.welcome); + expect(layout.activeCenterLeaf(), isNull); + + closeActiveEditorTab(ref); + }); +}