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

Animating background for tree nodes #3450

Closed
vitaliy-guliy opened this issue Nov 9, 2018 · 18 comments · Fixed by #7249
Closed

Animating background for tree nodes #3450

vitaliy-guliy opened this issue Nov 9, 2018 · 18 comments · Fixed by #7249
Assignees
Labels
navigator issues related to the navigator/explorer tree issues related to the tree (ex: tree widget) ui/ux issues related to user interface / user experience

Comments

@vitaliy-guliy
Copy link
Contributor

Now expanding of tree nodes works asynchronously and it is not clear how long it needs to wait until the children appear. It's looks strange when the user click on tree node, arrow on this node has been changed on opened but children appears with some delaying.
Let's see how expanding of tree nodes works with tree-sample-plugin provides ftp-explorer widget.

expanding-tree-nodes

In this example the opening time depends on the speed of the network and may take some time.

The proposal is to add animation to the background of the tree node when it's fetching the children.

We did something similar for our Che IDE. Let's see how it looks

animation when expanding tree nodes in che ide

The same video https://youtu.be/RYAopXBC0HU

The styles for the tree node may look like styles for Che IDE

https://github.com/eclipse/che/blob/master/ide/che-core-ide-ui/src/main/resources/org/eclipse/che/ide/ui/smartTree/TreeStyles.css#L144

@vitaliy-guliy vitaliy-guliy added good first issue good first issues for new contributors navigator issues related to the navigator/explorer tree issues related to the tree (ex: tree widget) ui/ux issues related to user interface / user experience labels Nov 9, 2018
@akosyakov
Copy link
Member

I like an idea of displaying loading, especially when a connection is lost. Also if loading is failed it would be nice to show a reason, maybe as a child error node.

Not sure about the animated background. It could be also a child node with a spinning loading indicator which either resolves to real children or to an error node.

//cc @svenefftinge

@akosyakov
Copy link
Member

Displaying resource resolution is not specific to trees. There can be different resources opened, not necessary from the navigator. In VS Code one can report progress as the blueish bar running at the top of the main area, maybe we can make use of something like that for Theia as well.

@vitaliy-guliy
Copy link
Contributor Author

What shall we do when node has no children? Adding and then removing temporary node may look KO.
I have just tried to play with VS Code but cannot see blue bar.
When use animation we can add a simple delay on start. Then it will not be displayed when children appear immediately.

@vitaliy-guliy
Copy link
Contributor Author

vitaliy-guliy commented Nov 9, 2018

Another issue. We have to show something when tree is opening root nodes?
Let's see at the first animated gif.
The FTP Explorer panel has a tree which begin loading root nodes as sun as become attached to the DOM. The loading time is about a second. It may be much more due to network issues.

@vitaliy-guliy
Copy link
Contributor Author

I would like to try a version with animated background as a cheapest option. And then decide how good it looks.

@akosyakov
Copy link
Member

I have just tried to play with VS Code but cannot see blue bar.

I am not sure what is triggering it in VS Code. I see it from time to time. It looks like here, only in the main area: microsoft/vscode#50730 (comment)

Another issue. We have to show something when tree is opening root nodes?

With the progress bar, we can run it till there is no content.

@vince-fugnitto
Copy link
Member

@vitaliy-guliy @akosyakov I really like the FTP Explorer styling with the title band and icons, is it something we can easily update our Files widget to? I'm also thinking of adding buttons to the band for the Files view to have for instance a Collapse All Nodes feature.

@vitaliy-guliy
Copy link
Contributor Author

@vince-fugnitto

As we discussed earlier

  • not every part is View Container (I may be wrong).
  • only View Container can show those title bands for his views.
  • when view container has only one view, title band does'n have expand/collapse arrow and is always visible (for an instance, in my demo FTP Explorer view can be collapsed).

Something like VS Code does

explorer part

@vince-fugnitto
Copy link
Member

@vitaliy-guliy yes exactly vscode does something quite similar. I'll see if I can somehow adopt to our Files view, I really like the feature and UI for it :)

@akosyakov
Copy link
Member

@vince-fugnitto please discuss it with @svenefftinge first, it was not done on the purpose to have more space and avoid duplicate title (we have a title already in the sidebar).

For collapse all action we can add a menu item which is available on all nodes.

@vince-fugnitto
Copy link
Member

@akosyakov thanks and I agree that we save some space but I was also thinking in the case that we display icons rather than titles in the sidebar (#2867). Perhaps in such a case we'd want to display widget titles within the widget themselves for easier identification.

@svenefftinge
Copy link
Contributor

svenefftinge commented Nov 13, 2018

I reopened #716 for the vertical tab bar icons. Let's discuss there

@svenefftinge
Copy link
Contributor

Regarding this issue, I'd prefer to look into a general way of reporting progress. Also because we need to support the VS Code Api in our plug-in system I'd say it makes sense to go with a similar approach. I.e. show progress on top of a widget.

@vitaliy-guliy
Copy link
Contributor Author

We can support the VS Code API and do whatever we want with Theia UI.
Having a general way of reporting the progress it's certainly good. But then in a case with the tree we need to connect it somehow with a parent container and this container must have the API to display a progress.
Let's describe which progress we have to track:

  • loading folder content in the tree
  • loading file content on double click tree node
  • something else??

I think having some animation in the tree is enough for now.

By the way, I don't see that VS Code API describes the way to display a progress inside view.
But it describes window.withProgress(...) method to display a progress on top of the window.

@svenefftinge
Copy link
Contributor

So we are in agreement that it would make sense to implement it similarly to VS Code?
I think that's the way to go, because

  • we would need to support the VS Code API anyway
  • the VS Code extension implementation has a certain expectation to how the UI does it.
    Also, I think it just works nicely. So no need to do it different.

svenefftinge added a commit that referenced this issue Jun 26, 2019
@akosyakov akosyakov self-assigned this Feb 25, 2020
@akosyakov akosyakov removed the good first issue good first issues for new contributors label Feb 25, 2020
@akosyakov
Copy link
Member

akosyakov commented Feb 25, 2020

The blue progress bar is available already. We are going to add loading spinner for any tree to align with VS Code UI.

36672590-1e8e5cb0-1b00-11e8-99b8-74aa7b2d2645

It will be shown on Tree.resolveChildern after some configurable delay.

@westbury
Copy link
Contributor

It will be shown on Tree.resolveChildern after some configurable delay. It could be disabled by setting delay to 0.

@akosyakov That looks like #5535. At the time I was holding off as other progress work was being done and I never got back to it. Our use-case was slightly different, as we wanted the progress indicator to be active when certain context actions are unavailable on a tree node, eg a tree node representing a project that is being built. If you have this in hand then I will close the PR, otherwise I will resurrect it and ensure it covers this issue.

@akosyakov
Copy link
Member

@westbury ok, thanks for reminding, i need quite urgent for the customer, so planing to work myself. I will have a look at your PR though.

akosyakov added a commit that referenced this issue Feb 29, 2020
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
akosyakov added a commit that referenced this issue Mar 1, 2020
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
akosyakov added a commit that referenced this issue Mar 1, 2020
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
akosyakov added a commit that referenced this issue Mar 1, 2020
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
akosyakov added a commit that referenced this issue Mar 1, 2020
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
akosyakov added a commit that referenced this issue Mar 2, 2020
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
JesterOrNot pushed a commit to JesterOrNot/theia that referenced this issue Mar 12, 2020
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
navigator issues related to the navigator/explorer tree issues related to the tree (ex: tree widget) ui/ux issues related to user interface / user experience
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants