From 33b8da098ef1a76df0f7d0e53318ed0c8b8050ae Mon Sep 17 00:00:00 2001 From: Dmitry Ratner <6830384+dmitrat@users.noreply.github.com> Date: Wed, 19 Aug 2026 13:14:12 +0300 Subject: [PATCH] A double click opens the thing the node is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Asked in chat: would a double click on the database not open its properties, the way «Database...» does in the menu? It would, and it is the same rule rather than a third exception. A table IS its rows, so it opens the editor; a view IS the rows it selects; the connection IS the database, so it opens the tab that describes it - the same tab the menu item opens, and only ever one of them. Everything else keeps the tree's own behaviour, which for a folder is to open and close. The decision moved out of the code-behind and into the ViewModel, and that is the larger half of this change. Which node opens what is a RULE; only the gesture belongs to the view. It was written in the code-behind, where the double click had already been broken once and repaired onto a route that does not exist, with a thousand tests unable to say a word about any of it. It is now CanOpenWhatItIs and OpenWhatItIsAsync, and the fixture drives them over every kind of node: three open something, nine open nothing, and a tree whose connection has gone opens nothing at all. Red with the Database arm removed, and red again with the Table arm removed. Studio: 1027 green. Co-Authored-By: Claude Opus 5 --- .../ADoubleClickOpensWhatTheNodeIsTests.cs | 221 ++++++++++++++++++ .../ViewModels/DatabaseExplorerViewModel.cs | 47 ++++ .../Views/DatabaseExplorer.axaml.cs | 26 +-- 3 files changed, 278 insertions(+), 16 deletions(-) create mode 100644 Tools/OutWit.Database.Studio.Tests/ViewModels/ADoubleClickOpensWhatTheNodeIsTests.cs diff --git a/Tools/OutWit.Database.Studio.Tests/ViewModels/ADoubleClickOpensWhatTheNodeIsTests.cs b/Tools/OutWit.Database.Studio.Tests/ViewModels/ADoubleClickOpensWhatTheNodeIsTests.cs new file mode 100644 index 0000000..6e83071 --- /dev/null +++ b/Tools/OutWit.Database.Studio.Tests/ViewModels/ADoubleClickOpensWhatTheNodeIsTests.cs @@ -0,0 +1,221 @@ +using OutWit.Database.Studio.Models; +using OutWit.Database.Studio.Tests.Helpers; +using OutWit.Database.Studio.ViewModels.Tabs; + +namespace OutWit.Database.Studio.Tests.ViewModels; + +/// +/// A double click opens the thing the node IS. +/// +/// +/// +/// One rule rather than three exceptions. A table IS its rows, so it opens the editor (WS-19); a view +/// IS the rows it selects; and the connection IS the database, so it opens the tab that describes it +/// - the same one Database… opens in the menu. Asked for in chat on 2026-08-19: would it +/// not be logical for a double click on the database to open its properties, as the menu item does? +/// +/// +/// The decision is in the ViewModel and this fixture is why. It used to be a switch in the +/// code-behind, where the double click had already been broken once and repaired onto a route that +/// does not exist, with 1014 tests unable to say a word about any of it. A gesture belongs to the +/// view; which node opens what is a rule, and a rule written where no test can read it is a rule that +/// holds only until someone edits it. +/// +/// +[TestFixture] +public class ADoubleClickOpensWhatTheNodeIsTests +{ + #region Fields + + private StudioFixture m_studio = null!; + + #endregion + + #region Setup + + [SetUp] + public async Task SetUp() + { + m_studio = await StudioFixture.CreateAsync(); + + await m_studio.Explorer.RefreshAsync(); + } + + [TearDown] + public async Task TearDown() + { + await m_studio.DisposeAsync(); + } + + #endregion + + #region What each node opens + + [Test] + public async Task ATableOpensItsRowsTest() + { + Select(DatabaseNodeType.Table, "Customers"); + + var opened = await OpenAsync(); + + Assert.Multiple(() => + { + Assert.That(opened, Is.InstanceOf(), + "a table IS its rows, and they are opened for editing rather than for reading"); + + Assert.That(((TableEditTabViewModel)opened!).TableName, Is.EqualTo("Customers")); + }); + } + + [Test] + public async Task AViewOpensTheRowsItSelectsTest() + { + Select(DatabaseNodeType.View, "ActiveOrders"); + + var opened = await OpenAsync(); + + Assert.Multiple(() => + { + Assert.That(opened, Is.InstanceOf(), + "a view has no rows of its own to edit, so its query is opened instead"); + + Assert.That(opened!.Title, Does.Contain("ActiveOrders")); + }); + } + + /// + /// The one this fixture was written for. + /// + [Test] + public async Task TheConnectionOpensTheTabThatDescribesItTest() + { + Select(DatabaseNodeType.Database); + + var opened = await OpenAsync(); + + Assert.That(opened, Is.InstanceOf(), + "the connection IS the database, and the double click opens what «Database…» opens"); + } + + /// + /// And the same tab as the menu item, not a second one beside it. + /// + [Test] + public async Task TheDoubleClickAndTheMenuItemOpenTheSameTabTest() + { + Select(DatabaseNodeType.Database); + + var fromTheDoubleClick = await OpenAsync(); + + await m_studio.Explorer.OpenWhatItIsAsync(); + + Assert.Multiple(() => + { + Assert.That(m_studio.Workspace.Tabs.OfType().Count(), Is.EqualTo(1), + "opening it twice is opening it once"); + + Assert.That(m_studio.Workspace.Tabs, Does.Contain(fromTheDoubleClick)); + }); + } + + #endregion + + #region What no node opens + + [Test] + public async Task NothingElseOpensAnythingTest() + { + var nothing = new[] + { + DatabaseNodeType.TablesFolder, DatabaseNodeType.ViewsFolder, + DatabaseNodeType.IndexesFolder, DatabaseNodeType.TriggersFolder, + DatabaseNodeType.SequencesFolder, DatabaseNodeType.RoutinesFolder, + DatabaseNodeType.Index, DatabaseNodeType.Trigger, DatabaseNodeType.Column + }; + + var offenders = new List(); + + foreach (var type in nothing) + { + Select(type); + + var before = m_studio.Workspace.Tabs.Count; + + if (m_studio.Explorer.CanOpenWhatItIs) + offenders.Add($"{type}: says it has something to open"); + + await m_studio.Explorer.OpenWhatItIsAsync(); + + if (m_studio.Workspace.Tabs.Count != before) + offenders.Add($"{type}: opened a tab anyway"); + } + + Assert.Multiple(() => + { + Assert.That(offenders, Is.Empty, string.Join(Environment.NewLine, offenders)); + + // CONTROL: the other direction, in the same case. A property that answered false to + // everything would satisfy every assertion above. + Select(DatabaseNodeType.Table, "Customers"); + Assert.That(m_studio.Explorer.CanOpenWhatItIs, Is.True, + "CONTROL: a table does have something to open"); + }); + } + + /// + /// A node that cannot be reached because its connection has gone is not offered either - the + /// same distinction the menu makes between «does not apply» and «cannot right now». + /// + [Test] + public async Task ADisconnectedTreeOpensNothingTest() + { + Select(DatabaseNodeType.Table, "Customers"); + + Assume.That(m_studio.Explorer.CanOpenWhatItIs, Is.True); + + await m_studio.Connections.CloseAllAsync(); + + Assert.That(m_studio.Explorer.CanOpenWhatItIs, Is.False, + "there is nothing left to open it in"); + } + + #endregion + + #region Tools + + /// Opens what the selected node is, and answers with the tab that appeared. + private async Task OpenAsync() + { + var before = m_studio.Workspace.Tabs.ToList(); + + Assert.That(m_studio.Explorer.CanOpenWhatItIs, Is.True, + "this node says it has nothing to open"); + + await m_studio.Explorer.OpenWhatItIsAsync(); + + return m_studio.Workspace.Tabs.Except(before).FirstOrDefault(); + } + + private void Select(DatabaseNodeType type, string? named = null) + { + var node = Walk(m_studio.Explorer.Nodes).FirstOrDefault(candidate => + candidate.NodeType == type && (named == null || candidate.Name == named)); + + Assert.That(node, Is.Not.Null, $"the tree has a {type} node{(named == null ? "" : " called " + named)}"); + + m_studio.Explorer.SelectedNode = node; + } + + private static IEnumerable Walk(IEnumerable nodes) + { + foreach (var node in nodes) + { + yield return node; + + foreach (var child in Walk(node.Children)) + yield return child; + } + } + + #endregion +} diff --git a/Tools/OutWit.Database.Studio/ViewModels/DatabaseExplorerViewModel.cs b/Tools/OutWit.Database.Studio/ViewModels/DatabaseExplorerViewModel.cs index 78177a8..a4d6e12 100644 --- a/Tools/OutWit.Database.Studio/ViewModels/DatabaseExplorerViewModel.cs +++ b/Tools/OutWit.Database.Studio/ViewModels/DatabaseExplorerViewModel.cs @@ -44,6 +44,7 @@ private void InitCommands() SelectTop100Command = new RelayCommand(SelectTop100); SelectTop1000Command = new RelayCommand(SelectTop1000); OpenDatabaseTabCommand = new RelayCommandAsync(OpenDatabaseTabAsync); + OpenWhatItIsCommand = new RelayCommandAsync(OpenWhatItIsAsync); EditDataCommand = new RelayCommandAsync(EditDataAsync); ViewStructureCommand = new RelayCommandAsync(ViewStructureAsync); ViewDefinitionCommand = new RelayCommandAsync(ViewDefinitionAsync); @@ -254,6 +255,49 @@ private async Task EditDataAsync() Logger.LogInformation("Edit data for table {TableName} in {Connection}", tableName, session.DisplayName); } + /// + /// What a double click opens: the thing the node IS. + /// + /// + /// + /// One rule rather than three exceptions. A table IS its rows, so it opens the editor (WS-19); a + /// view IS the rows it selects, so it opens them; and the connection IS the database, so it opens + /// the tab that describes it - the same one Database… opens in the menu. Everything else + /// answers with false and keeps the tree's own behaviour, which for + /// a folder is to open and close. + /// + /// + /// It lives here rather than in the code-behind because the code-behind is the half no test can + /// reach, and this is a decision - which node opens what - rather than a gesture. + /// + /// + public bool CanOpenWhatItIs => SelectedNode?.NodeType switch + { + DatabaseNodeType.Table => CanEditData, + DatabaseNodeType.View => CanBrowseData, + DatabaseNodeType.Database => CanOpenDatabaseTab, + _ => false + }; + + /// + public async Task OpenWhatItIsAsync() + { + switch (SelectedNode?.NodeType) + { + case DatabaseNodeType.Table when CanEditData: + await EditDataAsync(); + break; + + case DatabaseNodeType.View when CanBrowseData: + SelectTopRows(1000); + break; + + case DatabaseNodeType.Database when CanOpenDatabaseTab: + await OpenDatabaseTabAsync(); + break; + } + } + /// /// Opens the storage tab of the selected connection (WS-54). /// @@ -1451,6 +1495,9 @@ private void OnPropertyChangedInternal(object? sender, PropertyChangedEventArgs public ICommand OpenDatabaseTabCommand { get; private set; } = null!; + /// The double click, which opens the thing the node is. + public ICommand OpenWhatItIsCommand { get; private set; } = null!; + [Notify] public ICommand EditDataCommand { get; private set; } = null!; diff --git a/Tools/OutWit.Database.Studio/Views/DatabaseExplorer.axaml.cs b/Tools/OutWit.Database.Studio/Views/DatabaseExplorer.axaml.cs index f93d3fd..91c6266 100644 --- a/Tools/OutWit.Database.Studio/Views/DatabaseExplorer.axaml.cs +++ b/Tools/OutWit.Database.Studio/Views/DatabaseExplorer.axaml.cs @@ -103,7 +103,8 @@ public DatabaseExplorer() #region Event Handlers /// - /// A double click opens the DATA of a table, not its structure (WS-19). + /// A double click opens the thing the node IS - a table's data rather than its structure + /// (WS-19), a view's rows, and a connection's own tab. /// /// It used to open the structure, with the data hidden in the context menu - while looking at the /// data is what people come to a database tool to do, by an order of magnitude. The structure is @@ -116,10 +117,10 @@ public DatabaseExplorer() /// application's behaviour. /// /// - private void OpenTheDataUnderThePointer(PointerPressedEventArgs e) + private void OpenWhatIsUnderThePointer(PointerPressedEventArgs e) { // The chevron is a control of its own, and two clicks on it are two toggles rather than a - // request for the data. + // request for what is in the row. if (PressedOnTheChevron(e)) return; @@ -133,19 +134,12 @@ private void OpenTheDataUnderThePointer(PointerPressedEventArgs e) // order the tree does its own work in. explorer.SelectedNode = node; - switch (node.NodeType) - { - case DatabaseNodeType.Table when explorer.CanEditData: - explorer.EditDataCommand.Execute(null); - break; - - case DatabaseNodeType.View when explorer.CanBrowseData: - explorer.SelectTop1000Command.Execute(null); - break; + // WHAT is opened is the ViewModel's decision, not this handler's: a gesture belongs here, a + // rule does not, and a rule written here is a rule no test can read. + if (!explorer.CanOpenWhatItIs) + return; - default: - return; - } + explorer.OpenWhatItIsCommand.Execute(null); // The tap that follows this press will toggle the row. Remember what to put back. m_rowToLeaveAsItWas = item; @@ -213,7 +207,7 @@ private void OnPointerPressed(object? sender, PointerPressedEventArgs e) } if (properties.IsLeftButtonPressed && e.ClickCount == 2) - OpenTheDataUnderThePointer(e); + OpenWhatIsUnderThePointer(e); } /// The row the pointer is over, if it is over one.