Terminal.Gui icon indicating copy to clipboard operation
Terminal.Gui copied to clipboard
verified publisher icon gui-cs

Refactor/rewrite `TabView` to use modern v2 capabilities

Open tig opened this issue 5 months ago • 6 comments

These built-in Views have been introduced in v2 or re-written in v2 to remove massive amounts of complex custom code and complexity by leveraging the fact that v2 has better infrastructure for compound/nested views.

  • Menuv2 etc...
  • Shortcut
  • StatusBar
  • OptionSelector (RadioGroup)
  • FlagSelector
  • ColorPicker
  • ScrollBar
  • DatePicker
  • NumericUpDown

These Views remain using mostly the v1-legacy model and thus are good candidates for cleanup. Listed in priority order based on how I perceive how much bang-for-the-buck there is in re-working them.

  • TabView
  • ComboBox
  • Slider
  • TableView
  • TreeView

I think we should tackle TabView because:

  • TabView is a great control for testing the library. It has a lot of capabilities that stretch the library. The v1 version was internally very complex in order to work around limitations in TG. In theory, v2 should enable making this control a lot simpler.
  • Like Shortcut, StatusBar, and Menuv2 it is a container for arbitrary other Views. This means it can both take advantage of and prove out things like Command propagation, Content Scrolling,, KeyBindings, MouseBindings, Adornments, ViewArrangement, and so forth.
  • It is a bug-farm. The code is full of duplicative code, complex switch statements, and complex overrides and interactions that can and should be simplified.

I had a PR going (#3832) where I experimented with this but gave up to focus on other things. In it, I suggested a design incorporating things like this:

  • Content scrolling - The TabRowView can/should just be a horizontally scrolling View where each Tab is just a subview. All the complex TabView logic for renering the Tabs and supporting scrolling can go away.
  • Layout - Dim.Auto, deferred layout, etc... should reduce the need for a lot of the internal complexity. Especially the complex layout math scattered around.
  • Drawing - There should be no need for TabView to draw lines directly. Each element can just be a View and leverage Border an automatic line joins currently enabled by SuperViewRendersLineCanvas
  • Navigation - How a user navs with the keyboard across Tabs, TabRowView, and the tab content is complex. The new v2 Nav system should make this easier.

Rough Design Proposal

  • TabView - A host of mutiple Tabs which are displayed horizontally above or below. One tab at a time can have focus, and when it does, that tab's content is displayed and the user can interact. The TabView class has ONE subview of type TabRowView. This subview is aligned either at the top (Y = 0) or bottom (Y = Pos.AnchorEnd) and fills the TabView horizontally.
    • Uses an int based system for indexing on the tabs (e.g. SelectedTabIndex)
    • Listens to the Selecting event on TabRow - the index of the tab that was clicked on is passed in the CommandContext. The SelectedTabIndex is set based on this.
  • **TabRow ** - The host of the Tab objects for a TabView. Each Tab is simply added as a Subview, thus the list of Tabs is stored in this class (and accessable via the strongly typed TabRow.Tabs). Scrolls the Tabs horizontally within it's Viewport (the ContentSize is set to the sum of the widths of all the Tabs). Includes two scroll buttons that are made visible when needed to enable mouse scrolling across the tabs.
    • When the user clicks on a tab, presses it's hotkey, or changes focus to a tab and presses Space, the Selecting event is raised (see above).
  • Tab - Represents one of the tabs in the TabView. Holds a reference to a developer provided View as Tab.View that is activated when the Tab has focus.
    • When added to TabView via TabView.AddTab the Tab gets added to TabView._tabRow as a subview. TabRow takes care of changing the Tab.Border and Tab.Margin (as well as dimensions) such that the tabs render nicely as "tabs" simply leveraging the Border line rendering code. (Not complete in current WIP).

Screenshot of some code from that abandoned PR:

image

This is in AllViewsTester utilizing this

    public bool EnableForDesign ()
    {
        AddTab (new () { Text = "Tab_1", Id = "tab1", View = new Label { Text = "Label in Tab1" } }, false);
        AddTab (new () { Text = "Tab _2", Id = "tab2", View = new TextField { Text = "TextField in Tab2", Width = 10 } }, false);
        AddTab (new () { Text = "Tab _Three", Id = "tab3", View = new Label { Text = "Label in Tab3" } }, false);
        AddTab (new () { Text = "Tab _Quattro", Id = "tab4", View = new TextField { Text = "TextField in Tab4", Width = 10 } }, false);

        return true;
    }

This is vastly different design than the old design that SHOULD result in a far simpler and performant system. It aslo completely leverages new v2 infrastructure.

The current prototype has the following issues that need to be addressed:

  • The bottom line in the TabRow does not go all the way to the right past the last tab.
  • Scrolling of the Tabs in TabRow is not quite right. Needs tweaking.
  • A bunch of legacy code still exists in TabView

I think we should revisit this and I hope someone besides me finds the motivation to do so!

tig avatar Jul 01 '25 15:07 tig

Almost of this work was already done and with more features in the #3876. Every component is already a view and the lines are margin/border manipulation which require a lot of code by using SuperViewRendersLineCanvas. Could be simplified? Maybe but we need to find common factors which isn't very easy. You don't need to start from zero.

BDisp avatar Jul 01 '25 16:07 BDisp

This is also why I want you to rewrite TabView (not hack old code). TabView is also a great example of a View that is

  1. A compound view (TabRow, Tabs, ...).
  2. Intended to be used as a SubView of other complex/compound Views (Dialog, Window, ...).
  3. Intended to ALSO be a SuperView of other complex/compound Views (contents of each Tab).

In this case I don't quite agree with you. Imagine that you have 1000 tabs. If we add them immediately as subviews it will affect performance too much. It is better to store them in a property and just add them as they are visible on the screen, as it is now. I am not referring only to the tabs but mainly to the subviews that contain the content of each tab. In compound views with fewer subviews this is not a problem but when it involves a huge number of subviews it can be problematic in terms of the appropriate response.

Your response that #3876 does all this already indicates you do not understand what I'm struggling with. If you did, you'd immediately see why, for example, having TabRow have custom logic for dealing with mouse clicks means it is not leveraging the Command infrastructure.

I understand and I know that the changes in PR #3876 are not perfect yet and that is why I put it as a draft. Of course with the excellent implementations, thanks to you, for KeyBinding and MouseBinding we should take advantage of these advantages and indeed in this aspect it will have to be improved.

I am obviously failing to explain things to you, and that's my fault. I will not give up, but I would really appreciate it if you'd give doing what I've suggested a try: Try to rewrite TabView to use ONLY modern v2 capabilities. I really think it will help you understand the underlying problem better.

Starting from scratch will be problematic, as it is very complex. I can try to implement the changes related to keyboard and mouse processing, using Command. And mainly reduce as much of the code related to LineCanvas as possible. I will see what I can do.

BDisp avatar Jul 03 '25 21:07 BDisp

I agree with this sentiment around subviews. Especially if you are looking to tackle TableView next. Not every cell/tab needs to be a view.

The initial tab row view was intended to render 'as many tabs as possible' given the screen space available - scrolling is done 'one tab at a time' and not by characters like a regular scroll view would be.

Likewise TreeView should not have subviews for branches etc. We cannot layout the whole tree at once using View layout subsystem.

The initial code for TabView was only 800 lines of code:

  • #1137

The switch to LineCanvas while sensible has really ballooned the complexity

tznind avatar Jul 03 '25 21:07 tznind

I agree with this sentiment around subviews. Especially if you are looking to tackle TableView next. Not every cell/tab needs to be a view.

In my PR #3876 the tabs are already views, but I only add to SuperView the tabs that can be visible in the available space on the screen. This aspect has already been overcome.

The initial tab row view was intended to render 'as many tabs as possible' given the screen space available - scrolling is done 'one tab at a time' and not by characters like a regular scroll view would be.

That's my idea too. It doesn't make sense to present part of a tab that can't be fully rendered.

Likewise TreeView should not have subviews for branches etc. We cannot layout the whole tree at once using View layout subsystem.

Maybe it can be done. Each main branch that is also a view, contains subviews that in turn may contain subviews. But this only makes sense if you only add subviews as needed when expanding. When collapsing, only the main branch is visible by changing the size, not being able to show all the subviews. I think that manipulating the LineCanvas in this case will not be as complex as the TabView, but it would require a huge effort. But I confess that this will not be easy at all. It's more of a task for @tig, joking.

The initial code for TabView was only 800 lines of code:

The switch to LineCanvas while sensible has really ballooned the complexity

The problem is really the complexity of LineCanvas. It might be possible to use a common method that receives all the correct parameters for each orientation, but I still can't do it because the variation is not uniform. The ideal would be to create a class that uses all this complexity to fill all possible corners automatically. The code not only manipulates the border but also the margin, which increases the degree of difficulty even more.

BDisp avatar Jul 03 '25 23:07 BDisp

In this case I don't quite agree with you. Imagine that you have 1000 tabs.

If a developer builds an app with a TabView more than around 20 Tabs they are creating a horrible end-user experience. You are prematurely optimizing for a hypothetical that will never happen.

I agree with this sentiment around subviews. Especially if you are looking to tackle TableView next. Not every cell/tab needs to be a view.

I totally agree with this for TreeView and TableView. TabView is a different beast. As noted above, having dozens of Tabs means a developer is not thinking right about how to create a great user experience.

The initial tab row view was intended to render 'as many tabs as possible' given the screen space available - scrolling is done 'one tab at a time' and not by characters like a regular scroll view would be.

Agree. My abandonded PR on this made each Tab in the TabRow a Tab and scrolled by whole tabs.

https://github.com/gui-cs/Terminal.Gui/pull/3832

The initial code for TabView was only 800 lines of code:

The switch to LineCanvas while sensible has really ballooned the complexity

I assert a rewrite/refactor along the lines of what I started in https://github.com/gui-cs/Terminal.Gui/pull/3832 will get it back to roughly that amount of code.

Here's an example. These complex, bug-prone, hard to follow, examples of existing go away if, instead, we rely on subviews and the layout/draw engine which has been rebuilt with the goal of making compound subviews like this easy to build:

https://github.com/gui-cs/Terminal.Gui/blob/0a8be1082207def91da93ce01f9943941053d8c7/Terminal.Gui/Views/TabView/TabView.cs#L621

https://github.com/gui-cs/Terminal.Gui/blob/0a8be1082207def91da93ce01f9943941053d8c7/Terminal.Gui/Views/TabView/TabView.cs#L839

https://github.com/gui-cs/Terminal.Gui/blob/0a8be1082207def91da93ce01f9943941053d8c7/Terminal.Gui/Views/TabView/TabRow.cs#L153

(And those links don't even highlight the LineCanvas junk).

The problem is really the complexity of LineCanvas. It might be possible to use a common method that receives all the correct parameters for each orientation, but I still can't do it because the variation is not uniform. The ideal would be to create a class that uses all this complexity to fill all possible corners automatically. The code not only manipulates the border but also the margin, which increases the degree of difficulty even more.

Oh, you mean by "create a class", the View class with this Issue fully resolved:

  • #2994

In that PR I had this working:

Image

Starting from scratch will be problematic, as it is very complex. I can try to implement the changes related to keyboard and mouse processing, using Command. And mainly reduce as much of the code related to LineCanvas as possible. I will see what I can do.

Yes, fixing Border and perhaps some LineCanvas auto-join issues, is required to make that render so they actually look like tabs, but it is just work... not rocket science. The biggest work item is

  • #3407

I didn't invest all the energy I put into building/refactoring Adornment, Layout, Draw, Command, etc... for v2 just because I thought it was cool. Nor did I do it without having existing problematic areas of TG in mind. Shortcut->StatusBar->Menuv2 were the things I focused on first, but TabView was always next; with the hope that it could be redone to be a shining example of using that infrastructure correctly.

If you are not interested in doing it right (which means tackling #3407 as part of it), then I'd rather you not do it at all, because it means that all that stuff I built doesn't have a critical "customer" (TabView) and thus is not fully proven out.

tig avatar Jul 06 '25 19:07 tig

I think you already know that the image of your early work on TabView was what I faced when I started working on it and that was the easiest to face. Now to complete the decorations depending on whether the tab is selected or not, the orientation, etc..., then you will see the difficulty that this represents. I can't promise anything, except the commitment to do my best and within my knowledge.

BDisp avatar Jul 06 '25 20:07 BDisp