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

SimpleTree widget #1767

Closed
bruno-rino opened this issue Feb 3, 2023 · 10 comments · Fixed by #2017
Closed

SimpleTree widget #1767

bruno-rino opened this issue Feb 3, 2023 · 10 comments · Fixed by #2017
Labels
enhancement New features, or improvements to existing features.

Comments

@bruno-rino
Copy link
Contributor

What is the problem or limitation you are having?

My use case calls for a Tree widget with a single column.

The current implementation (on macOS) always displays the header. The result is weird.
I also noticed the winforms implementation of the Tree widget is not_implemented. Which makes sense, because it seems winforms' TreeView only allows for a single column.

Describe the solution you'd like

I purpose a new SimpleTree widget that implements this behavior. Basically a Tree widget that allows only a single column, and does not show a header.

The SimpleTree could be implemented in winforms.

Describe alternatives you've considered

  • The simplest solution would be to add a header_visible parameter to the Tree widget. But the winforms implementation would still be incomplete.
  • I would actually prefer that this new widget would be called Tree, and the current widget be renamed TreeTable or HierarchicalTable. But that might be too much of a breaking change.
  • Another option might be to only show the header when there is more than one column. But isn't that a bit too magical? And it also doesn't address the winforms implementation.

Additional context

How to hide the header:

macOS

NSOutlineView.headerView = None

I couldn't find apple docs on it, but there are online sources, e.g. https://stackoverflow.com/questions/3514210/hiding-nstableview-header#63551501

GTK

https://docs.gtk.org/gtk3/method.TreeView.set_headers_visible.html

@bruno-rino bruno-rino added the enhancement New features, or improvements to existing features. label Feb 3, 2023
@freakboy3742
Copy link
Member

Agreed that this is a problem that needs a solution - Tree on Winforms (and Android/iOS for that matter) is one of the last big missing pieces.

Obviously the ideal case would be a fully-featured "TreeTable" on Winforms; but that's obviously going to be a complicated prospect given the "native" control that is available.

I'm not sure how I feel about having multiple tree widgets; If we're going to take this approach, the second option ("Tree" and "TreeTable") is the more appealing - but I'm not sure how we'd deal with the issue with backwards compatibility.

However, I don't think the third approach is necessarily as "magical" as you describe. I can see 2 possible API-level signals to control what style of tree is displayed:

  • Was the Table constructed with a "headings" list? If the user didn't specify headings, then (arguably) a heading shouldn't be displayed.
  • Does the accessor controlling the data source specify multiple values?

In the macOS/GTK case, we'd change the behavior to hide the header row unless the user explicitly specified a header (which would then necessitate that they define an accessor list). The rest of the widget would be largely as-is. We might also want to modify "accessors" to allow for a single string accessor, in addition to a list of accessors.

In the Windows case, we'd only (initially) support a reduced feature set:

  1. If header is specified, it would be ignored (with a warning)
  2. If the accessors specify multiple values, we'd either raise an error, or ignore all but the first attribute (again, with a warning)

Looking at iOS/Android, I'm guessing Tree won't manifest as a traditional desktop "expand the triangle to see the child tree nodes" tree, but as a Navigation hierarchy - show the list of top level items; if you click on one, you navigate "deeper" into the tree.

Does that make sense as an approach?

@bruno-rino
Copy link
Contributor Author

Was the Table constructed with a "headings" list? If the user didn't specify headings, then (arguably) a heading shouldn't be displayed.

I like this idea. If the user doesn't specify headings, what else could we do?

Does the accessor controlling the data source specify multiple values?

You mean the accessors parameter, as in toga.Tree(headings=None, data=data_list, accessors=['item']), right? I don't like that so much. Someone might want to have the header in place even with a single column. Or someone might even want do hide the header when there are multiple columns!

hide the header row unless the user explicitly specified a header (which would then necessitate that they define an accessor list).

That indeed makes for a straightforward implementation . But a bit odd in the case of the most basic usage of the widget, e.g.:

data = [['item1'], ['item2']]
toga.Tree(headings=None, data=data, accessors=['somethinghastobehere'])

Maybe we should synthesize an accessor for this case?

@freakboy3742
Copy link
Member

Was the Table constructed with a "headings" list? If the user didn't specify headings, then (arguably) a heading shouldn't be displayed.

I like this idea. If the user doesn't specify headings, what else could we do?

We could potentially synthesise headings from the accessors... but I don't think that's an especially good idea. Best to keep the two

Does the accessor controlling the data source specify multiple values?

You mean the accessors parameter, as in toga.Tree(headings=None, data=data_list, accessors=['item']), right? I don't like that so much. Someone might want to have the header in place even with a single column. Or someone might even want do hide the header when there are multiple columns!

Yes, I was referring to the accessors parameter.

In terms of the two edge cases you describe:

  • Header in place with a single column:
    • toga.Tree(headings=['foo'], data=...) - accessor is implied from the heading
    • toga.Tree(headings=['foo'], data=..., accessors=['item']) - explicit accessor definition
  • Hide the header, multiple columns of data:
    • toga.Tree(data=..., accessors=['item1', 'item2']) - no headings provided, implied "no headings"
    • toga.Tree(headings=None, data=..., accessors=['item1', 'item2']) - explicit "no headings".

I'm not sure I follow where the confusion/problem is though. What I'm proposing is a strict separation of concerns:

  • headings specifies whether you want headings to be displayed, and if so, what they are.
  • accessors tells us how many columns of data you want to see.

Accessors can be inferred from headings, if headings are provided. If you're not displaying headings, you need to give some signal to the tree on what needs to be displayed (with one caveat for the 'basic usage' that you mention - discussed below)

hide the header row unless the user explicitly specified a header (which would then necessitate that they define an accessor list).

That indeed makes for a straightforward implementation . But a bit odd in the case of the most basic usage of the widget, e.g.:

data = [['item1'], ['item2']]
toga.Tree(headings=None, data=data, accessors=['somethinghastobehere'])

Maybe we should synthesize an accessor for this case?

Agreed - I guess I should clarify my API specification as:

  • If an explicit data source is specified, then accessors have to be specified to describe which attributes of the source are being displayed.
  • If a data source is generated by the widget to wrap a list/dict, the user can provide accessors; but in the absence of explicit accessors, they will be implied from the data to the extent possible:
    • An empty list - an error is raised; you need to specify at least an accessor list to scope out the size of the data table (or, implied accessors from a heading specification).
    • A list of single items - one column accessor.
    • A list of lists: as many columns as the size of the first item in the list.
    • A list of dictionaries: as many columns as the first item in the list has keys.

tl;dr - Tree will try to make the most of whatever it's given. Specifying headings is an explicit signal to display them; accessors describe how many columns there will be; accessors can be implied from headings, if they're provided, or the data allows.

@bruno-rino
Copy link
Contributor Author

Thank you for the detailed reply - I'm always impressed how much thought you put into these.

I still a few questions:

1.

A list of single items - one column accessor.

You mean toga.Tree(data=[['item1'], ['item2']])? Because that's just a specific case of the next rule ("A list of lists")

If you mean toga.Tree(data=['item1', 'item2']), the Tree widget doesn't support that. Maybe it should?

2.

There's one case missing in yout API specification (the Tree widget is really very versatile):

    • A dict of list/tuples as keys: as many columns as the size of the first item in the list.

Here's an example:

        data = {
            ('item2', 'desc2'): {
                ('item3', 'desc3'): None
            },
            ('item1', 'desc1'): None,
        }

3.

Off-topic from the header issue, but on-topic for the tree widget: in the case of "a dict of list/tuples as keys" (the example above) for some reason the items are sorted by the Tree widget. Weird.

@freakboy3742
Copy link
Member

Thank you for the detailed reply - I'm always impressed how much thought you put into these.

I still a few questions:

1.

A list of single items - one column accessor.

You mean toga.Tree(data=[['item1'], ['item2']])? Because that's just a specific case of the next rule ("A list of lists")

If you mean toga.Tree(data=['item1', 'item2']), the Tree widget doesn't support that. Maybe it should?

Yeah - I didn't think about that enough. What I was aiming for was using "cardinality of the first leaf node" as an indicator of the number of columns; however, as you've pointed out, that's potentially ambiguous: is [['item1'], ['item2']] a tree with 2 (unnamed?) root nodes, each of which has a single child; or is it a tree with 2 root-level leaf nodes, indicating a single column of data.

Mapping primitive data structures into trees is always going to be a little ambiguous; the best we can do is try to make sure that it's at least predictably ambiguous. I'm definitely open to suggestions here - and I'm not fundamentally opposed to breaking some existing use cases if it makes the overall situation more explicit and less ambiguous.

2.

There's one case missing in yout API specification (the Tree widget is really very versatile):

    • A dict of list/tuples as keys: as many columns as the size of the first item in the list.

Here's an example:

        data = {
            ('item2', 'desc2'): {
                ('item3', 'desc3'): None
            },
            ('item1', 'desc1'): None,
        }

I think that follows - although there's possibly an ambiguity around a 2-item tuple where the first item is an icon.

The more I think about this, the more I'm becoming convinced it might be desirable to lock down the number of possible forms that truly "free-form" primitive data can take, with a view to being very clear about how many columns there are, and what those columns are. It's annoying that this may lock out some existing use cases, but otherwise we're going to tie ourselves in knots trying to reason about what input data "means" in terms of tree rendering.

3.

Off-topic from the header issue, but on-topic for the tree widget: in the case of "a dict of list/tuples as keys" (the example above) for some reason the items are sorted by the Tree widget. Weird.

I can't say for sure, but my guess is that this was done for testing consistency. Some of this code predates Python 3.6, and prior to Python 3.6, dictionary order wasn't guaranteed - so testing was unreliable as you couldn't guarantee iteration order. Dictionary iteration is now guaranteed to be insertion order, so the sort is probabaly not needed. Probably :-)

@bruno-rino
Copy link
Contributor Author

I'm back. Apologies for the long absence.

Below is my try at documenting the form primitive data can take. I made up some concept names (collection, nodes, cells). I'm not a huge fan of the names I came up with, but I found it hard to explain without assigning names to the concepts.

In the meantime, I'm working on tests (and migrating to pytest). I'll submit a PR soon.

The tree data are a collection of nodes. If it is not an object with the same interface as `toga.sources.tree_source.TreeSource`, it will be converted:
- if dict: the key is the parent node, the value is a collection of children nodes
- if tuple/list: the nodes cannot have children

Nodes are a collection of cells. If it is not an object with the same interface as `toga.sources.tree_source.Node`, it will be converted:
- if dict: the key is the accessor, the order is not relevant
- if tuple/list: the order will match the accessor list

A cell is an object to display in the cell as a string, or a tuple of (icon, object)

@freakboy3742
Copy link
Member

Welcome back :-)

I'm not sure if you've noticed, but we're in the process of doing a complete audit of all the widgets, including trying to get them all to 100% pytest coverage and fully documented, including documenting the related data constructs. ListSource has already been documented (as part of the Selection widget - see #1955); the Table and Tree widgets are on the list to be looked at in the near future. However, if you want to beat us to it, I won't complain :-)

@freakboy3742
Copy link
Member

Reopening this since we opted to not implement it as part of this pass of Tree.

@freakboy3742 freakboy3742 reopened this Aug 30, 2023
@bruno-rino
Copy link
Contributor Author

The issue I raised has been implemented: providing no headings will not show the header.

Of course, there's still no winforms implementation, but that's a different issue.

The implementation for "primitive data" is quite different from the discussion on this ticket, but at least it is documented now.

So, as far as I'm concerned, this issue can be closed.

@freakboy3742
Copy link
Member

Yeah - it's probably better to close this, and open a separate ticket (once the audit is complete) to backfill the Windows Tree widget specifically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New features, or improvements to existing features.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants