Skip to content

Commit 01638cb

Browse files
authored
ng: fix tiny charts (tensorflow#3442)
There are many instances when vz_line_chart can be waiting to be redrawn: it can wait for debouncing, requestAnimationFrame, and/or data. One of the way Polymer based TensorBoard solved this asynchronousity is by "visibility: hidden"ing the plugin container instead of "display: none". This has an effect of giving the container non-zero width and allows the vz_line_chart to properly render even in the background. This change replicates https://github.com/tensorflow/tensorboard/blob/34530f4789647a81687cc2983866820096b831b1/tensorboard/components/tf_tensorboard/tf-tensorboard.html#L412-L418.
1 parent 34530f4 commit 01638cb

File tree

2 files changed

+59
-7
lines changed

2 files changed

+59
-7
lines changed

tensorboard/webapp/plugins/plugins_component.ts

+23-3
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,12 @@ import {PluginRegistryModule} from './plugin_registry_module';
4141
selector: 'plugins-component',
4242
templateUrl: './plugins_component.ng.html',
4343
styles: [
44-
'.plugins { height: 100%; }',
44+
`
45+
.plugins {
46+
height: 100%;
47+
position: relative;
48+
}
49+
`,
4550
'iframe { border: 0; height: 100%; width: 100%; }',
4651
],
4752
changeDetection: ChangeDetectionStrategy.OnPush,
@@ -79,12 +84,27 @@ export class PluginsComponent implements OnChanges {
7984

8085
private renderPlugin(plugin: UiPluginMetadata) {
8186
for (const element of this.pluginInstances.values()) {
82-
element.style.display = 'none';
87+
Object.assign(element.style, {
88+
maxHeight: 0,
89+
overflow: 'hidden',
90+
/**
91+
* We further make containers invisible. Some elements may anchor to
92+
* the viewport instead of the container, in which case setting the max
93+
* height here to 0 will not hide them.
94+
**/
95+
visibility: 'hidden',
96+
position: 'absolute',
97+
});
8398
}
8499

85100
if (this.pluginInstances.has(plugin.id)) {
86101
const instance = this.pluginInstances.get(plugin.id) as HTMLElement;
87-
instance.style.removeProperty('display');
102+
Object.assign(instance.style, {
103+
maxHeight: null,
104+
overflow: null,
105+
visibility: null,
106+
position: null,
107+
});
88108
return;
89109
}
90110

tensorboard/webapp/plugins/plugins_container_test.ts

+36-4
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,19 @@ function expectPluginIframe(element: HTMLElement, name: string) {
4747
);
4848
}
4949

50+
class TestableCustomElement extends HTMLElement {
51+
constructor() {
52+
super();
53+
54+
const shadow = this.attachShadow({mode: 'open'});
55+
const wrapper = document.createElement('div');
56+
wrapper.textContent = 'Test TensorBoard';
57+
shadow.appendChild(wrapper);
58+
}
59+
}
60+
61+
customElements.define('tb-bar', TestableCustomElement);
62+
5063
describe('plugins_component', () => {
5164
let store: MockStore<State>;
5265
const PLUGINS = {
@@ -163,9 +176,9 @@ describe('plugins_component', () => {
163176
expect(nativeElement.childElementCount).toBe(2);
164177
const [fooElement, barElement] = nativeElement.children;
165178
expectPluginIframe(fooElement, 'foo');
166-
expect(fooElement.style.display).toBe('none');
179+
expect(fooElement.style.visibility).toBe('hidden');
167180
expect(barElement.tagName).toBe('TB-BAR');
168-
expect(barElement.style.display).not.toBe('none');
181+
expect(barElement.style.visibility).not.toBe('hidden');
169182
});
170183

171184
it('does not create same instance of plugin', async () => {
@@ -189,9 +202,9 @@ describe('plugins_component', () => {
189202

190203
const {nativeElement} = fixture.debugElement.query(By.css('.plugins'));
191204
expect(nativeElement.childElementCount).toBe(2);
192-
const [fooElement, barElement] = nativeElement.children;
205+
const [fooElement] = nativeElement.children;
193206
expectPluginIframe(fooElement, 'foo');
194-
expect(fooElement.style.display).not.toBe('none');
207+
expect(fooElement.style.visibility).not.toBe('hidden');
195208
});
196209

197210
it('creates components for plugins registered dynamically', async () => {
@@ -208,6 +221,25 @@ describe('plugins_component', () => {
208221
const pluginElement = nativeElement.children[0];
209222
expect(pluginElement.tagName).toBe('EXTRA-DASHBOARD');
210223
});
224+
225+
it('hides inactive plugin but keeps their width', async () => {
226+
setActivePlugin('bar');
227+
228+
const fixture = TestBed.createComponent(PluginsContainer);
229+
fixture.detectChanges();
230+
231+
setActivePlugin('foo');
232+
fixture.detectChanges();
233+
234+
const {nativeElement} = fixture.debugElement.query(By.css('.plugins'));
235+
const [barElement] = nativeElement.children;
236+
expect(barElement.shadowRoot.firstElementChild.textContent).toBe(
237+
'Test TensorBoard'
238+
);
239+
expect(
240+
barElement.shadowRoot.firstElementChild.clientWidth
241+
).toBeGreaterThan(0);
242+
});
211243
});
212244

213245
describe('updates', () => {

0 commit comments

Comments
 (0)