Closed chihuahua closed 6 years ago
@wchargin, we could modify this logic to attach and detach dashboards to/from the DOM. https://github.com/tensorflow/tensorboard/blob/c52bd79c7d99a306cadb5c203810cb10f3660c42/tensorboard/components/tf_tensorboard/tf-tensorboard.html#L202
Then, dashboards could respond to being selected or unselected via whether handling attachment. What do you think?
An alternative solution: Make tf-tensorboard
set isSelected
on individual dashboards.
Come to think I prefer the alternative of setting isSelected
. @dandelionmane noted that we've switched back and forth between using dom-if
vs hiding dashboards.
Setting isSelected
would decouple dashboard selection with DOM attachment, which I think is clearer to developers. Plugins would just observe changes to isSelected
. Recognition over recall.
It is worth keeping in mind that the root cause of this issue and related issues is that Plottable’s drawing code relies extensively on interacting with the DOM: measuring, tweaking, remeasuring, etc. If we render a Plottable chart when the component is not actually shown on screen, the measurements that it gets will not be valid (e.g., will be all zeros), and this manifests as “squished plots” and the like. To combat this, we try our best—at least in the scalars dashboard—to only update Plottable charts when they’re in view: we’ve added integration points in a number of places, such as adapting tf-collapsable-pane
to report which of its pages is active, to compute an approximation of the visibility function. But this is still an approximation, and while adding something like isSelected
will offer the opportunity for one hole to be plugged, it is not a robust solution, and does not solve the root problem. Note also that I say “offer the opportunity” to plug this hole: every plugin will still have to include integration code to inspect/observe the isSelected
property and change its rendering behavior appropriately, which is not great.
It would be wonderful if we could address the underlying problem instead of bloating all plugins with patching code. I hear that @dandelionmane knows something about Plottable; do you have any thoughts on the matter? (I don’t really know what a solution would look like.)
Yeah, @dandelionmane, any way we could fix this within Plottable? That would help others too.
Visit the scalars dashboard. Navigate to the image dashboard, and toggle any run. Navigate back to the scalars dashboard. Note the squished line charts.
Part of why this happens is that TensorBoard includes all active dashboards in the DOM. All are hidden except for the selected dashboard. https://github.com/tensorflow/tensorboard/blob/c52bd79c7d99a306cadb5c203810cb10f3660c42/tensorboard/components/tf_tensorboard/tf-tensorboard.html#L202
Hence, the
attach
handler for each dashboard is not a proxy for whether the dashboard is selected. In general, there is no systematic way for TensorBoard to inform each dashboard of whether it is currently selected.