Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #505. Finishes merge of TileView (previously SplitContainer) #2258

Merged
merged 21 commits into from Mar 2, 2023

Conversation

tznind
Copy link
Collaborator

@tznind tznind commented Dec 23, 2022

Fixes #505 - Adds new View TileView

TileView can have any number of Tiles. Each tile is seperated with a line which can be moved. Tiles can be added and removed adhoc (see InsertTile, RemoveTile).

All Tiles must follow the same Orientation. You can achieve grids/ T shapes etc of tiles through nesting. To make this easier you can use the method bool TrySplitTile(int idx, int panels, out TileView result).

This PR contains the following sub PR which could be merged seperately

Noteworthy changes

  • Makes ViewToScreen public. This is to position ContextMenu when right clicking TabView headers correctly. I have used this method via reflection in TerminalGuiDesigner and don't really see why ScreenToView is public but ViewToScreen is private.
  • Fix bug in TabView where the 'damage region' property of bounds passed to Redraw is used for drawing border instead of Bounds which is always correct. This manifested as a missing border when moving splitter lines (which reapeared after moving cursor or clicking... most strange).

ToDos

  • Titles with the new Tile system
  • Verify min tile sizes with 3+ tiles
  • Respect Visible property to hide tiles
    • Test all combinations for 3x2 tile configuration with border
    • Test all combinations for 3x2 tile configuration without border
  • Needs Tests
  • Needs More Tests
  • When switching from horizontal to vertical the SplitterDistance is usually way off (see in video) because consoles are far wider than they are tall.
  • Actually respect the Min Panel Size properties
  • Max/Min panel sizes should preserve Percent/Absolute state of SplitterDistance (currently resizing below the min results in a change to absolute splitter positioning
  • Refactor UICatalogApp to use this as part of the PR

splitter

Modeled after the WinForms / Windows SplitContainer I've used the same name and public property names (Panel1, Panel2, SplitterDistance etc).

Draft for now as there's a few kinks to work out. Theres a bit of a challenge in supporting both Percent and Absolute splitter distances. Its mostly working for both but needs some thourough testing and to consider the other Pos types users could assign to SplitterDistance.

Out of Scope

  • SplitterDistance should handle Percent and Absolute but not other types of Pos. Letting user resize with keyboard/mouse means preserving the PosType is difficult. Its most feasible if we restrict to Percent and Absolute. So we should throw ArgumentException if user sets SplitterDistance to non percent/absolute (at least for time being).

Pull Request checklist:

  • I've named my PR in the form of "Fixes #issue. Terse description."
  • My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
  • My code follows the Terminal.Gui library design guidelines
  • I ran dotnet test before commit
  • I have made corresponding changes to the API documentation (using /// style comments)
  • My changes generate no new warnings
  • I have checked my code and corrected any poor grammar or misspellings
  • I conducted basic QA to assure all features are working

@tig
Copy link
Collaborator

tig commented Dec 23, 2022

Awesome.

Can you refactor UICatalogApp to use this as part of the PR? That would add another use/test case.

@tznind
Copy link
Collaborator Author

tznind commented Dec 24, 2022

UPDATE: have got this working quite nicely now in dd246b7

Awesome.

Can you refactor UICatalogApp to use this as part of the PR? That would add another use/test case.

@tig I took a look at changing UICatalog to use a split container. The simplest solution would be just to put the 2 FrameView into either side of the parent SplitContainer but this would leave 3 separating lines down the middle (1 for each FrameView and 1 for the SplitterLine).

I looked into how to do it with just 1 line down the middle and have made some progress.

The biggest issue with this approach is that the 'Scenarios' caption has to go somewhere. It also ideally has to move with the Splitter when dragged (see video).

I have a quite hacky solution using a Label floating over the Title but its pretty janky (see code below). The Label also vanishes periodically so need to come up with something better. Might be possible to just change the Title on the FrameView to have both strings (Categories and Scenarios) seperated by HLine.

But I thought I'd show the route I am going down before I put too much work into changing UICatalog.cs . Was this what you were thinking of?

I've not committed the change but put the patch diff below so you can give me some feedback on whether you think this is an improvement?

UICatalogWithSplitter

--- a/UICatalog/UICatalog.cs
+++ b/UICatalog/UICatalog.cs
@@ -1,4 +1,4 @@
-´╗┐using NStack;
+using NStack;
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
@@ -151,9 +151,9 @@ namespace UICatalog {
                        public MenuItem miIsMouseDisabled;
                        public MenuItem miHeightAsBuffer;

-                       public FrameView LeftPane;
+                       public FrameView ContentPane;
+                       public SplitContainer SplitContainer;
                        public ListView CategoryListView;
-                       public FrameView RightPane;
                        public ListView ScenarioListView;

                        public StatusItem Capslock;
@@ -201,24 +201,29 @@ namespace UICatalog {
                                        }),
                                        new StatusItem(Key.F10, "~F10~ Hide/Show Status Bar", () => {
                                                StatusBar.Visible = !StatusBar.Visible;
-                                               LeftPane.Height = Dim.Fill(StatusBar.Visible ? 1 : 0);
-                                               RightPane.Height = Dim.Fill(StatusBar.Visible ? 1 : 0);
+                                               ContentPane.Height = Dim.Fill(StatusBar.Visible ? 1 : 0);
                                                LayoutSubviews();
                                                SetChildNeedsDisplay();
                                        }),
                                        DriverName,
                                };

-                               LeftPane = new FrameView ("Categories") {
+                               ContentPane = new FrameView ("Categories") {
                                        X = 0,
                                        Y = 1, // for menu
-                                       Width = 25,
+                                       Width = Dim.Fill (),
                                        Height = Dim.Fill (1),
                                        CanFocus = true,
                                        Shortcut = Key.CtrlMask | Key.C
                                };
-                               LeftPane.Title = $"{LeftPane.Title} ({LeftPane.ShortcutTag})";
-                               LeftPane.ShortcutAction = () => LeftPane.SetFocus ();
+                               ContentPane.Title = $"{ContentPane.Title} ({ContentPane.ShortcutTag})";
+                               ContentPane.ShortcutAction = () => ContentPane.SetFocus ();
+
+                               SplitContainer = new SplitContainer {
+                                       Width = Dim.Fill (0),
+                                       Height = Dim.Fill (0),
+                                       SplitterDistance = 25
+                               };

                                CategoryListView = new ListView (_categories) {
                                        X = 0,
@@ -229,21 +234,23 @@ namespace UICatalog {
                                        CanFocus = true,
                                };
                                CategoryListView.OpenSelectedItem += (a) => {
-                                       RightPane.SetFocus ();
+                                       ScenarioListView.SetFocus ();
                                };
                                CategoryListView.SelectedItemChanged += CategoryListView_SelectedChanged;
-                               LeftPane.Add (CategoryListView);
+                               ContentPane.Add (SplitContainer);

-                               RightPane = new FrameView ("Scenarios") {
-                                       X = 25,
+                               SplitContainer.Panel1.Add (CategoryListView);
+
+                               Label lblScenarios = new Label ("Scenarios") {
+                                       X = SplitContainer.SplitterDistance + 1,
                                        Y = 1, // for menu
-                                       Width = Dim.Fill (),
-                                       Height = Dim.Fill (1),
-                                       CanFocus = true,
-                                       Shortcut = Key.CtrlMask | Key.S
                                };
-                               RightPane.Title = $"{RightPane.Title} ({RightPane.ShortcutTag})";
-                               RightPane.ShortcutAction = () => RightPane.SetFocus ();
+                               SplitContainer.SplitterMoved += (s, e) => {
+                                       lblScenarios.X = e.SplitterDistance + 1;
+                                       Application.Top.BringSubviewToFront (lblScenarios);
+                                       lblScenarios.SetNeedsDisplay ();
+                                       Application.Top.SetNeedsDisplay ();
+                               };

                                ScenarioListView = new ListView () {
                                        X = 0,
@@ -255,13 +262,13 @@ namespace UICatalog {
                                };

                                ScenarioListView.OpenSelectedItem += ScenarioListView_OpenSelectedItem;
-                               RightPane.Add (ScenarioListView);
+                               SplitContainer.Panel2.Add (ScenarioListView);

                                KeyDown += KeyDownHandler;
                                Add (MenuBar);
-                               Add (LeftPane);
-                               Add (RightPane);
+                               Add (ContentPane);
                                Add (StatusBar);
+                               Add (lblScenarios);

                                Loaded += LoadedHandler;

@@ -287,7 +294,7 @@ namespace UICatalog {
                                        _isFirstRunning = false;
                                }
                                if (!_isFirstRunning) {
-                                       RightPane.SetFocus ();
+                                       ScenarioListView.SetFocus ();
                                }
                                Loaded -= LoadedHandler;
                        }

@tznind tznind marked this pull request as ready for review December 24, 2022 20:27
@tznind
Copy link
Collaborator Author

tznind commented Dec 25, 2022

Ok UICatalog is now using the new Splitter pane and it looks good. Its one commit so if we don't think it is an improvement I can just revert dd246b7

I've also marked the PR as no longer draft.

Merry Christmas!

@tznind tznind mentioned this pull request Dec 28, 2022
46 tasks
@tig
Copy link
Collaborator

tig commented Dec 31, 2022

Why can't we have a "T" here?

image

UICatalog/UICatalog.cs Outdated Show resolved Hide resolved
Copy link
Collaborator

@tig tig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a PR to this PR incoming. You'll like it!

UICatalog/UICatalog.cs Outdated Show resolved Hide resolved
Terminal.Gui/Views/SplitContainer.cs Outdated Show resolved Hide resolved
@tznind tznind marked this pull request as draft January 1, 2023 10:34
@tznind
Copy link
Collaborator Author

tznind commented Jan 1, 2023

Thanks for your code @tig its looking way better now! I've fixed the compilation errors in the unit tests and added support for SplitContainer detecting when theres no border set (which results in it not using T's). But there does seem to be some bad layout logic when the split container gets very small (most evident in tests since they use a 11x3 size). But you can repro it in the UICatalog Scenario too:

Wierder still the splitter line seems to jump around when the height is small and you mouse on and off it (see end of this video):

small-splits

@BDisp
Copy link
Collaborator

BDisp commented Jan 1, 2023

Here my opinion. I think SplitContainer must obtain the border configuration of his host. If has no border then a single line must be drawn without T. If it's a single border then it must drawn a single T (left-right or top-bottom). If it's a double border it must drawn double T. In the bellow image the A and B was created with a single vertical SplitContainer. In the B panel was created another horizontal SplitContainer generating the C panel. I think this strategic is more controllable to deal with odd panels.

imagem

@tznind
Copy link
Collaborator Author

tznind commented Jan 1, 2023

I think SplitContainer must obtain the border configuration of his host. If has no border then a single line must be drawn without T.

I think we can get the best of both worlds by tapping into the Border property.

@tig 's code changed SplitContainer to inherits from FrameView which looks great and works well with Titles. But if user sets the BorderStyle to None then we should adapt to that. I've started that in
e4cb4b9 but few kinks to work out.

@tznind
Copy link
Collaborator Author

tznind commented Jan 2, 2023

UPDATE: Ahh just noticed that Height is 0 on that top panel, bet that is the issue. I'm out of time thismorning but can probably crack it from that.

I've still got 3 failing tests after your changes @tig. They are all the same and manifest when having horizontal splitter and height 3. The top panel does not render (or is cleared, or something renders over the top of it. I've not been able to track down what the issue is. The positions are all correct according to the Watch window:

image

    Expected:
    11111111111
    ───────────
    22222222222
    But Was:
    ───────────
    22222222222

@tznind tznind marked this pull request as ready for review January 2, 2023 14:57
@tznind tznind marked this pull request as draft January 3, 2023 12:18
@tznind
Copy link
Collaborator Author

tznind commented Jan 3, 2023

Have added a 3 way split to the SplitContainerExample which makes this more exciting demo. But it has also revealed an issue with Tab navigation not working.

Ill dig into it when I have some time but marked as Draft for now.

I can tab fine on the UICatalog home screen but in the example I can only tab into the first split container and not the nested one.

@BDisp
Copy link
Collaborator

BDisp commented Jan 3, 2023

Have added a 3 way split to the SplitContainerExample which makes this more exciting demo. But it has also revealed an issue with Tab navigation not working.

Ill dig into it when I have some time but marked as Draft for now.

I can tab fine on the UICatalog home screen but in the example I can only tab into the first split container and not the nested one.

Thanks for your effort. That great. I think the SplitContainer must deal with tabbing. So, if the the last view on the is selected, tabbing will move to the first view of the nested slit container. If the first view of he nested split container is selected, shift tabbing will move to the last view of the first split container.

@tznind tznind marked this pull request as ready for review January 3, 2023 20:05
@tznind
Copy link
Collaborator Author

tznind commented Jan 3, 2023

Ok turns out I just needed CanFocus = true; on SplitContainer. Seems to work correctly now

Terminal.Gui/Views/SplitContainer.cs Outdated Show resolved Hide resolved
UICatalog/Scenarios/SplitContainerExample.cs Outdated Show resolved Hide resolved
UICatalog/Scenarios/SplitContainerExample.cs Outdated Show resolved Hide resolved
UICatalog/Scenarios/SplitContainerExample.cs Outdated Show resolved Hide resolved
Terminal.Gui/Views/SplitContainer.cs Outdated Show resolved Hide resolved
UICatalog/Scenarios/SplitContainerExample.cs Outdated Show resolved Hide resolved
UICatalog/Scenarios/SplitContainerExample.cs Outdated Show resolved Hide resolved
@tznind
Copy link
Collaborator Author

tznind commented Jan 6, 2023

Ok I've done some thinking (dangerous I know) and I think this is doable.

We need 2 systems for this to work:

Arbitrary Connected Lines Drawer

This would work like a 'back buffer' in in graphics. It would store all the vector lines that are added to it. Then user would call 'Finalize' or 'Merge' or something which would create a 2D array in which all the lines are drawn. Corners would be rendered with the appropriate symbol (T, Crosshair or L) based on the direction of intersecting vectors at that point.

The full final frame would then be used for whatever (e.g. drawn to screen). This would be pretty easy to test and develop independently and demoable with a kind of 'drawing' Scenario.

Integrated nesting of SplitContainers

Currently a SplitContainer has 2 panels. If we change SplitPanel to be a binary tree we can do let the user define as many sublevels as they want without having to create additional instances (good!).

In layout logic we can use recursion to pass ever reducing Bounds sizes into child branches based on the Splitter position consmed at the current level.

Estimate

Lots of work! but sounds fun!

@tznind
Copy link
Collaborator Author

tznind commented Jan 6, 2023

@tig
Copy link
Collaborator

tig commented Jan 6, 2023

Ok I've done some thinking (dangerous I know) and I think this is doable.

We need 2 systems for this to work:

Arbitrary Connected Lines Drawer

This would work like a 'back buffer' in in graphics. It would store all the vector lines that are added to it. Then user would call 'Finalize' or 'Merge' or something which would create a 2D array in which all the lines are drawn. Corners would be rendered with the appropriate symbol (T, Crosshair or L) based on the direction of intersecting vectors at that point.

The full final frame would then be used for whatever (e.g. drawn to screen). This would be pretty easy to test and develop independently and demoable with a kind of 'drawing' Scenario.

Integrated nesting of SplitContainers

Currently a SplitContainer has 2 panels. If we change SplitPanel to be a binary tree we can do let the user define as many sublevels as they want without having to create additional instances (good!).

In layout logic we can use recursion to pass ever reducing Bounds sizes into child branches based on the Splitter position consmed at the current level.

Estimate

Lots of work! but sounds fun!

I'm so in love with this thinking I can't stand it.

I wonder if we can do just enough to the current PR to ship it quickly while ensuring we can layer this other stuff in without breaking backwards compat?

@tznind
Copy link
Collaborator Author

tznind commented Jan 7, 2023

StraightLineCanvas is comming along:

lines

I wonder if we can do just enough to the current PR to ship it quickly while ensuring we can layer this other stuff in without breaking backwards compat?

It should be possible but I think i need to work this a bit longer to be sure that the solution will work. Don't want to rename our panels Left and Right in anticipation of a binary tree structure then find out its not going to work out.

@tig
Copy link
Collaborator

tig commented Jan 8, 2023

You guys are going to be disgusted by this, but for giggles, I asked ChatGPT to write a paneled window manager:

I want to build a paneling window manager for Terminal.Gui in c#. Please write the code for a c# class that implements a paneled window manager that supports an arbitrary number of panels. The lines separating the panels should be draggable to allow the panels to be resized.

using System;
using System.Collections.Generic;
using Terminal.Gui;

namespace PanelDemo
{
    public class PanelManager
    {
        private List<Panel> panels;
        private List<Line> lines;
        private Window window;
        private int panelSize;
        private int lineSize;
        private int numPanels;

        public PanelManager(Window window, int numPanels, int panelSize, int lineSize)
        {
            this.window = window;
            this.numPanels = numPanels;
            this.panelSize = panelSize;
            this.lineSize = lineSize;

            panels = new List<Panel>();
            lines = new List<Line>();
            for (int i = 0; i < numPanels; i++)
            {
                Panel panel = new Panel(new Rect(0, 0, panelSize, window.Frame.Height));
                panels.Add(panel);
                window.Add(panel);

                if (i < numPanels - 1)
                {
                    Line line = new Line(new Rect(panelSize, 0, lineSize, window.Frame.Height));
                    lines.Add(line);
                    window.Add(line);
                }
            }
        }

        public void ResizePanel(int panelIndex, int newSize)
        {
            Panel panel = panels[panelIndex];
            int oldSize = panel.Frame.Width;
            panel.Frame = new Rect(panel.Frame.X, panel.Frame.Y, newSize, panel.Frame.Height);
            if (panelIndex < numPanels - 1)
            {
                Line line = lines[panelIndex];
                line.Frame = new Rect(line.Frame.X + newSize - oldSize, line.Frame.Y, line.Frame.Width, line.Frame.Height);
                panels[panelIndex + 1].Frame = new Rect(panels[panelIndex + 1].Frame.X + newSize - oldSize, panels[panelIndex + 1].Frame.Y, panels[panelIndex + 1].Frame.Width - newSize + oldSize, panels[panelIndex + 1].Frame.Height);
            }
            window.Refresh();
        }
    }
}

To use this class, you will need to create an instance of the PanelManager class and pass in the Window object that you want to add the panels to, the number of panels you want, the initial size of each panel, and the size of the lines separating the panels. Then you can use the ResizePanel method to resize a panel by passing in the index of the panel and the new size.

Window window = new Window("Panel Demo");
PanelManager panelManager = new PanelManager(window, 3, 50, 2);

// Resize the second panel to be 60 units wide
panelManager.ResizePanel(1, 60);

@tznind
Copy link
Collaborator Author

tznind commented Jan 9, 2023

Starting to feel like a deep rabbit hole!

Not sure this recursive approach is the best. I'll keep digging and see but does feel a bit like I am working against API rather than with it.

Starting to feel like a deep rabbit hole!

Not sure this recursive approach is the best.  I'll keep digging and see but does feel a bit like I am working against API rather than with it.

```csharp
class SplitPanel : View

    View Left      
    View Right
    Pos SplitterDistance   
    
    public virtual void LayoutSubviews ()
    {
        var currentSize = Bounds;
        
        if(HasBorder())
        {
            currentSize = new Rect(1,1,Bounds.Width,Bounds.Height);
        }
        
        LayoutSubviews(this, currentSize);
        
    }
    
    private LayoutSubviews(SplitPanel panel, Rect bounds)
    {
    
        var leftSpace = panel.SplitterDistance.Anchor(currentSize)
        var rightSpace = bounds.Width - leftSpace;
        
        if(panel.Left is SplitPanel ls)
        {
            LayoutSubviews(ls,new Rect(bounds.X,bounds.Y, width:leftSpace, height: bounds.Height));
        }
        else
        {
            panel.Left.X = bounds.X;
            panel.Left.Y = bounds.Y;
            panel.Width = leftSpace;
            panel.Height = bounds.Height;
        }
        
        
        if(panel.Right is SplitPanel rs)
        {
            LayoutSubviews(rs,new Rect(bounds.X+leftSpace,bounds.Y, width:rightSpace, height: bounds.Height));
        }
        else
        {
            panel.Right.X = bounds.X + leftSpace;
            panel.Right.Y = bounds.Y;
            panel.Right.Width = rightSpace;
            panel.Right.Height = bounds.Height;
        }
        
        // TODO: Also position line view
    }
    
    // TODO: Override get child views to return from tree
    
    // TODO: How to suppress child layouts?

    // TODO: How to add new child views

	public override void Redraw (Rect bounds)
	{
		base.Redraw (bounds);

		if (!HasBorder ())
			return;

		var canvas = new LineCanvas ();

		foreach (var l in Subviews.OfType<SplitContainerLineView> ()) {
			// TODO: Add to canvas
		}
		canvas.Draw(this,bounds)
	}
    
}
    

@BDisp
Copy link
Collaborator

BDisp commented Jan 9, 2023

Not sure this recursive approach is the best. I'll keep digging and see but does feel a bit like I am working against API rather than with it.

No, what's happening is you are more closer to find the best way to do this.

@tznind
Copy link
Collaborator Author

tznind commented Jan 9, 2023

No, what's happening is you are more closer to find the best way to do this.

Haha thanks for your confidence :)

@BDisp
Copy link
Collaborator

BDisp commented Feb 25, 2023

Ok I think the problem is this code in OnEnter

Ah right. You have to ensure if there have or not another tile view. If exist and is the last view focused and Tab pressed then must navigate to the first view of the tile view.

@tznind
Copy link
Collaborator Author

tznind commented Feb 25, 2023

Ok should be working now. I have deleted the entire OnEnter override and made the TileView itself CanFocus false (the default).

@BDisp
Copy link
Collaborator

BDisp commented Feb 25, 2023

Yet better, let the View base work :-)

@BDisp
Copy link
Collaborator

BDisp commented Feb 25, 2023

It's working but has a new wrong behavior. The navigation is backwards.

Edit:
Never mind. It's the right navigation. So when a new tile is inserted the last being focused. So first "222", "111" and last "000". I suppose but I might be wrong, due the tests fails.

@tznind
Copy link
Collaborator Author

tznind commented Feb 25, 2023

It's working but has a new wrong behavior. The navigation is backwards.

Edit: Never mind. It's the right navigation. So when a new tile is inserted the last being focused. So first "222", "111" and last "000".

Yeah I think it mostly depends what order you split and add to TileView. For example in the Notepad example if you SplitLeft then take left pane and SplitDown then tab navigation works correctly but feels off since it enters the bottom from the right then goes up:

split-left

But if you SplitLeft then take the left pane and SplitUp then tab navigation works and feels right.

split-up

@tznind
Copy link
Collaborator Author

tznind commented Feb 25, 2023

Hmn test failures are System.InvalidOperationException : Cannot set CanFocus to true if the SuperView CanFocus is false!

They are for when you make the splitter lines focusable during runtime.

I guess maybe I need to remove the line views and then add them again as focusable or something

Strangely it doesn't affect the Scenarios only the tests.

@BDisp
Copy link
Collaborator

BDisp commented Feb 25, 2023

Well if you want to order always by left top corner to down and right bottom corner, then you have to use BringSubviewForward, SendSubviewBackwards, BringSubviewToFront or SendSubviewBackwards, accordingly.

@tznind
Copy link
Collaborator Author

tznind commented Feb 25, 2023

Well if you want to order always by left top corner to down and right bottom corner, then you have to use BringSubviewForward, SendSubviewBackwards, BringSubviewToFront or SendSubviewBackwards, accordingly.

Yeah don't think I want to do that! feels a bit hacky/risky and I think the current behaviour is ok.

Might look into adjusting the tabIndexes during InsertTile or TrySplitTile though.

@BDisp
Copy link
Collaborator

BDisp commented Feb 25, 2023

Hmn test failures are System.InvalidOperationException : Cannot set CanFocus to true if the SuperView CanFocus is false!

They are for when you make the splitter lines focusable during runtime.

At runtime you have to set the superview can focus to true before.

I guess maybe I need to remove the line views and then add them again as focusable or something

If you are adding at load a line view that has CanFocus as true it will also sets the superview CanFocus to true.

Strangely it doesn't affect the Scenarios only the tests.
Because the scenarios ha already initialized by the Application.Begin.

Let me say some clarification on this. Toplevel.CanFocus is false. When a subview with CanFocus as true and if toplevel is enabled, then the Toplevel.CanFocus is sets to true, but this is done when the subview is adding to the superview. At runtime if you sets the superview CanFocus to false it also will sets all the subviews CanFocus to false.

@BDisp
Copy link
Collaborator

BDisp commented Feb 25, 2023

Might look into adjusting the tabIndexes during InsertTile or TrySplitTile though.

I forgot to remember that solution before, sorry. It's the better and ensured way.

@tznind
Copy link
Collaborator Author

tznind commented Feb 25, 2023

Cool, so the fix I applied for the CanFocus buisness is 7929084 which seems to avoid the throw even if it seems a bit hacky. And since the LineView has no children it shouldn't have any unforseen consequences I think.

@BDisp
Copy link
Collaborator

BDisp commented Feb 25, 2023

Cool, so the fix I applied for the CanFocus buisness is 7929084 which seems to avoid the throw even if it seems a bit hacky. And since the LineView has no children it shouldn't have any unforseen consequences I think.

Well I don't agree. Even it have no children, but it could can get the focus to manage his position. Imagine you want to move the line through the keyboard. For that it needed to get focus first for being receiving keys events. But if the superview CanFocus is false, then you can't also focused the LineView. Make sense.

@tznind
Copy link
Collaborator Author

tznind commented Feb 25, 2023

Cool, so the fix I applied for the CanFocus buisness is 7929084 which seems to avoid the throw even if it seems a bit hacky. And since the LineView has no children it shouldn't have any unforseen consequences I think.

Well I don't agree. Even it have no children, but it could can get the focus to manage his position. Imagine you want to move the line through the keyboard. For that it needed to get focus first for being receiving keys events. But if the superview CanFocus is false, then you can't also focused the LineView. Make sense.

Ah there is a keypress (Ctrl+F10 by default) that makes the lines focusable so user can tab to them and move them with cursor keys :). In fact it is that toggling that was originally throwing the InvalidArgumentException.

I've also seen that even if CanFocus is false you can still focus children. Even the default View class has CanFocus of false but you can select stuff that you have added to it no problem.

@BDisp
Copy link
Collaborator

BDisp commented Feb 25, 2023

I've also seen that even if CanFocus is false you can still focus children. Even the default View class has CanFocus of false but you can select stuff that you have added to it no problem.

The trick is bellow if it isn't overridden (in this case maybe is better call the base.Add). It's handled on View.Add method. At runtime if the superview CanFocus is false you can't add a subview with CanFocus as true without first sets the superview CanFocus to true. This is the expected the desired behavior. If a superview CanFocus was sets to false at runtime, we need to inspect his value before adding a subview with CanFocus as true to it.

if (view.CanFocus) {
addingView = true;
if (SuperView?.CanFocus == false) {
SuperView.addingView = true;
SuperView.CanFocus = true;
SuperView.addingView = false;
}
CanFocus = true;
view.tabIndex = tabIndexes.IndexOf (view);
addingView = false;
}

@tig
Copy link
Collaborator

tig commented Feb 28, 2023

@tznind - same question about v1 v v2 for this.

@tznind
Copy link
Collaborator Author

tznind commented Feb 28, 2023 via email

@tznind tznind changed the base branch from develop to v2 March 1, 2023 08:20
@tznind
Copy link
Collaborator Author

tznind commented Mar 1, 2023

I hadn't realized this was already in v2. I've fetched and merged v2 into this and now the only changes are the toggle button and tab order fixes. It also targets the v2 branch now.

@tig
Copy link
Collaborator

tig commented Mar 1, 2023

I hadn't realized this was already in v2. I've fetched and merged v2 into this and now the only changes are the toggle button and tab order fixes. It also targets the v2 branch now.

Yeah, I needed it for Config Manager so merged it a bit ago.

Why does this PR have all the config manager stuff in it as well?

@tznind
Copy link
Collaborator Author

tznind commented Mar 1, 2023

Yeah, I needed it for Config Manager so merged it a bit ago.

Why does this PR have all the config manager stuff in it as well?

This PR is now only +102 / −62 after merging v2 branch into it (splitcontainer) and re targeting the pr to merge back into v2. I thought that was the plan? but didn't realize it had in effect already been merged into v2.

Looks like there are only very recent (unmerged) changes now in this branch that are not still on v2.

The configuration manager stuff is already in v2.

@tig tig changed the title Fixes #505 Adds TileView (previously SplitContainer) Fixes #505. Finishes merge of TileView (previously SplitContainer) Mar 2, 2023
Copy link
Collaborator

@tig tig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff. Thanks for doing the dirty work on this one.

Be aware, however, that I have a crazy idea that TileView may not be needed as a separate view my v2 View redesign works. We shall see... ;-)

@tig tig merged commit 0bd32c2 into gui-cs:v2 Mar 2, 2023
@tznind tznind deleted the splitcontainer branch March 3, 2023 08:52
@tig tig mentioned this pull request Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New Control: Split Panel
3 participants