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.

Sign up for GitHub

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

Jump to bottom

graph: fix spurious error modal on page load #2651

Merged
merged 1 commit into from
Sep 12, 2019
Merged

graph: fix spurious error modal on page load #2651

merged 1 commit into from
Sep 12, 2019

Conversation

wchargin
Copy link
Contributor

Summary:
Fixes #2646, and fixes #2650 as a side effect.

I’ve gone with an additional boolean field _datasetsFetched rather
than making _datasets nullable because _datasets is also passed down
directly to tf-graph-controls, which expects a non-null array. This
seemed simpler, all things considered.

Test Plan:

  1. Launch TensorBoard. Navigate to http://localhost:6006/#graphs.
  2. Observe that there is no flash of “No graph definition files were
    found” (though there is instead a brief flash of white).
  3. Note that the list of graphs loads, the first graph is selected, and
    the fragment updates.
  4. Refresh the page. Note that the page behaves as in the initial load:
    in particular, there is no error modal.
  5. Change the fragment such that the run is invalid (#graphs&run=bad)
    and note that the error modal appears without refreshing the page.
  6. Refresh the page, and note that the error modal appears, and the
    first graph is automatically selected and loaded underneath it.

wchargin-branch: graph-fix-error-modal

@wchargin
graph: fix spurious error modal on page load
d1bc332 
Summary:
Fixes #2646, and fixes #2650 as a side effect.

I’ve gone with an additional boolean field `_datasetsFetched` rather
than making `_datasets` nullable because `_datasets` is also passed down
directly to `tf-graph-controls`, which expects a non-null array. This
seemed simpler, all things considered.

Test Plan:

 1. Launch TensorBoard. Navigate to <http://localhost:6006/#graphs>.
 2. Observe that there is no flash of “No graph definition files were
    found” (though there is instead a brief flash of white).
 3. Note that the list of graphs loads, the first graph is selected, and
    the fragment updates.
 4. Refresh the page. Note that the page behaves as in the initial load:
    in particular, there is no error modal.
 5. Change the fragment such that the run is invalid (`#graphs&run=bad`)
    and note that the error modal appears without refreshing the page.
 6. Refresh the page, and note that the error modal appears, and the
    first graph is automatically selected and loaded underneath it.

wchargin-branch: graph-fix-error-modal
@wchargin wchargin mentioned this pull request Sep 12, 2019
Merged
@wchargin wchargin merged commit f33f6b8 into master Sep 12, 2019
@wchargin wchargin deleted the wchargin-graph-fix-error-modal branch September 12, 2019 19:12
wchargin added a commit to wchargin/tensorboard that referenced this pull request Sep 12, 2019
@wchargin
graph: fix spurious error modal on page load (tensorflow#2651)
f408546 
Summary:
Fixes tensorflow#2646, and fixes tensorflow#2650 as a side effect.

I’ve gone with an additional boolean field `_datasetsFetched` rather
than making `_datasets` nullable because `_datasets` is also passed down
directly to `tf-graph-controls`, which expects a non-null array. This
seemed simpler, all things considered.

Test Plan:

 1. Launch TensorBoard. Navigate to <http://localhost:6006/#graphs>.
 2. Observe that there is no flash of “No graph definition files were
    found” (though there is instead a brief flash of white).
 3. Note that the list of graphs loads, the first graph is selected, and
    the fragment updates.
 4. Refresh the page. Note that the page behaves as in the initial load:
    in particular, there is no error modal.
 5. Change the fragment such that the run is invalid (`#graphs&run=bad`)
    and note that the error modal appears without refreshing the page.
 6. Refresh the page, and note that the error modal appears, and the
    first graph is automatically selected and loaded underneath it.

wchargin-branch: graph-fix-error-modal
@wchargin wchargin mentioned this pull request Sep 12, 2019
Merged
wchargin added a commit that referenced this pull request Sep 12, 2019
@wchargin
graph: fix spurious error modal on page load (#2651)
f1aa5b8 
Summary:
Fixes #2646, and fixes #2650 as a side effect.

I’ve gone with an additional boolean field `_datasetsFetched` rather
than making `_datasets` nullable because `_datasets` is also passed down
directly to `tf-graph-controls`, which expects a non-null array. This
seemed simpler, all things considered.

Test Plan:

 1. Launch TensorBoard. Navigate to <http://localhost:6006/#graphs>.
 2. Observe that there is no flash of “No graph definition files were
    found” (though there is instead a brief flash of white).
 3. Note that the list of graphs loads, the first graph is selected, and
    the fragment updates.
 4. Refresh the page. Note that the page behaves as in the initial load:
    in particular, there is no error modal.
 5. Change the fragment such that the run is invalid (`#graphs&run=bad`)
    and note that the error modal appears without refreshing the page.
 6. Refresh the page, and note that the error modal appears, and the
    first graph is automatically selected and loaded underneath it.

wchargin-branch: graph-fix-error-modal
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stephanwlee stephanwlee stephanwlee approved these changes

Assignees
No one assigned
Labels
cla: yes plugin:graph type:bug
Projects
None yet
Milestone
No milestone
3 participants
@wchargin @stephanwlee @googlebot