Skip to content

Commit e742a53

Browse files
authored
time series: move the fit button to header (#4856)
The scalar line chart in the time series has had the feature where we automatically fit domain to the new data when it is unaltered. However, because it was hidden inside the overflow menu and because there was no clear visual indication when the chart will fit to the domain or not, it was largely confusing. With the recent changes to make it more salient with `title` and `disabled` on the button, we improved in this area but was still not too obvious. To fix this problem, we are now moving the control out of the overflow menu into the chart header so there is a clearer visual difference between domains overridden vs. not.
1 parent 1501f46 commit e742a53

File tree

2 files changed

+64
-55
lines changed

2 files changed

+64
-55
lines changed

tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,21 @@
2121
value="{{ title }}"
2222
></tb-truncated-path>
2323
<span class="controls">
24+
<button
25+
mat-icon-button
26+
[disabled]="!seriesDataList || !seriesDataList.length || (newLineChart && !newLineChart.getIsViewBoxOverridden())"
27+
(click)="resetDomain()"
28+
[title]="
29+
(newLineChart && !newLineChart.getIsViewBoxOverridden()) ?
30+
'Line chart is already fitted to data. When data updates, the line chart '
31+
+ 'will auto fit to its domain.' :
32+
'Fit line chart domains to data'
33+
"
34+
i18n-aria-label="A button that resets line chart domain to the data"
35+
aria-label="Fit line chart domains to data"
36+
>
37+
<mat-icon svgIcon="settings_overscan_24px"></mat-icon>
38+
</button>
2439
<button
2540
mat-icon-button
2641
class="pin-button"
@@ -64,22 +79,6 @@
6479
<mat-icon svgIcon="line_weight_24px"></mat-icon>
6580
<span>Toggle Y-axis log scale</span>
6681
</button>
67-
<button
68-
mat-menu-item
69-
[disabled]="!seriesDataList || !seriesDataList.length || (newLineChart && !newLineChart.getIsViewBoxOverridden())"
70-
(click)="resetDomain()"
71-
[title]="
72-
(newLineChart && !newLineChart.getIsViewBoxOverridden) ?
73-
'Line chart is already fitted to data. When data updates, the line chart '
74-
+ 'will auto fit to its domain.' :
75-
''
76-
"
77-
i18n-aria-label="A button that resets line chart domain to the data"
78-
aria-label="Fit line chart domains to data"
79-
>
80-
<mat-icon svgIcon="settings_overscan_24px"></mat-icon>
81-
<span>Fit domain to data</span>
82-
</button>
8382
<button
8483
mat-menu-item
8584
(click)="openDataDownloadDialog()"

tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts

Lines changed: 49 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -779,13 +779,16 @@ describe('scalar card', () => {
779779
expect(visibleRunIds).toEqual(['run3']);
780780
}));
781781

782-
describe('overflow menu', () => {
783-
beforeEach(() => {
782+
describe('controls', () => {
783+
const ByCss = {
784+
FIT_TO_DOMAIN: By.css('[aria-label="Fit line chart domains to data"]'),
785+
};
786+
787+
it('resets domain when user clicks on reset button', fakeAsync(() => {
784788
const runToSeries = {
785-
run1: [
786-
{wallTime: 100, value: 1, step: 333},
787-
{wallTime: 101, value: 2, step: 555},
788-
],
789+
run1: [{wallTime: 100, value: 1, step: 333}],
790+
run2: [{wallTime: 100, value: 1, step: 333}],
791+
run3: [{wallTime: 100, value: 1, step: 333}],
789792
};
790793
provideMockCardRunToSeriesData(
791794
selectSpy,
@@ -794,31 +797,6 @@ describe('scalar card', () => {
794797
null /* metadataOverride */,
795798
runToSeries
796799
);
797-
});
798-
799-
it('toggles yAxisType when you click on button in overflow menu', fakeAsync(() => {
800-
const fixture = createComponent('card1');
801-
802-
openOverflowMenu(fixture);
803-
getMenuButton('Toggle Y-axis log scale on line chart').click();
804-
fixture.detectChanges();
805-
806-
const lineChartEl = fixture.debugElement.query(
807-
By.directive(TestableLineChart)
808-
);
809-
expect(lineChartEl.componentInstance.yAxisType).toBe(YAxisType.LOG);
810-
811-
openOverflowMenu(fixture);
812-
getMenuButton('Toggle Y-axis log scale on line chart').click();
813-
fixture.detectChanges();
814-
815-
expect(lineChartEl.componentInstance.yAxisType).toBe(YAxisType.LINEAR);
816-
817-
// Clicking on overflow menu and mat button enqueue asyncs. Flush them.
818-
flush();
819-
}));
820-
821-
it('resets domain when user clicks on reset button', fakeAsync(() => {
822800
const fixture = createComponent('card1');
823801

824802
const lineChartEl = fixture.debugElement.query(
@@ -829,14 +807,10 @@ describe('scalar card', () => {
829807
'resetDomain'
830808
);
831809

832-
openOverflowMenu(fixture);
833-
getMenuButton('Fit line chart domains to data').click();
810+
fixture.debugElement.query(ByCss.FIT_TO_DOMAIN).nativeElement.click();
834811
fixture.detectChanges();
835812

836813
expect(resetDomainSpy).toHaveBeenCalledTimes(1);
837-
838-
// Clicking on overflow menu and mat button enqueue asyncs. Flush them.
839-
flush();
840814
}));
841815

842816
it('disables the resetDomain button when there are no runs', fakeAsync(() => {
@@ -850,11 +824,47 @@ describe('scalar card', () => {
850824
);
851825
const fixture = createComponent('card1');
852826

827+
const button = fixture.debugElement.query(ByCss.FIT_TO_DOMAIN);
828+
expect(button.properties['disabled']).toBe(true);
829+
}));
830+
});
831+
832+
describe('overflow menu', () => {
833+
beforeEach(() => {
834+
const runToSeries = {
835+
run1: [
836+
{wallTime: 100, value: 1, step: 333},
837+
{wallTime: 101, value: 2, step: 555},
838+
],
839+
};
840+
provideMockCardRunToSeriesData(
841+
selectSpy,
842+
PluginType.SCALARS,
843+
'card1',
844+
null /* metadataOverride */,
845+
runToSeries
846+
);
847+
});
848+
849+
it('toggles yAxisType when you click on button in overflow menu', fakeAsync(() => {
850+
const fixture = createComponent('card1');
851+
852+
openOverflowMenu(fixture);
853+
getMenuButton('Toggle Y-axis log scale on line chart').click();
854+
fixture.detectChanges();
855+
856+
const lineChartEl = fixture.debugElement.query(
857+
By.directive(TestableLineChart)
858+
);
859+
expect(lineChartEl.componentInstance.yAxisType).toBe(YAxisType.LOG);
860+
853861
openOverflowMenu(fixture);
854-
const button = getMenuButton('Fit line chart domains to data');
855-
expect(button.disabled).toBe(true);
862+
getMenuButton('Toggle Y-axis log scale on line chart').click();
863+
fixture.detectChanges();
864+
865+
expect(lineChartEl.componentInstance.yAxisType).toBe(YAxisType.LINEAR);
856866

857-
// Clicking on overflow menu enqueues async.
867+
// Clicking on overflow menu and mat button enqueue asyncs. Flush them.
858868
flush();
859869
}));
860870
});

0 commit comments

Comments
 (0)