Skip to content

Commit 6169001

Browse files
authored
HParams: Fix bug where context menu stays open when another modal is opened (#6504)
## Motivation for features / changes There was a bug where opening another modal (such as the column adder using the + button) the context menu would not close. This happened because of event propagation is disabled to allow nested context menus. ## Screenshots of UI changes (or N/A) Before ![2f4c9735-0bc4-4356-97a2-49e3e85149a7](https://github.com/tensorflow/tensorboard/assets/78179109/61ae353e-96a0-424f-99f0-a46806b7cd0e) After ![a95c50dd-c33f-4cd4-a8a1-fdde88885391](https://github.com/tensorflow/tensorboard/assets/78179109/7b3c5ba9-c62d-4781-b949-5908efe6cdbc)
1 parent 1eea176 commit 6169001

File tree

3 files changed

+37
-13
lines changed

3 files changed

+37
-13
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
limitations under the License.
1313
-->
1414

15-
<custom-modal #contextMenu>
15+
<custom-modal #contextMenu (onClose)="onContextMenuClosed()">
1616
<div class="context-menu">
1717
<div
1818
*ngIf="!contextMenuHeader?.removable && !contextMenuHeader?.sortable && !canContextMenuInsert()"
@@ -50,15 +50,15 @@
5050
mat-button
5151
*ngIf="canContextMenuInsert()"
5252
class="context-menu-button"
53-
(click)="openColumnSelector($event, Side.LEFT)"
53+
(click)="openColumnSelector($event, {insertTo: Side.LEFT, isSubMenu: true})"
5454
>
5555
<mat-icon svgIcon="add_24px"></mat-icon>Insert Column Left
5656
</button>
5757
<button
5858
mat-button
5959
*ngIf="canContextMenuInsert()"
6060
class="context-menu-button"
61-
(click)="openColumnSelector($event, Side.RIGHT)"
61+
(click)="openColumnSelector($event, {insertTo: Side.RIGHT, isSubMenu: true})"
6262
>
6363
<mat-icon svgIcon="add_24px"></mat-icon>Insert Column Right
6464
</button>

tensorboard/webapp/widgets/data_table/data_table_component.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,7 @@ export class DataTableComponent implements OnDestroy, AfterContentInit {
240240
openContextMenu(header: ColumnHeader, event: MouseEvent) {
241241
event.stopPropagation();
242242
event.preventDefault();
243+
this.columnSelectorModal.close();
243244

244245
this.contextMenuHeader = header;
245246
this.contextMenu.openAtPosition({
@@ -248,9 +249,18 @@ export class DataTableComponent implements OnDestroy, AfterContentInit {
248249
});
249250
}
250251

251-
openColumnSelector(event: MouseEvent, insertTo?: Side) {
252-
event.stopPropagation();
253-
this.insertColumnTo = insertTo;
252+
onContextMenuClosed() {
253+
this.contextMenuHeader = undefined;
254+
}
255+
256+
openColumnSelector(
257+
event: MouseEvent,
258+
options?: {insertTo?: Side; isSubMenu?: boolean}
259+
) {
260+
if (options?.isSubMenu) {
261+
event.stopPropagation();
262+
}
263+
this.insertColumnTo = options?.insertTo;
254264
const rect = (event.target as HTMLElement).getBoundingClientRect();
255265
this.columnSelectorModal.openAtPosition({
256266
x: rect.x + rect.width,
@@ -260,7 +270,6 @@ export class DataTableComponent implements OnDestroy, AfterContentInit {
260270
}
261271

262272
onColumnSelectorClosed() {
263-
this.contextMenuHeader = undefined;
264273
this.insertColumnTo = undefined;
265274
this.columnSelector.deactivate();
266275
}

tensorboard/webapp/widgets/data_table/data_table_test.ts

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,7 @@ limitations under the License.
1414
==============================================================================*/
1515

1616
import {Component, EventEmitter, Input, Output, ViewChild} from '@angular/core';
17-
import {
18-
ComponentFixture,
19-
TestBed,
20-
fakeAsync,
21-
flush,
22-
} from '@angular/core/testing';
17+
import {ComponentFixture, TestBed} from '@angular/core/testing';
2318
import {MatIconTestingModule} from '../../testing/mat_icon_module';
2419
import {By} from '@angular/platform-browser';
2520
import {
@@ -867,5 +862,25 @@ describe('data table', () => {
867862
contextMenu.nativeElement.innerHTML.includes('No Actions Available')
868863
).toBeTrue();
869864
});
865+
866+
it('closes when the + button is clicked', () => {
867+
const fixture = createComponent({
868+
headers: mockHeaders,
869+
data: mockTableData,
870+
potentialColumns: mockPotentialColumns,
871+
});
872+
const fixedHeader = fixture.debugElement.queryAll(
873+
By.directive(HeaderCellComponent)
874+
)[0];
875+
fixedHeader.nativeElement.dispatchEvent(new MouseEvent('contextmenu'));
876+
fixture.detectChanges();
877+
expect(fixture.debugElement.query(By.css('.context-menu'))).toBeTruthy();
878+
879+
const addBtn = fixture.debugElement.query(By.css('.add-column-btn'));
880+
addBtn.nativeElement.click();
881+
fixture.detectChanges();
882+
883+
expect(fixture.debugElement.query(By.css('.context-menu'))).toBeFalsy();
884+
});
870885
});
871886
});

0 commit comments

Comments
 (0)