Skip to content

Commit 40b9bda

Browse files
HParams: Ignore key presses in ColumnSelector when not active (#6484)
## Motivation for features / changes This fixes a bug. These key presses are subscribed to at all times. Therefore clicking enter anytime would add an HParam to the runs table. ## Technical description of changes I simply added a boolean isActive to track the state of the ColumnSelector. If it is not active I ignore the key presses. --------- Co-authored-by: Riley Jones <[email protected]>
1 parent 21b6bc6 commit 40b9bda

File tree

3 files changed

+52
-0
lines changed

3 files changed

+52
-0
lines changed

tensorboard/webapp/widgets/data_table/column_selector_component.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ export class ColumnSelectorComponent implements OnInit, AfterViewInit {
4545

4646
searchInput = '';
4747
selectedIndex$ = new BehaviorSubject(0);
48+
private isActive = false;
4849

4950
ngOnInit() {
5051
/**
@@ -121,13 +122,23 @@ export class ColumnSelectorComponent implements OnInit, AfterViewInit {
121122
this.columnSelected.emit(header);
122123
}
123124

125+
activate() {
126+
this.isActive = true;
127+
}
128+
129+
deactivate() {
130+
this.isActive = false;
131+
}
132+
124133
@HostListener('document:keydown.arrowup', ['$event'])
125134
onUpArrow() {
135+
if (!this.isActive) return;
126136
this.selectedIndex$.next(Math.max(this.selectedIndex$.getValue() - 1, 0));
127137
}
128138

129139
@HostListener('document:keydown.arrowdown', ['$event'])
130140
onDownArrow() {
141+
if (!this.isActive) return;
131142
this.selectedIndex$.next(
132143
Math.min(
133144
this.selectedIndex$.getValue() + 1,
@@ -138,6 +149,7 @@ export class ColumnSelectorComponent implements OnInit, AfterViewInit {
138149

139150
@HostListener('document:keydown.enter', ['$event'])
140151
onEnterPressed() {
152+
if (!this.isActive) return;
141153
this.selectColumn(
142154
this.getFilteredColumns()[this.selectedIndex$.getValue()]
143155
);

tensorboard/webapp/widgets/data_table/column_selector_test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ describe('column selector', () => {
3030
if (props.selectableColumns) {
3131
fixture.componentInstance.selectableColumns = props.selectableColumns;
3232
}
33+
fixture.componentInstance.activate();
34+
3335
fixture.detectChanges();
3436

3537
return fixture;
@@ -77,6 +79,22 @@ describe('column selector', () => {
7779
expect(getSelectedButton().nativeElement.innerText).toEqual('Runs');
7880
});
7981

82+
it('does not change selected index on up arrow press when deactivated', () => {
83+
fixture.componentInstance.selectedIndex$.next(1);
84+
fixture.componentInstance.deactivate();
85+
fixture.detectChanges();
86+
expect(getSelectedButton().nativeElement.innerText).toEqual(
87+
'Learning Rate'
88+
);
89+
90+
const event = new KeyboardEvent('keydown', {key: 'arrowup'});
91+
document.dispatchEvent(event);
92+
fixture.detectChanges();
93+
expect(getSelectedButton().nativeElement.innerText).toEqual(
94+
'Learning Rate'
95+
);
96+
});
97+
8098
it('increases selected index when the down arrow is pressed', () => {
8199
expect(getSelectedButton().nativeElement.innerText).toEqual('Runs');
82100
const event = new KeyboardEvent('keydown', {key: 'arrowdown'});
@@ -87,6 +105,15 @@ describe('column selector', () => {
87105
);
88106
});
89107

108+
it('does not change selected index when the down arrow is pressed while deactivated', () => {
109+
expect(getSelectedButton().nativeElement.innerText).toEqual('Runs');
110+
fixture.componentInstance.deactivate();
111+
const event = new KeyboardEvent('keydown', {key: 'arrowdown'});
112+
document.dispatchEvent(event);
113+
fixture.detectChanges();
114+
expect(getSelectedButton().nativeElement.innerText).toEqual('Runs');
115+
});
116+
90117
it('does not change index when columns are selected', () => {
91118
fixture.componentInstance.selectedIndex$.next(1);
92119
fixture.detectChanges();
@@ -114,6 +141,17 @@ describe('column selector', () => {
114141
expect(selectedColumn!.name).toEqual('runs');
115142
}));
116143

144+
it('does not select a column when the enter key is pressed while deactivated', fakeAsync(() => {
145+
fixture.componentInstance.columnSelected.subscribe(() => {
146+
fail('should not be called');
147+
});
148+
fixture.componentInstance.deactivate();
149+
150+
const event = new KeyboardEvent('keydown', {key: 'enter'});
151+
document.dispatchEvent(event);
152+
flush();
153+
}));
154+
117155
it('cannot select indexes below 0', () => {
118156
expect(getSelectedButton().nativeElement.innerText).toEqual('Runs');
119157
const event = new KeyboardEvent('keydown', {key: 'arrowup'});

tensorboard/webapp/widgets/data_table/data_table_component.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,11 +256,13 @@ export class DataTableComponent implements OnDestroy, AfterContentInit {
256256
x: rect.x + rect.width,
257257
y: rect.y + rect.height,
258258
});
259+
this.columnSelector.activate();
259260
}
260261

261262
onColumnSelectorClosed() {
262263
this.contextMenuHeader = undefined;
263264
this.insertColumnTo = undefined;
265+
this.columnSelector.deactivate();
264266
}
265267

266268
canContextMenuRemoveColumn() {

0 commit comments

Comments
 (0)