Skip to content

Commit f3872e6

Browse files
authored
HParams: Implement selection checkbox double click (#6481)
## Motivation for features / changes We will soon be replacing the RunsTableComponent with the RunsDataTable. In an effort to reach feature parity with the current runs table this PR implements the dblClick functionality of the selected check boxes. When the checkboxes are double clicked it has the effect of selecting this and only this run. ## Technical description of changes I implemented the dblclick logic differently than the RunsTableComponent. I utilized the MouseEvent details property to decide if a click was a click or a dblclick. This stops the click from firing both the onSelectionToggle and the onSelectionDblClick events for a single click. ## Screenshots of UI changes (or N/A) ![2023-07-05_16-57-38 (3)](https://github.com/tensorflow/tensorboard/assets/8672809/ea29d54e-5bb4-437d-9cf5-a09142f691fe)
1 parent 1d135f9 commit f3872e6

File tree

6 files changed

+57
-10
lines changed

6 files changed

+57
-10
lines changed

tensorboard/webapp/runs/views/runs_table/runs_data_table.ng.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@
8383
<div *ngSwitchCase="'selected'">
8484
<mat-checkbox
8585
[checked]="dataRow['selected']"
86-
(change)="onSelectionToggle.emit(dataRow.id)"
86+
(click)="selectionClick($event, dataRow['id'])"
8787
></mat-checkbox>
8888
</div>
8989
</ng-container>

tensorboard/webapp/runs/views/runs_table/runs_data_table.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ export class RunsDataTable {
5858
index?: number | undefined;
5959
}>();
6060
@Output() removeColumn = new EventEmitter<ColumnHeader>();
61+
@Output() onSelectionDblClick = new EventEmitter<string>();
6162

6263
// These columns must be stored and reused to stop needless re-rendering of
6364
// the content and headers in these columns. This has been known to cause
@@ -88,6 +89,22 @@ export class RunsDataTable {
8889
);
8990
}
9091

92+
selectionClick(event: MouseEvent, runId: string) {
93+
// Prevent checkbox from switching checked state on its own.
94+
event.preventDefault();
95+
96+
// event.details on mouse click events gives the number of clicks in quick
97+
// succession. This logic is used to differentiate between single and double
98+
// clicks.
99+
// Note: This means any successive click after the second are noops.
100+
if (event.detail === 1) {
101+
this.onSelectionToggle.emit(runId);
102+
}
103+
if (event.detail === 2) {
104+
this.onSelectionDblClick.emit(runId);
105+
}
106+
}
107+
91108
allRowsSelected() {
92109
return this.data.every((row) => row['selected']);
93110
}

tensorboard/webapp/runs/views/runs_table/runs_data_table_test.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import {sendKeys} from '../../../testing/dom';
4545
(onSelectionToggle)="onSelectionToggle($event)"
4646
(onAllSelectionToggle)="onAllSelectionToggle($event)"
4747
(onRegexFilterChange)="onRegexFilterChange($event)"
48+
(onSelectionDblClick)="onSelectionDblClick($event)"
4849
></runs-data-table>
4950
`,
5051
})
@@ -59,11 +60,13 @@ class TestableComponent {
5960
@Input() onSelectionToggle!: (runId: string) => void;
6061
@Input() onAllSelectionToggle!: (runIds: string[]) => void;
6162
@Input() onRegexFilterChange!: (regex: string) => void;
63+
@Input() onSelectionDblClick!: (runId: string) => void;
6264
}
6365

6466
describe('runs_data_table', () => {
6567
let onSelectionToggleSpy: jasmine.Spy;
6668
let onAllSelectionToggleSpy: jasmine.Spy;
69+
let onSelectionDblClickSpy: jasmine.Spy;
6770
let onRegexFilterChangeSpy: jasmine.Spy;
6871
function createComponent(input: {
6972
data?: TableData[];
@@ -107,6 +110,9 @@ describe('runs_data_table', () => {
107110
onAllSelectionToggleSpy = jasmine.createSpy();
108111
fixture.componentInstance.onAllSelectionToggle = onAllSelectionToggleSpy;
109112

113+
onSelectionDblClickSpy = jasmine.createSpy();
114+
fixture.componentInstance.onSelectionDblClick = onSelectionDblClickSpy;
115+
110116
onRegexFilterChangeSpy = jasmine.createSpy();
111117
fixture.componentInstance.onRegexFilterChange = onRegexFilterChangeSpy;
112118

@@ -284,11 +290,33 @@ describe('runs_data_table', () => {
284290

285291
const firstCheckbox = selectedContentCells[0].query(By.css('mat-checkbox'));
286292

287-
firstCheckbox.nativeElement.dispatchEvent(new Event('change'));
293+
firstCheckbox.nativeElement.dispatchEvent(
294+
new MouseEvent('click', {detail: 1})
295+
);
288296

289297
expect(onSelectionToggleSpy).toHaveBeenCalledWith('runid');
290298
});
291299

300+
it('emits onSelectionDblClick event when selected header checkbox is double clicked', () => {
301+
const fixture = createComponent({});
302+
303+
const dataTable = fixture.debugElement.query(
304+
By.directive(DataTableComponent)
305+
);
306+
const cells = dataTable.queryAll(By.directive(ContentCellComponent));
307+
308+
const selectedContentCells = cells.filter((cell) => {
309+
return cell.componentInstance.header.name === 'selected';
310+
});
311+
312+
const firstCheckbox = selectedContentCells[0].query(By.css('mat-checkbox'));
313+
firstCheckbox.nativeElement.dispatchEvent(
314+
new MouseEvent('click', {detail: 2})
315+
);
316+
317+
expect(onSelectionDblClickSpy).toHaveBeenCalledWith('runid');
318+
});
319+
292320
it('fire onRegexFilterChange when input is entered into the tb-filter-input', () => {
293321
const fixture = createComponent({});
294322
const filterInput = fixture.debugElement.query(By.css('tb-filter-input'));

tensorboard/webapp/runs/views/runs_table/runs_table_component.ng.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@
237237
<mat-checkbox
238238
[checked]="item.selected"
239239
(change)="onSelectionToggle.emit(item)"
240-
(dblclick)="onSelectionDblClick.emit(item)"
240+
(dblclick)="onSelectionDblClick.emit(item.run.id)"
241241
title="Click to toggle run selection or double click to select only this run."
242242
></mat-checkbox>
243243
</span>

tensorboard/webapp/runs/views/runs_table/runs_table_component.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ export class RunsTableComponent implements OnChanges {
104104

105105
@Output() onRegexFilterChange = new EventEmitter<string>();
106106
@Output() onSelectionToggle = new EventEmitter<RunTableItem>();
107-
@Output() onSelectionDblClick = new EventEmitter<RunTableItem>();
107+
@Output() onSelectionDblClick = new EventEmitter<string>();
108108
@Output() onPageSelectionToggle = new EventEmitter<{items: RunTableItem[]}>();
109109
@Output()
110110
onPaginationChange = new EventEmitter<{

tensorboard/webapp/runs/views/runs_table/runs_table_container.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,7 @@ function matchFilter(
281281
(onAllSelectionToggle)="onAllSelectionToggle($event)"
282282
(onRunColorChange)="onRunColorChange($event)"
283283
(onRegexFilterChange)="onRegexFilterChange($event)"
284+
(onSelectionDblClick)="onRunSelectionDblClick($event)"
284285
(toggleFullScreen)="toggleFullScreen()"
285286
(addColumn)="addColumn($event)"
286287
(removeColumn)="removeColumn($event)"
@@ -694,11 +695,12 @@ export class RunsTableContainer implements OnInit, OnDestroy {
694695
);
695696
}
696697

697-
onRunSelectionDblClick(item: RunTableItem) {
698-
// Note that a user's double click will trigger both 'change' and 'dblclick'
699-
// events so onRunSelectionToggle() will also be called and we will fire
700-
// two somewhat conflicting actions: runSelectionToggled and
701-
// singleRunSelected. This is ok as long as singleRunSelected is fired last.
698+
onRunSelectionDblClick(runId: string) {
699+
// Note that a user's double click in the Legacy RunsTableComponent will
700+
// trigger both 'change' and 'dblclick' events so onRunSelectionToggle()
701+
// will also be called and we will fire two somewhat conflicting actions:
702+
// runSelectionToggled and singleRunSelected. This is ok as long as
703+
// singleRunSelected is fired last.
702704
//
703705
// We are therefore relying on the mat-checkbox 'change' event consistently
704706
// being fired before the 'dblclick' event. Although we don't have any
@@ -709,7 +711,7 @@ export class RunsTableContainer implements OnInit, OnDestroy {
709711
// event.
710712
this.store.dispatch(
711713
singleRunSelected({
712-
runId: item.run.id,
714+
runId,
713715
})
714716
);
715717
}

0 commit comments

Comments
 (0)