Skip to content

Commit 56c51b0

Browse files
HParams: project color header (#6434)
## Motivation for features / changes In #6422 we started projecting the header components into the data table widget. However, the content was still not projected and the color column was hard coded in. Therefore we left a hard coded color header. The content started being projected in #6427. Therefore we can stop hardcoding this color column. This PR removes the hardcoded header column and starts adding it from the ScalarCardDataTable ## Technical description of changes The color header cannot be sorted, dismissed, or dragged. There was some logic in place for this. However, it was incomplete. This completes that logic and uses it for the color header. We also add tests for that logic which were not in place before. Note: I did some test clean up in the HeaderCell ## Screenshot of changes It still works. <img width="148" alt="Screenshot 2023-06-15 at 5 41 33 AM" src="https://github.com/tensorflow/tensorboard/assets/8672809/0fe6e62e-f79a-46a8-bf60-cad3d87a28d4"> ComponentTests for consistency. --------- Co-authored-by: Riley Jones <[email protected]>
1 parent 0848088 commit 56c51b0

File tree

6 files changed

+100
-21
lines changed

6 files changed

+100
-21
lines changed

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,13 @@
2424
(removeColumn)="removeColumn.emit($event)"
2525
>
2626
<ng-container header>
27-
<ng-container *ngFor="let header of columnHeaders">
27+
<ng-container *ngFor="let header of getHeaders()">
2828
<tb-data-table-header-cell
2929
*ngIf="header.enabled && (header.type !== ColumnHeaderType.SMOOTHED || smoothingEnabled)"
3030
[header]="header"
3131
[sortingInfo]="sortingInfo"
3232
[hparamsEnabled]="hparamsEnabled"
33+
[controlsEnabled]="header.type !== ColumnHeaderType.COLOR"
3334
></tb-data-table-header-cell> </ng-container
3435
></ng-container>
3536

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

+6-3
Original file line numberDiff line numberDiff line change
@@ -2649,7 +2649,7 @@ describe('scalar card', () => {
26492649
expect(dataTableComponent).toBeFalsy();
26502650
}));
26512651

2652-
it('projects tb-data-table-header-cell for enabled headers', fakeAsync(() => {
2652+
it('projects tb-data-table-header-cell for color and enabled headers', fakeAsync(() => {
26532653
store.overrideSelector(getMetricsLinkedTimeSelection, {
26542654
start: {step: 20},
26552655
end: null,
@@ -2681,13 +2681,16 @@ describe('scalar card', () => {
26812681
By.directive(DataTableComponent)
26822682
).componentInstance;
26832683

2684-
expect(dataTableComponentInstance.headerCells.length).toEqual(2);
2684+
expect(dataTableComponentInstance.headerCells.length).toEqual(3);
26852685

26862686
expect(
26872687
dataTableComponentInstance.headerCells.get(0).header.name
2688-
).toEqual('run');
2688+
).toEqual('color');
26892689
expect(
26902690
dataTableComponentInstance.headerCells.get(1).header.name
2691+
).toEqual('run');
2692+
expect(
2693+
dataTableComponentInstance.headerCells.get(2).header.name
26912694
).toEqual('step');
26922695
}));
26932696

tensorboard/webapp/widgets/data_table/data_table_component.ng.html

-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414

1515
<div class="data-table">
1616
<div class="header">
17-
<div class="col"></div>
1817
<ng-content select="[header]"></ng-content>
1918
</div>
2019
<ng-content select="[content]"></ng-content>

tensorboard/webapp/widgets/data_table/header_cell_component.ng.html

+3-3
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,15 @@
1414

1515
<div
1616
class="cell"
17-
[draggable]="sortable"
17+
[draggable]="controlsEnabled"
1818
(dragstart)="dragStart.emit(header)"
1919
(dragend)="dragEnd.emit()"
2020
(dragenter)="dragEnter.emit(header)"
2121
(click)="headerClicked.emit(header.name)"
2222
[ngClass]="highlightStyle$ | async"
2323
>
2424
<ng-content></ng-content>
25-
<div *ngIf="hparamsEnabled" class="delete-icon-container">
25+
<div *ngIf="hparamsEnabled && controlsEnabled" class="delete-icon-container">
2626
<button
2727
class="delete-icon"
2828
mat-icon-button
@@ -34,7 +34,7 @@
3434
</button>
3535
</div>
3636
<tb-data-table-header [header]="header"></tb-data-table-header>
37-
<div *ngIf="sortable" class="sorting-icon-container">
37+
<div *ngIf="controlsEnabled" class="sorting-icon-container">
3838
<mat-icon
3939
*ngIf="
4040
sortingInfo.order === SortingOrder.ASCENDING ||

tensorboard/webapp/widgets/data_table/header_cell_component.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import {BehaviorSubject} from 'rxjs';
3737
export class HeaderCellComponent {
3838
@Input() header!: ColumnHeader;
3939
@Input() sortingInfo!: SortingInfo;
40-
@Input() sortable: boolean = true;
40+
@Input() controlsEnabled: boolean = true;
4141
@Input() hparamsEnabled?: boolean = false;
4242

4343
@Output() dragStart = new EventEmitter<ColumnHeader>();

tensorboard/webapp/widgets/data_table/header_cell_component_test.ts

+88-12
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {HeaderCellComponent} from './header_cell_component';
3333
[header]="header"
3434
[sortingInfo]="sortingInfo"
3535
[hparamsEnabled]="hparamsEnabled"
36+
[controlsEnabled]="controlsEnabled"
3637
(headerClicked)="headerClicked($event)"
3738
(deleteButtonClicked)="deleteButtonClicked($event)"
3839
></tb-data-table-header-cell>
@@ -44,7 +45,8 @@ class TestableComponent {
4445

4546
@Input() header!: ColumnHeader;
4647
@Input() sortingInfo!: SortingInfo;
47-
@Input() hparamsEnabled?: boolean;
48+
@Input() hparamsEnabled!: boolean;
49+
@Input() controlsEnabled!: boolean;
4850

4951
@Input() headerClicked!: (sortingInfo: SortingInfo) => void;
5052
@Input() deleteButtonClicked!: (header: ColumnHeader) => void;
@@ -63,6 +65,8 @@ describe('header cell', () => {
6365
function createComponent(input: {
6466
header?: ColumnHeader;
6567
sortingInfo?: SortingInfo;
68+
hparamsEnabled?: boolean;
69+
controlsEnabled?: boolean;
6670
}): ComponentFixture<TestableComponent> {
6771
const fixture = TestBed.createComponent(TestableComponent);
6872

@@ -76,26 +80,27 @@ describe('header cell', () => {
7680
name: 'run',
7781
order: SortingOrder.ASCENDING,
7882
};
83+
fixture.componentInstance.hparamsEnabled = input.hparamsEnabled ?? true;
84+
fixture.componentInstance.controlsEnabled = input.controlsEnabled ?? true;
7985

8086
headerClickedSpy = jasmine.createSpy();
8187
fixture.componentInstance.headerClicked = headerClickedSpy;
8288

8389
deleteButtonClickedSpy = jasmine.createSpy();
8490
fixture.componentInstance.deleteButtonClicked = deleteButtonClickedSpy;
8591

92+
fixture.detectChanges();
8693
return fixture;
8794
}
8895

8996
it('renders', () => {
9097
const fixture = createComponent({});
91-
fixture.detectChanges();
9298
const cell = fixture.debugElement.query(By.css('.cell'));
9399
expect(cell).toBeTruthy();
94100
});
95101

96102
it('emits headerClicked event when cell element is clicked', () => {
97103
const fixture = createComponent({});
98-
fixture.detectChanges();
99104
fixture.debugElement
100105
.query(By.directive(HeaderCellComponent))
101106
.componentInstance.headerClicked.subscribe();
@@ -104,14 +109,8 @@ describe('header cell', () => {
104109
});
105110

106111
describe('delete column button', () => {
107-
let fixture: ComponentFixture<TestableComponent>;
108-
beforeEach(() => {
109-
fixture = createComponent({});
110-
fixture.componentInstance.hparamsEnabled = true;
111-
fixture.detectChanges();
112-
});
113-
114112
it('emits removeColumn event when delete button clicked', () => {
113+
const fixture = createComponent({hparamsEnabled: true});
115114
fixture.debugElement
116115
.query(By.directive(HeaderCellComponent))
117116
.componentInstance.deleteButtonClicked.subscribe();
@@ -128,14 +127,91 @@ describe('header cell', () => {
128127
});
129128

130129
it('renders delete button when hparamsEnabled is true', () => {
130+
const fixture = createComponent({hparamsEnabled: true});
131+
131132
expect(fixture.debugElement.query(By.css('.delete-icon'))).toBeTruthy();
132133
});
133134

134135
it('does not render delete button when hparamsEnabled is false', () => {
135-
fixture.componentInstance.hparamsEnabled = false;
136-
fixture.detectChanges();
136+
const fixture = createComponent({hparamsEnabled: false});
137137

138138
expect(fixture.debugElement.query(By.css('.delete-icon'))).toBeFalsy();
139139
});
140+
141+
it('does not render delete button when controlsEnabled is false', () => {
142+
const fixture = createComponent({controlsEnabled: false});
143+
144+
expect(fixture.debugElement.query(By.css('.delete-icon'))).toBeFalsy();
145+
});
146+
});
147+
148+
describe('sorting icon', () => {
149+
it('shows sorting icon when sortingInfo matches header', () => {
150+
const fixture = createComponent({
151+
sortingInfo: {
152+
name: 'run',
153+
order: SortingOrder.ASCENDING,
154+
},
155+
});
156+
157+
expect(
158+
fixture.debugElement
159+
.query(By.css('.sorting-icon-container mat-icon'))
160+
.nativeElement.classList.contains('show')
161+
).toBe(true);
162+
});
163+
164+
it('does not render sorting icon when sortingInfo does not match header', () => {
165+
const fixture = createComponent({
166+
sortingInfo: {
167+
name: 'not-this-header',
168+
order: SortingOrder.ASCENDING,
169+
},
170+
});
171+
172+
expect(
173+
fixture.debugElement
174+
.query(By.css('.sorting-icon-container mat-icon'))
175+
.nativeElement.classList.contains('show')
176+
).toBe(false);
177+
});
178+
179+
it('renders downward arrow if order is DESCENDING', () => {
180+
const fixture = createComponent({
181+
sortingInfo: {
182+
name: 'run',
183+
order: SortingOrder.DESCENDING,
184+
},
185+
});
186+
187+
expect(
188+
fixture.debugElement
189+
.query(By.css('.sorting-icon-container mat-icon'))
190+
.nativeElement.getAttribute('svgIcon')
191+
).toBe('arrow_downward_24px');
192+
});
193+
194+
it('renders downward arrow if order is DESCENDING', () => {
195+
const fixture = createComponent({
196+
sortingInfo: {
197+
name: 'run',
198+
order: SortingOrder.ASCENDING,
199+
},
200+
});
201+
202+
expect(
203+
fixture.debugElement
204+
.query(By.css('.sorting-icon-container mat-icon'))
205+
.nativeElement.getAttribute('svgIcon')
206+
).toBe('arrow_upward_24px');
207+
});
208+
209+
it('does not render sorting icon when controlsEnabled is false', () => {
210+
const fixture = createComponent({controlsEnabled: false});
211+
212+
expect(
213+
fixture.debugElement.query(By.css('.sorting-icon-container mat-icon'))
214+
).toBeFalsy();
215+
});
140216
});
141217
});

0 commit comments

Comments
 (0)