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 #2683 - Adds an ITableSource which wraps a TreeView<T> #2685

Merged
merged 19 commits into from
Jul 5, 2023

Conversation

tznind
Copy link
Collaborator

@tznind tznind commented May 29, 2023

Fixes #2683 - Adds an ITableSource which wraps a TreeView<T>

@LPeter1997 the quick win is done, now the 90% effort polishing happens ;)
tree-table-go-go

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

@tznind tznind mentioned this pull request May 29, 2023
@LPeter1997
Copy link

Oh my, this looks perfect! Can't wait to put my hands on it

@tznind tznind changed the title WIP: Add TreeTableSource Fixes #2683 - Adds an ITableSource which wraps a TreeView<T> May 31, 2023
@tznind tznind marked this pull request as ready for review June 1, 2023 04:22
@tznind tznind requested review from migueldeicaza and tig as code owners June 1, 2023 04:22
@tznind
Copy link
Collaborator Author

tznind commented Jun 1, 2023

Ok I've marked this as ready for review now. I've added both mouse and keyboard support for expansion/collapse. I've also put the scenario into TableEditor in UICatalog so it can be more easily tested with other TableView features (styles, checkboxes etc).

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. See my minor comments.

/// </summary>
/// <typeparam name="T"></typeparam>
public class TreeTableSource<T> : IEnumerableTableSource<T>, IDisposable where T : class {
private TreeView<T> tree;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please prefix privates with _ and don't use this..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in bc14970

@@ -0,0 +1,92 @@
nf-dev-git
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a demo file? Put it in the UI Catalog/Scenarios folder? Give it a name that will help identify it's purpose?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops was not meant to be comitted. This was a left over temp file from when I was adding the nerd fonts to file dialog. Have removed.

UnitTests/Views/TreeTableSourceTests.cs Show resolved Hide resolved
@tznind tznind marked this pull request as draft June 7, 2023 18:19
@tznind tznind marked this pull request as ready for review June 7, 2023 18:35
@Nutzzz Nutzzz mentioned this pull request Jun 12, 2023
@tznind tznind requested a review from tig June 14, 2023 15:23
@tznind
Copy link
Collaborator Author

tznind commented Jun 14, 2023

Think I have addressed all the points. Let me know if I missed any or misinterpreted.

@tig tig merged commit cd6cfd7 into gui-cs:v2_develop Jul 5, 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.

3 participants