diff --git a/tensorboard/webapp/metrics/actions/BUILD b/tensorboard/webapp/metrics/actions/BUILD index 8a53a69923..891ccbaaa0 100644 --- a/tensorboard/webapp/metrics/actions/BUILD +++ b/tensorboard/webapp/metrics/actions/BUILD @@ -12,6 +12,7 @@ tf_ts_library( deps = [ "//tensorboard/webapp/metrics:internal_types", "//tensorboard/webapp/metrics/data_source", + "//tensorboard/webapp/util:dom", "@npm//@ngrx/store", ], ) diff --git a/tensorboard/webapp/metrics/actions/index.ts b/tensorboard/webapp/metrics/actions/index.ts index c043923a66..2e533d4429 100644 --- a/tensorboard/webapp/metrics/actions/index.ts +++ b/tensorboard/webapp/metrics/actions/index.ts @@ -14,6 +14,7 @@ limitations under the License. ==============================================================================*/ import {createAction, props} from '@ngrx/store'; +import {ElementId} from '../../util/dom'; import { TagMetadata, TimeSeriesRequest, @@ -113,12 +114,15 @@ export const fetchTimeSeriesLoaded = createAction( ); /** - * An event when some cards enter or exit the viewport. The card sets must be - * mutually exclusive. + * An event when some cards enter or exit the viewport. An element within + * `enteredCards` must not be found within `exitedCards`, and vice versa. */ export const cardVisibilityChanged = createAction( '[Metrics] Card Visibility Changed', - props<{enteredCards: Set; exitedCards: Set}>() + props<{ + enteredCards: Array<{elementId: ElementId; cardId: CardId}>; + exitedCards: Array<{elementId: ElementId; cardId: CardId}>; + }>() ); export const cardStepSliderChanged = createAction( diff --git a/tensorboard/webapp/metrics/effects/BUILD b/tensorboard/webapp/metrics/effects/BUILD index b01a06451c..b0979fc828 100644 --- a/tensorboard/webapp/metrics/effects/BUILD +++ b/tensorboard/webapp/metrics/effects/BUILD @@ -46,6 +46,7 @@ tf_ts_library( "//tensorboard/webapp/metrics/data_source", "//tensorboard/webapp/metrics/store", "//tensorboard/webapp/types", + "//tensorboard/webapp/util:dom", "//tensorboard/webapp/webapp_data_source:http_client_testing", "@npm//@ngrx/effects", "@npm//@ngrx/store", diff --git a/tensorboard/webapp/metrics/effects/metrics_effects_test.ts b/tensorboard/webapp/metrics/effects/metrics_effects_test.ts index 952edc8b2f..9d0e1eb32c 100644 --- a/tensorboard/webapp/metrics/effects/metrics_effects_test.ts +++ b/tensorboard/webapp/metrics/effects/metrics_effects_test.ts @@ -26,6 +26,7 @@ import {of, Subject} from 'rxjs'; import {buildNavigatedAction} from '../../app_routing/testing'; import {State} from '../../app_state'; import * as selectors from '../../selectors'; +import {nextElementId} from '../../util/dom'; import * as actions from '../actions'; import { METRICS_PLUGIN_ID, @@ -450,10 +451,7 @@ describe('metrics effects', () => { store.refreshState(); actions$.next( - actions.cardVisibilityChanged({ - enteredCards: new Set(), - exitedCards: new Set(), - }) + actions.cardVisibilityChanged({enteredCards: [], exitedCards: []}) ); expect(fetchTimeSeriesSpy).not.toHaveBeenCalled(); @@ -474,6 +472,7 @@ describe('metrics effects', () => { loadState: DataLoadState.NOT_LOADED, }); + const card1ElementId = nextElementId(); store.overrideSelector( selectors.getVisibleCardIdSet, new Set([]) @@ -481,8 +480,8 @@ describe('metrics effects', () => { store.refreshState(); actions$.next( actions.cardVisibilityChanged({ - enteredCards: new Set(), - exitedCards: new Set(['card1']), + enteredCards: [], + exitedCards: [{elementId: card1ElementId, cardId: 'card1'}], }) ); @@ -496,8 +495,8 @@ describe('metrics effects', () => { store.refreshState(); actions$.next( actions.cardVisibilityChanged({ - enteredCards: new Set(['card1']), - exitedCards: new Set(), + enteredCards: [{elementId: card1ElementId, cardId: 'card1'}], + exitedCards: [], }) ); @@ -530,6 +529,7 @@ describe('metrics effects', () => { }); // Initial load. + const card1ElementId = nextElementId(); store.overrideSelector( selectors.getVisibleCardIdSet, new Set(['card1']) @@ -537,8 +537,8 @@ describe('metrics effects', () => { store.refreshState(); actions$.next( actions.cardVisibilityChanged({ - enteredCards: new Set(['card1']), - exitedCards: new Set(), + enteredCards: [{elementId: card1ElementId, cardId: 'card1'}], + exitedCards: [], }) ); @@ -550,8 +550,8 @@ describe('metrics effects', () => { store.refreshState(); actions$.next( actions.cardVisibilityChanged({ - enteredCards: new Set(), - exitedCards: new Set(['card1']), + enteredCards: [], + exitedCards: [{elementId: card1ElementId, cardId: 'card1'}], }) ); @@ -566,8 +566,8 @@ describe('metrics effects', () => { store.refreshState(); actions$.next( actions.cardVisibilityChanged({ - enteredCards: new Set(['card1']), - exitedCards: new Set(), + enteredCards: [{elementId: card1ElementId, cardId: 'card1'}], + exitedCards: [], }) ); @@ -628,8 +628,11 @@ describe('metrics effects', () => { store.refreshState(); actions$.next( actions.cardVisibilityChanged({ - enteredCards: new Set(['card1', 'card2']), - exitedCards: new Set(), + enteredCards: [ + {elementId: nextElementId(), cardId: 'card1'}, + {elementId: nextElementId(), cardId: 'card2'}, + ], + exitedCards: [], }) ); @@ -673,8 +676,8 @@ describe('metrics effects', () => { store.refreshState(); actions$.next( actions.cardVisibilityChanged({ - enteredCards: new Set(['card1']), - exitedCards: new Set(), + enteredCards: [{elementId: nextElementId(), cardId: 'card1'}], + exitedCards: [], }) ); diff --git a/tensorboard/webapp/metrics/store/BUILD b/tensorboard/webapp/metrics/store/BUILD index 2f4bdaa349..326d83341b 100644 --- a/tensorboard/webapp/metrics/store/BUILD +++ b/tensorboard/webapp/metrics/store/BUILD @@ -22,6 +22,7 @@ tf_ts_library( "//tensorboard/webapp/metrics/data_source", "//tensorboard/webapp/persistent_settings", "//tensorboard/webapp/types", + "//tensorboard/webapp/util:dom", "//tensorboard/webapp/util:lang", "//tensorboard/webapp/util:ngrx", "//tensorboard/webapp/util:types", @@ -66,6 +67,7 @@ tf_ts_library( "//tensorboard/webapp/metrics:internal_types", "//tensorboard/webapp/metrics/data_source", "//tensorboard/webapp/types", + "//tensorboard/webapp/util:dom", ], ) @@ -91,6 +93,7 @@ tf_ts_library( "//tensorboard/webapp/metrics/data_source", "//tensorboard/webapp/persistent_settings", "//tensorboard/webapp/types", + "//tensorboard/webapp/util:dom", "@npm//@types/jasmine", ], ) diff --git a/tensorboard/webapp/metrics/store/metrics_reducers.ts b/tensorboard/webapp/metrics/store/metrics_reducers.ts index 8603632b7f..819a96a07c 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers.ts @@ -20,6 +20,7 @@ import {RouteKind} from '../../app_routing/types'; import * as coreActions from '../../core/actions'; import {globalSettingsLoaded} from '../../persistent_settings'; import {DataLoadState} from '../../types/data'; +import {ElementId} from '../../util/dom'; import {mapObjectValues} from '../../util/lang'; import {composeReducers} from '../../util/ngrx'; import * as actions from '../actions'; @@ -255,7 +256,7 @@ const {initialState, reducers: routeContextReducer} = createRouteContextedState< }, settings: METRICS_SETTINGS_DEFAULT, settingOverrides: {}, - visibleCards: new Set(), + visibleCardMap: new Map(), }, /** onRouteIdChanged */ @@ -265,7 +266,7 @@ const {initialState, reducers: routeContextReducer} = createRouteContextedState< // Reset visible cards in case we resume a route that was left dirty. // Since visibility tracking is async, the state may not have received // 'exited card' updates when it was cached by the router. - visibleCards: new Set(), + visibleCardMap: new Map(), }; } ); @@ -742,25 +743,24 @@ const reducer = createReducer( return {...state, tagGroupExpanded}; }), on(actions.cardVisibilityChanged, (state, {enteredCards, exitedCards}) => { - if (enteredCards.size === 0 && exitedCards.size === 0) { + if (!enteredCards.length && !exitedCards.length) { return state; } - const visibleCards = new Set(state.visibleCards); - enteredCards.forEach((cardId) => { - visibleCards.add(cardId); - }); - exitedCards.forEach((cardId) => { - visibleCards.delete(cardId); - - if (enteredCards.has(cardId)) { + const visibleCardMap = new Map(state.visibleCardMap); + enteredCards.forEach(({elementId, cardId}) => { + const existingCardId = visibleCardMap.get(elementId) ?? null; + if (existingCardId !== null && existingCardId !== cardId) { throw new Error( - `A 'cardVisibilityChanged' with an invalid ` + - `payload contains overlapping sets` + `A DOM element cannot be reused for more than 1 unique card metadata` ); } + visibleCardMap.set(elementId, cardId); + }); + exitedCards.forEach(({elementId}) => { + visibleCardMap.delete(elementId); }); - return {...state, visibleCards}; + return {...state, visibleCardMap}; }), on(actions.cardPinStateToggled, (state, {cardId}) => { const isPinnedCopy = state.pinnedCardToOriginal.has(cardId); diff --git a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts index a6c23ee4ac..acf124c582 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts @@ -18,6 +18,7 @@ import {RouteKind} from '../../app_routing/types'; import * as coreActions from '../../core/actions'; import {globalSettingsLoaded} from '../../persistent_settings'; import {DataLoadState} from '../../types/data'; +import {nextElementId} from '../../util/dom'; import * as actions from '../actions'; import { PluginType, @@ -501,7 +502,10 @@ describe('metrics reducers', () => { describe('route id changes', () => { it('resets data when mounting a new route', () => { const prevState = buildMetricsState({ - visibleCards: new Set(['card1', 'card2']), + visibleCardMap: new Map([ + [nextElementId(), 'card1'], + [nextElementId(), 'card2'], + ]), }); const navigateFrom1to2 = routingActions.navigated({ @@ -529,9 +533,9 @@ describe('metrics reducers', () => { nextState = reducers(nextState, navigateFrom2to1); const expectedState = buildMetricsState({ - visibleCards: new Set(), + visibleCardMap: new Map(), }); - expect(nextState.visibleCards).toEqual(expectedState.visibleCards); + expect(nextState.visibleCardMap).toEqual(expectedState.visibleCardMap); }); }); @@ -1224,26 +1228,29 @@ describe('metrics reducers', () => { describe('card visibility', () => { it('no-ops when nothing is changed', () => { const beforeState = buildMetricsState({ - visibleCards: new Set(['card1']), + visibleCardMap: new Map([[nextElementId(), 'card1']]), }); const action = actions.cardVisibilityChanged({ - enteredCards: new Set(), - exitedCards: new Set(), + enteredCards: [], + exitedCards: [], }); const nextState = reducers(beforeState, action); - expect(nextState.visibleCards).toEqual(new Set(['card1'])); + expect(nextState.visibleCardMap).toEqual( + new Map([[jasmine.any(Symbol), 'card1']]) + ); expect(nextState).toBe(beforeState); }); it('handles bad payloads', () => { + const existingElementId = nextElementId(); const beforeState = buildMetricsState({ - visibleCards: new Set(['card1']), + visibleCardMap: new Map([[existingElementId, 'card1']]), }); const action = actions.cardVisibilityChanged({ - enteredCards: new Set(['duplicateCard']), - exitedCards: new Set(['duplicateCard']), + enteredCards: [{elementId: existingElementId, cardId: 'card2'}], + exitedCards: [], }); let nextState = beforeState; expect(() => { @@ -1253,19 +1260,57 @@ describe('metrics reducers', () => { }); it('handles adding and removing cards', () => { + const existingElementIds = [nextElementId(), nextElementId()]; const beforeState = buildMetricsState({ - visibleCards: new Set(['existingCard1', 'existingCard2']), + visibleCardMap: new Map([ + [existingElementIds[0], 'existingCard1'], + [existingElementIds[1], 'existingCard2'], + ]), }); + const newCard1ElementId = nextElementId(); const action = actions.cardVisibilityChanged({ - enteredCards: new Set(['existingCard1', 'newCard1']), - exitedCards: new Set(['existingCard2', 'newCard2']), + enteredCards: [ + {elementId: existingElementIds[0], cardId: 'existingCard1'}, + {elementId: newCard1ElementId, cardId: 'newCard1'}, + ], + exitedCards: [ + {elementId: existingElementIds[1], cardId: 'existingCard2'}, + {elementId: nextElementId(), cardId: 'newCard2'}, + ], }); const nextState = reducers(beforeState, action); - expect(nextState.visibleCards).toEqual( - new Set(['existingCard1', 'newCard1']) + expect(nextState.visibleCardMap).toEqual( + new Map([ + [existingElementIds[0], 'existingCard1'], + [newCard1ElementId, 'newCard1'], + ]) ); }); + + it( + 'marks a card as visible when it enters and exits on different ' + + 'elements', + () => { + const existingElementIds = [nextElementId(), nextElementId()]; + const beforeState = buildMetricsState({ + visibleCardMap: new Map(), + }); + + const action = actions.cardVisibilityChanged({ + enteredCards: [ + {elementId: existingElementIds[0], cardId: 'duplicateCard'}, + ], + exitedCards: [ + {elementId: existingElementIds[1], cardId: 'duplicateCard'}, + ], + }); + const nextState = reducers(beforeState, action); + expect(nextState.visibleCardMap).toEqual( + new Map([[existingElementIds[0], 'duplicateCard']]) + ); + } + ); }); describe('cardPinStateToggled', () => { diff --git a/tensorboard/webapp/metrics/store/metrics_selectors.ts b/tensorboard/webapp/metrics/store/metrics_selectors.ts index d6b10aa25d..c342ddbacc 100644 --- a/tensorboard/webapp/metrics/store/metrics_selectors.ts +++ b/tensorboard/webapp/metrics/store/metrics_selectors.ts @@ -16,6 +16,7 @@ import {createFeatureSelector, createSelector} from '@ngrx/store'; import {State} from '../../app_state'; import {DataLoadState} from '../../types/data'; +import {ElementId} from '../../util/dom'; import {DeepReadonly} from '../../util/types'; import { CardId, @@ -143,17 +144,17 @@ export const getCardMetadata = createSelector( ); // A cheap identity selector to skip recomputing selectors when `state` changes. -const selectVisibleCardIdSet = createSelector( +const selectVisibleCardMap = createSelector( selectMetricsState, - (state): Set => { - return state.visibleCards; + (state): Map => { + return state.visibleCardMap; } ); export const getVisibleCardIdSet = createSelector( - selectVisibleCardIdSet, - (cardIdSet): Set => { - return cardIdSet; + selectVisibleCardMap, + (visibleCardMap): Set => { + return new Set(visibleCardMap.values()); } ); diff --git a/tensorboard/webapp/metrics/store/metrics_selectors_test.ts b/tensorboard/webapp/metrics/store/metrics_selectors_test.ts index a5a75cfbbb..ab652fff7f 100644 --- a/tensorboard/webapp/metrics/store/metrics_selectors_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_selectors_test.ts @@ -13,6 +13,7 @@ See the License for the specific language governing permissions and limitations under the License. ==============================================================================*/ import {DataLoadState} from '../../types/data'; +import {nextElementId} from '../../util/dom'; import {PluginType} from '../data_source'; import {HistogramMode, TooltipSort, XAxisType} from '../internal_types'; import { @@ -296,7 +297,7 @@ describe('metrics selectors', () => { it('returns an emtpy array', () => { const state = appStateFromMetricsState( buildMetricsState({ - visibleCards: new Set(), + visibleCardMap: new Map(), }) ); expect(selectors.getVisibleCardIdSet(state)).toEqual(new Set([])); @@ -305,7 +306,10 @@ describe('metrics selectors', () => { it('returns a non-empty array', () => { const state = appStateFromMetricsState( buildMetricsState({ - visibleCards: new Set(['card1', 'card2']), + visibleCardMap: new Map([ + [nextElementId(), 'card1'], + [nextElementId(), 'card2'], + ]), }) ); expect(selectors.getVisibleCardIdSet(state)).toEqual( diff --git a/tensorboard/webapp/metrics/store/metrics_types.ts b/tensorboard/webapp/metrics/store/metrics_types.ts index ee952ef4fc..d23e2fa35c 100644 --- a/tensorboard/webapp/metrics/store/metrics_types.ts +++ b/tensorboard/webapp/metrics/store/metrics_types.ts @@ -15,6 +15,7 @@ limitations under the License. import {DataLoadState} from '../../types/data'; import {RouteContextedState} from '../../app_routing/route_contexted_reducer_helper'; +import {ElementId} from '../../util/dom'; import { HistogramStepDatum, ImageStepDatum, @@ -193,7 +194,10 @@ export interface MetricsRoutelessState { // prop. It either is set by application or a user via settings storage. settings: MetricsSettings; settingOverrides: Partial; - visibleCards: Set; + /** + * Map from ElementId to CardId. Only contains all visible cards. + */ + visibleCardMap: Map; } export type MetricsState = RouteContextedState< diff --git a/tensorboard/webapp/metrics/testing.ts b/tensorboard/webapp/metrics/testing.ts index c5f5a4fef3..67edb50b02 100644 --- a/tensorboard/webapp/metrics/testing.ts +++ b/tensorboard/webapp/metrics/testing.ts @@ -86,7 +86,7 @@ function buildBlankState(): MetricsState { unresolvedImportedPinnedCards: [], cardMetadataMap: {}, cardStepIndex: {}, - visibleCards: new Set(), + visibleCardMap: new Map(), tagFilter: '', tagGroupExpanded: new Map(), selectedTime: null, diff --git a/tensorboard/webapp/metrics/views/card_renderer/BUILD b/tensorboard/webapp/metrics/views/card_renderer/BUILD index 27e90afce0..5d79739ae8 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/BUILD +++ b/tensorboard/webapp/metrics/views/card_renderer/BUILD @@ -29,6 +29,7 @@ tf_ng_module( "//tensorboard/webapp/metrics/actions", "//tensorboard/webapp/metrics/data_source", "//tensorboard/webapp/types:ui", + "//tensorboard/webapp/util:dom", "@npm//@angular/common", "@npm//@angular/core", "@npm//@ngrx/store", @@ -300,6 +301,7 @@ tf_ts_library( "//tensorboard/webapp/testing:mat_icon", "//tensorboard/webapp/types", "//tensorboard/webapp/types:ui", + "//tensorboard/webapp/util:dom", "//tensorboard/webapp/widgets:resize_detector_testing", "//tensorboard/webapp/widgets/histogram:types", "//tensorboard/webapp/widgets/line_chart_v2", diff --git a/tensorboard/webapp/metrics/views/card_renderer/card_lazy_loader.ts b/tensorboard/webapp/metrics/views/card_renderer/card_lazy_loader.ts index 1723cc656d..a443a35f31 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/card_lazy_loader.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/card_lazy_loader.ts @@ -16,14 +16,18 @@ import {Directive, ElementRef, Input, OnDestroy, OnInit} from '@angular/core'; import {Store} from '@ngrx/store'; import {State} from '../../../app_state'; +import {ElementId, nextElementId} from '../../../util/dom'; import * as actions from '../../actions'; import {CardId} from '../../types'; -const elementToCardIdMap = new WeakMap(); +const elementToIds = new WeakMap< + Element, + {elementId: ElementId; cardId: CardId} +>(); type CardObserverCallback = ( - enteredCards: Set, - exitedCards: Set + enteredCards: Set, + exitedCards: Set ) => void; export class CardObserver { @@ -92,23 +96,15 @@ export class CardObserver { */ entries.sort((a, b) => a.time - b.time); - const enteredCards = new Set(); - const exitedCards = new Set(); - for (const entry of entries) { - const target = entry.target; - const cardId = elementToCardIdMap.get(target); - if (!cardId) { - throw new Error( - 'A CardObserver element must be associated with a CardId' - ); - } - - if (entry.isIntersecting) { - enteredCards.add(cardId); - exitedCards.delete(cardId); + const enteredElements = new Set(); + const exitedElements = new Set(); + for (const {isIntersecting, target} of entries) { + if (isIntersecting) { + enteredElements.add(target); + exitedElements.delete(target); } else { - enteredCards.delete(cardId); - exitedCards.add(cardId); + enteredElements.delete(target); + exitedElements.add(target); } /** @@ -118,12 +114,12 @@ export class CardObserver { * - Callback fires for just A, unobserving B * - B's callback never fires */ - if (this.destroyedTargets.has(target) && !entry.isIntersecting) { + if (this.destroyedTargets.has(target) && !isIntersecting) { this.destroyedTargets.delete(target); this.intersectionObserver!.unobserve(target); } } - this.intersectionCallback!(enteredCards, exitedCards); + this.intersectionCallback!(enteredElements, exitedElements); } onCardIntersectionForTest(entries: IntersectionObserverEntry[]) { @@ -160,20 +156,45 @@ export class CardLazyLoader implements OnInit, OnDestroy { private readonly store: Store ) {} - onCardIntersection(enteredCards: Set, exitedCards: Set) { + onCardIntersection( + enteredElements: Set, + exitedElements: Set + ) { + const enteredCards = [...enteredElements].map((element) => { + const ids = elementToIds.get(element); + if (!ids) { + throw new Error( + 'A CardObserver element must have an associated element id and card id.' + ); + } + return {elementId: ids.elementId, cardId: ids.cardId}; + }); + const exitedCards = [...exitedElements].map((element) => { + const ids = elementToIds.get(element); + if (!ids) { + throw new Error( + 'A CardObserver element must have an associated element id and card id.' + ); + } + return {elementId: ids.elementId, cardId: ids.cardId}; + }); this.store.dispatch( actions.cardVisibilityChanged({enteredCards, exitedCards}) ); } ngOnInit() { - elementToCardIdMap.set(this.host.nativeElement, this.cardId); + const element = this.host.nativeElement; + elementToIds.set(element, { + elementId: nextElementId(), + cardId: this.cardId, + }); if (!this.cardObserver) { this.cardObserver = new CardObserver(); } this.cardObserver.initialize(this.onCardIntersection.bind(this)); - this.cardObserver.add(this.host.nativeElement); + this.cardObserver.add(element); } ngOnDestroy() { diff --git a/tensorboard/webapp/metrics/views/card_renderer/card_lazy_loader_test.ts b/tensorboard/webapp/metrics/views/card_renderer/card_lazy_loader_test.ts index 1b1da79b95..2e216c7105 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/card_lazy_loader_test.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/card_lazy_loader_test.ts @@ -146,8 +146,10 @@ describe('card view test', () => { expect(unobserveSpy).not.toHaveBeenCalled(); expect(dispatchedActions).toEqual([ actions.cardVisibilityChanged({ - enteredCards: new Set(['card1']), - exitedCards: new Set(), + enteredCards: [ + {elementId: jasmine.any(Symbol) as any, cardId: 'card1'}, + ], + exitedCards: [], }), ]); @@ -163,12 +165,14 @@ describe('card view test', () => { expect(unobserveSpy).toHaveBeenCalled(); expect(dispatchedActions).toEqual([ actions.cardVisibilityChanged({ - enteredCards: new Set(['card1']), - exitedCards: new Set(), + enteredCards: [ + {elementId: jasmine.any(Symbol) as any, cardId: 'card1'}, + ], + exitedCards: [], }), actions.cardVisibilityChanged({ - enteredCards: new Set(), - exitedCards: new Set(['card1']), + enteredCards: [], + exitedCards: [{elementId: jasmine.any(Symbol) as any, cardId: 'card1'}], }), ]); }); diff --git a/tensorboard/webapp/metrics/views/main_view/main_view_test.ts b/tensorboard/webapp/metrics/views/main_view/main_view_test.ts index 4d3f553b58..a837fd77ef 100644 --- a/tensorboard/webapp/metrics/views/main_view/main_view_test.ts +++ b/tensorboard/webapp/metrics/views/main_view/main_view_test.ts @@ -484,8 +484,13 @@ describe('metrics main view', () => { expect(dispatchedActions).toEqual([ actions.cardVisibilityChanged({ - enteredCards: new Set([directives[0].cardId]), - exitedCards: new Set(), + enteredCards: [ + { + elementId: jasmine.any(Symbol) as any, + cardId: directives[0].cardId, + }, + ], + exitedCards: [], }), ]); @@ -509,12 +514,31 @@ describe('metrics main view', () => { expect(dispatchedActions).toEqual([ actions.cardVisibilityChanged({ - enteredCards: new Set([directives[0].cardId]), - exitedCards: new Set(), + enteredCards: [ + { + elementId: jasmine.any(Symbol) as any, + cardId: directives[0].cardId, + }, + ], + exitedCards: [], }), actions.cardVisibilityChanged({ - enteredCards: new Set([directives[1].cardId, directives[2].cardId]), - exitedCards: new Set([directives[0].cardId]), + enteredCards: [ + { + elementId: jasmine.any(Symbol) as any, + cardId: directives[1].cardId, + }, + { + elementId: jasmine.any(Symbol) as any, + cardId: directives[2].cardId, + }, + ], + exitedCards: [ + { + elementId: jasmine.any(Symbol) as any, + cardId: directives[0].cardId, + }, + ], }), ]); }); @@ -545,8 +569,13 @@ describe('metrics main view', () => { // The more recent entry does not intersect. expect(dispatchedActions).toEqual([ actions.cardVisibilityChanged({ - enteredCards: new Set(), - exitedCards: new Set([directives[0].cardId]), + enteredCards: [], + exitedCards: [ + { + elementId: jasmine.any(Symbol) as any, + cardId: directives[0].cardId, + }, + ], }), ]); @@ -565,12 +594,22 @@ describe('metrics main view', () => { expect(dispatchedActions).toEqual([ actions.cardVisibilityChanged({ - enteredCards: new Set(), - exitedCards: new Set([directives[0].cardId]), + enteredCards: [], + exitedCards: [ + { + elementId: jasmine.any(Symbol) as any, + cardId: directives[0].cardId, + }, + ], }), actions.cardVisibilityChanged({ - enteredCards: new Set([directives[0].cardId]), - exitedCards: new Set(), + enteredCards: [ + { + elementId: jasmine.any(Symbol) as any, + cardId: directives[0].cardId, + }, + ], + exitedCards: [], }), ]); }); diff --git a/tensorboard/webapp/util/dom.ts b/tensorboard/webapp/util/dom.ts index 0f5ef75061..dbf758f8af 100644 --- a/tensorboard/webapp/util/dom.ts +++ b/tensorboard/webapp/util/dom.ts @@ -21,3 +21,22 @@ export enum MouseEventButtons { FOURTH = 0b1000, // often 'back' button, but can differ by mouse controller. FIFTH = 0b100000, // often 'forward' button, but can differ by mouse controller. } + +let currElementId = 0; + +/** + * An opaque id intended to refer to exactly one specific DOM Element. + */ +export type ElementId = Symbol; + +/** + * Generates a new opaque id for the consumer to associate with a unique DOM + * Element. Consumers are responsible for the 1:1 association between Elements + * and ids. + */ +export function nextElementId(): ElementId { + // An incrementing number is actually unnecessary, but makes it easier to + // identify whether 2 elements are the same in the console when debugging. + currElementId++; + return Symbol(currElementId); +}