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

Allow to reveal an empty TreeView programmatically #90005

Open
apupier opened this issue Feb 4, 2020 · 13 comments
Open

Allow to reveal an empty TreeView programmatically #90005

apupier opened this issue Feb 4, 2020 · 13 comments
Assignees
Labels
api api-proposal feature-request Request for new features or functionality tree-views Extension tree view issues
Milestone

Comments

@apupier
Copy link
Contributor

apupier commented Feb 4, 2020

Currently, it i possible to reveal/expand a TreeView only when there is an element in the TreeView. it is oputting revealing this element in the TreeView.

it would useful to be abel to expand the TreeView even when there are no elements in the Tree.

Use case:

asked on StackOverflow https://stackoverflow.com/questions/59983711/how-to-programmatically-make-a-treeview-visible

@vscodebot
Copy link

vscodebot bot commented Feb 4, 2020

(Experimental duplicate detection)
Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

@alexr00 alexr00 added tree-views Extension tree view issues feature-request Request for new features or functionality labels Feb 5, 2020
@alexr00 alexr00 added this to the Backlog Candidates milestone Feb 5, 2020
@vscodebot
Copy link

vscodebot bot commented Feb 5, 2020

This feature request is now a candidate for our backlog. The community has 60 days to upvote the issue. If it receives 20 upvotes we will move it to our backlog. If not, we will close it. To learn more about how we handle feature requests, please see our documentation.

Happy Coding!

@vscodebot
Copy link

vscodebot bot commented Feb 14, 2020

🙂 This feature request received a sufficient number of community upvotes and we moved it to our backlog. To learn more about how we handle feature requests, please see our documentation.

Happy Coding!

@vscodebot vscodebot bot modified the milestones: Backlog Candidates, Backlog Feb 14, 2020
@alexr00 alexr00 modified the milestones: Backlog, November 2020 Nov 6, 2020
@alexr00 alexr00 closed this as completed in e95c40c Nov 6, 2020
@alexr00 alexr00 reopened this Nov 6, 2020
@alexr00
Copy link
Member

alexr00 commented Nov 6, 2020

Proposal is to allow the first parameter here to be undefined:

export interface TreeView<T> extends Disposable {
reveal(element: T | undefined, options?: { select?: boolean, focus?: boolean, expand?: boolean | number }): Thenable<void>;
}

@eamodio
Copy link
Contributor

eamodio commented Nov 6, 2020

I think we should add a show method, rather than overloading reveal. This also aligns with WebviewViews which have:

show(preserveFocus?: boolean): void;

I also think we should add a hide method to both tree views and webview views (which has been requested). Which also better aligns with a show method as we use that on lots of other API objects.

@alexr00
Copy link
Member

alexr00 commented Nov 9, 2020

I think that could result in this pattern:

let element: T | undefined;
// ... element gets set to something
if (element) {
    tree.reveal(element);
} else {
    tree.show();
}

@eamodio
Copy link
Contributor

eamodio commented Nov 9, 2020

It could, but imo that still feels more right to me. Also the reveal method has an options bag that while there is some overlap, feels strange to apply to the view itself. And I feel the alignment with WebviewViews is quite important as well.

@wandyezj
Copy link

wandyezj commented Mar 15, 2022

I'd prefer to simply allow the reveal element to be undefined (as currently proposed).

A questionable alternative is to allow an overload to reveal that moves element to the options parameter.

 export interface TreeView<T> extends Disposable { 
 	reveal(options?: { element?: T, select?: boolean, focus?: boolean, expand?: boolean | number }): Thenable<void>; 
 } 

note: the issue with the overload is that it can clash with the existing reveal definition if the properties of T are a set or subset of the options properties, which while unlikely could be confusing.

Another option is a reveal overload without any arguments.

 export interface TreeView<T> extends Disposable { 
 	reveal(): Thenable<void>; 
 } 

I think a show / hide API might be better as a separate proposal.

@cweijan
Copy link

cweijan commented Mar 18, 2022

I tried to pass a null to reveal, it can switch the treeview normally, which has met my needs.

treeView.reveal(null)

@gar1t
Copy link

gar1t commented Jun 17, 2022

IMHO the overload of reveal is objectively wrong :) show() is established as the function name for this (if I read the OP correctly). It's used by most (if not all) of the other view-like components (webviews, terminal, channels, etc.)

To reveal an item inside a view is view specific, whereas to make the view itself visible is something that applies to any view at all. The fact that show() doesn't exist on the tree view, and does pervasively elsewhere, looks like an implementation oversight.

The options in reveal() show that reveal(null) is nonsensical. These are node specific.

@gar1t
Copy link

gar1t commented Jun 17, 2022

As a driver for this behavior, I want to implement a command that shows a custom tree view in the same way that any of the other views are shown. You can do this manually via View -> Open view... -> . This is the behavior that tree.show() should have IMO. This should not effect the current tree selection but merely show the view and put it into focus (same every other show view command).

@gar1t
Copy link

gar1t commented Jun 17, 2022

Lol, reveal(undefined) already seems to work as described above! (I'm running 1.68.1) It's not yet documented though :)

I propose that an alias of show() be added (I'm glad the behavior's at least now supported!)

@EhabY
Copy link
Contributor

EhabY commented Jan 4, 2024

Indeed, a dedicated show() would be much better especially when using a tree view with type TreeView<SomeNode> then we cannot pass undefined/null. I would have to (falsely) make it TreeView<SomeNode | undefined> and deal with the non-existent undefined or hack Typescript and do treeView.reveal(undefined as unknown as SomeNode). Both options are not ideal..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api api-proposal feature-request Request for new features or functionality tree-views Extension tree view issues
Projects
None yet
Development

No branches or pull requests

7 participants