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 #3875. Add left and right tab sides with alignments to the TabView. #3876

Open
wants to merge 21 commits into
base: v2_develop
Choose a base branch
from

Conversation

BDisp
Copy link
Collaborator

@BDisp BDisp commented Dec 2, 2024

Fixes

Proposed Changes/Todos

  • Add tabs on left and right with alignments feature. Note that alignments are only noticed on tabs on left and right because tabs on top and on bottom the tabs length are always justified to the text length.

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

@BDisp BDisp requested a review from tig as a code owner December 2, 2024 02:15
@BDisp
Copy link
Collaborator Author

BDisp commented Dec 2, 2024

@tig there are 4 unit tests failing because the Clip is returning a empty region when the down arrow being draw. This only happen when it's needed to draw over the tab border, which could be possible because the down arrow super-view is the TabRow and not the last tab. Only when the selected tab is the last on the botton the down arrow is draw. I need your help to find what it's causing this failure, please. Here the unit tests failing:

Add_Three_TabsSide_Left_ShowInitialLine_False_ChangesTab_Height5

    Expected:
───────────────────┐
Tab1 hi            │
────╮              │
Tab2│              │
────▼──────────────┘
 But Was:
───────────────────┐
Tab1 hi            │
────╮              │
Tab2│              │
────╯──────────────┘

Add_Three_TabsSide_Left_ShowInitialLine_True_ChangesTab_Height5

Expected:
╭──────────────────┐
│Tab1 hi           │
├────╮             │
│Tab2│             │
╰────▼─────────────┘
 But Was:
╭──────────────────┐
│Tab1 hi           │
├────╮             │
│Tab2│             │
╰────╯─────────────┘

Add_Three_TabsSide_Right_ShowInitialLine_False_ChangesTab_Height5

Expected:
┌───────────────────
│hi             Tab1
│              ╭────
│              │Tab2
└──────────────▼────
 But Was:
┌───────────────────
│hi             Tab1
│              ╭────
│              │Tab2
└──────────────╰────

Add_Three_TabsSide_Right_ShowInitialLine_True_ChangesTab_Height5

Expected:
┌──────────────────╮
│hi            Tab1│
│             ╭────┤
│             │Tab2│
└─────────────▼────╯
 But Was:
┌──────────────────╮
│hi            Tab1│
│             ╭────┤
│             │Tab2│
└─────────────╰────╯

The strange is if there isn't enough space to add another tab an vertical line is added and the down arrow is draw. This issue only happens if the height fit exactly the tabs than can be currently visible.

────┐
Ta h│
──╮ │
  ▼─┘

┌────
│h Ta
│ ╭──
└─▼  

@BDisp
Copy link
Collaborator Author

BDisp commented Dec 3, 2024

It was a bug with the _rightDownScrollIndicator using the MoveSubviewToStart method instead of the MoveSubviewToEnd. Fixed in c932306.

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.

As long as you rename the 'RenderTabLine` methods, I'll merge this.

But it doesn't make me happy that you've taken an already complex beast and made it more complex just because it would be cool to have tabs on the sides.

private void Tab_DisplayTextChanged (object? sender, EventArgs e) { _tabLocations = CalculateViewport (Viewport); }

/// <summary>Renders the line with the tab names in it.</summary>
private void RenderTabLine (Tab []? tabLocations)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, this method is just mis-named causing me to think you were doing drawing w/in Layout.

}

LineCanvas.Merge (lc);
}

/// <summary>Renders the line of the tab that adjoins the content of the tab.</summary>
private void RenderUnderline ()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please rename this method to something that actually describes what it does.

}

LineCanvas.Merge (lc);
Copy link
Collaborator

Choose a reason for hiding this comment

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

The above code is insane.

1000 lines of highly repetitive logic that could be replaced with just a few lines if this were completed:

I get annoyed that you continue to expand on this vs. helping me think through how to make #3407 really work well.

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.

Add left and right tab sides with alignments to the TabView.
2 participants