Skip to content

[Debugger Plugin]: Add spinners for the debugger page #792

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

Merged
merged 1 commit into from
Dec 8, 2017
Merged

[Debugger Plugin]: Add spinners for the debugger page #792

merged 1 commit into from
Dec 8, 2017

Conversation

caisq
Copy link
Contributor

@caisq caisq commented Dec 6, 2017

The spinners are activated during rendering of the node list and the
graph, as well as the TF runtime execution time following the
stepping/continuing actions.

The spinners are activated during rendering of the node list and the
graph, as well as the TF runtime execution time following the
stepping/continuing actions.
@caisq
Copy link
Contributor Author

caisq commented Dec 6, 2017

cc @chihuahua

@chihuahua chihuahua self-requested a review December 6, 2017 20:25
Copy link
Member

@chihuahua chihuahua left a comment

Choose a reason for hiding this comment

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

So this spinner generally makes parts of the debugger dashboard await progress during graph loading? Makes sense to me.

_renderHierarchyWithTimeout(watchHierarchy, debugWatchChange, filterMode, filterInput) {
this._clearSelectorHierarchy();
this.set('_isLoading', true);
setTimeout(() => {
Copy link
Member

Choose a reason for hiding this comment

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

I believe

this.async(() => {
  this._renderHierarchy(watchHierarchy, debugWatchChange, filterMode, filterInput);
});

works here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As per offline discussion, setTimeout is correct here. The purpose is to let the spinner DOM update by pausing for a few milliseconds fist.

@caisq caisq merged commit f5dec9c into tensorflow:master Dec 8, 2017
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.

2 participants