diff --git a/tensorboard/webapp/metrics/actions/index.ts b/tensorboard/webapp/metrics/actions/index.ts index f3f6d602fd..063676205a 100644 --- a/tensorboard/webapp/metrics/actions/index.ts +++ b/tensorboard/webapp/metrics/actions/index.ts @@ -277,5 +277,9 @@ export const metricsUnresolvedPinnedCardsFromLocalStorageAdded = createAction( props<{cards: CardUniqueInfo[]}>() ); +export const metricsClearAllPinnedCards = createAction( + '[Metrics] Clear all pinned cards' +); + // TODO(jieweiwu): Delete after internal code is updated. export const stepSelectorTimeSelectionChanged = timeSelectionChanged; diff --git a/tensorboard/webapp/metrics/data_source/saved_pins_data_source.ts b/tensorboard/webapp/metrics/data_source/saved_pins_data_source.ts index da02bf4b11..8a71cae4c4 100644 --- a/tensorboard/webapp/metrics/data_source/saved_pins_data_source.ts +++ b/tensorboard/webapp/metrics/data_source/saved_pins_data_source.ts @@ -45,4 +45,8 @@ export class SavedPinsDataSource { } return []; } + + removeAllScalarPins(): void { + window.localStorage.setItem(SAVED_SCALAR_PINS_KEY, JSON.stringify([])); + } } diff --git a/tensorboard/webapp/metrics/data_source/saved_pins_data_source_test.ts b/tensorboard/webapp/metrics/data_source/saved_pins_data_source_test.ts index 325acc32d2..d0d7fb4e90 100644 --- a/tensorboard/webapp/metrics/data_source/saved_pins_data_source_test.ts +++ b/tensorboard/webapp/metrics/data_source/saved_pins_data_source_test.ts @@ -114,4 +114,15 @@ describe('SavedPinsDataSource Test', () => { expect(dataSource.getSavedScalarPins()).toEqual(['tag1']); }); }); + + describe('removeAllScalarPins', () => { + it('removes all existing pins', () => { + dataSource.saveScalarPin('tag3'); + dataSource.saveScalarPin('tag4'); + + dataSource.removeAllScalarPins(); + + expect(dataSource.getSavedScalarPins().length).toEqual(0); + }); + }); }); diff --git a/tensorboard/webapp/metrics/effects/index.ts b/tensorboard/webapp/metrics/effects/index.ts index af49b9fbb5..4b8061e4e1 100644 --- a/tensorboard/webapp/metrics/effects/index.ts +++ b/tensorboard/webapp/metrics/effects/index.ts @@ -316,6 +316,21 @@ export class MetricsEffects implements OnInitEffects { }) ); + private readonly removeAllPins$ = this.actions$.pipe( + ofType(actions.metricsClearAllPinnedCards), + withLatestFrom( + this.store.select(selectors.getEnableGlobalPins), + this.store.select(selectors.getShouldPersistSettings) + ), + filter( + ([, enableGlobalPins, shouldPersistSettings]) => + enableGlobalPins && shouldPersistSettings + ), + tap(() => { + this.savedPinsDataSource.removeAllScalarPins(); + }) + ); + /** * In general, this effect dispatch the following actions: * @@ -356,7 +371,11 @@ export class MetricsEffects implements OnInitEffects { /** * Subscribes to: dashboard shown (initAction). */ - this.loadSavedPins$ + this.loadSavedPins$, + /** + * Subscribes to: metricsClearAllPinnedCards. + */ + this.removeAllPins$ ); }, {dispatch: false} diff --git a/tensorboard/webapp/metrics/effects/metrics_effects_test.ts b/tensorboard/webapp/metrics/effects/metrics_effects_test.ts index 4dfbccdb2e..bf0bb695f3 100644 --- a/tensorboard/webapp/metrics/effects/metrics_effects_test.ts +++ b/tensorboard/webapp/metrics/effects/metrics_effects_test.ts @@ -985,5 +985,42 @@ describe('metrics effects', () => { expect(actualActions).toEqual([]); }); }); + + describe('removeAllPins', () => { + let removeAllScalarPinsSpy: jasmine.Spy; + + beforeEach(() => { + removeAllScalarPinsSpy = spyOn( + savedPinsDataSource, + 'removeAllScalarPins' + ); + store.overrideSelector(selectors.getEnableGlobalPins, true); + store.refreshState(); + }); + + it('removes all pins by calling removeAllScalarPins method', () => { + actions$.next(actions.metricsClearAllPinnedCards()); + + expect(removeAllScalarPinsSpy).toHaveBeenCalled(); + }); + + it('does not remove pins if getEnableGlobalPins is false', () => { + store.overrideSelector(selectors.getEnableGlobalPins, false); + store.refreshState(); + + actions$.next(actions.metricsClearAllPinnedCards()); + + expect(removeAllScalarPinsSpy).not.toHaveBeenCalled(); + }); + + it('does not remove pins if getShouldPersistSettings is false', () => { + store.overrideSelector(selectors.getShouldPersistSettings, false); + store.refreshState(); + + actions$.next(actions.metricsClearAllPinnedCards()); + + expect(removeAllScalarPinsSpy).not.toHaveBeenCalled(); + }); + }); }); }); diff --git a/tensorboard/webapp/metrics/store/metrics_reducers.ts b/tensorboard/webapp/metrics/store/metrics_reducers.ts index 2fd1165788..8a6abda21b 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers.ts @@ -77,6 +77,8 @@ import { TagMetadata, TimeSeriesData, TimeSeriesLoadable, + CardToPinnedCard, + PinnedCardToCard, } from './metrics_types'; import {dataTableUtils} from '../../widgets/data_table/utils'; @@ -1525,7 +1527,28 @@ const reducer = createReducer( ], }; } - ) + ), + on(actions.metricsClearAllPinnedCards, (state) => { + const nextCardMetadataMap = {...state.cardMetadataMap}; + const nextCardStepIndex = {...state.cardStepIndex}; + const nextCardStateMap = {...state.cardStateMap}; + + for (const cardId of state.pinnedCardToOriginal.keys()) { + delete nextCardMetadataMap[cardId]; + delete nextCardStepIndex[cardId]; + delete nextCardStateMap[cardId]; + } + + return { + ...state, + cardMetadataMap: nextCardMetadataMap, + cardStateMap: nextCardStateMap, + cardStepIndex: nextCardStepIndex, + cardToPinnedCopy: new Map() as CardToPinnedCard, + cardToPinnedCopyCache: new Map() as CardToPinnedCard, + pinnedCardToOriginal: new Map() as PinnedCardToCard, + }; + }) ); export function reducers(state: MetricsState | undefined, action: Action) { diff --git a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts index c2658587f4..9f72c28a09 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts @@ -4488,5 +4488,57 @@ describe('metrics reducers', () => { expect(state2.unresolvedImportedPinnedCards).toEqual([fakePinnedCard]); }); }); + + describe('#metricsClearAllPinnedCards', () => { + it('unpins all pinned cards', () => { + const beforeState = buildMetricsState({ + cardMetadataMap: { + card1: createScalarCardMetadata(), + pinnedCopy1: createScalarCardMetadata(), + card2: createScalarCardMetadata(), + pinnedCopy2: createScalarCardMetadata(), + }, + cardList: ['card1', 'card2'], + cardStepIndex: { + card1: buildStepIndexMetadata({index: 10}), + pinnedCopy1: buildStepIndexMetadata({index: 20}), + card2: buildStepIndexMetadata({index: 11}), + pinnedCopy2: buildStepIndexMetadata({index: 21}), + }, + cardToPinnedCopy: new Map([ + ['card1', 'pinnedCopy1'], + ['card2', 'pinnedCopy2'], + ]), + cardToPinnedCopyCache: new Map([ + ['card1', 'pinnedCopy1'], + ['card2', 'pinnedCopy2'], + ]), + pinnedCardToOriginal: new Map([ + ['pinnedCopy1', 'card1'], + ['pinnedCopy2', 'card2'], + ]), + }); + const nextState = reducers( + beforeState, + actions.metricsClearAllPinnedCards() + ); + + const expectedState = buildMetricsState({ + cardMetadataMap: { + card1: createScalarCardMetadata(), + card2: createScalarCardMetadata(), + }, + cardList: ['card1', 'card2'], + cardStepIndex: { + card1: buildStepIndexMetadata({index: 10}), + card2: buildStepIndexMetadata({index: 11}), + }, + cardToPinnedCopy: new Map(), + cardToPinnedCopyCache: new Map(), + pinnedCardToOriginal: new Map(), + }); + expect(nextState).toEqual(expectedState); + }); + }); }); }); diff --git a/tensorboard/webapp/metrics/testing.ts b/tensorboard/webapp/metrics/testing.ts index a941997693..ea65d86d69 100644 --- a/tensorboard/webapp/metrics/testing.ts +++ b/tensorboard/webapp/metrics/testing.ts @@ -405,6 +405,8 @@ export class TestingSavedPinsDataSource { getSavedScalarPins() { return []; } + + removeAllScalarPins() {} } export function provideTestingSavedPinsDataSource() { 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 5793a6a2e3..9ce15ee4a7 100644 --- a/tensorboard/webapp/metrics/views/main_view/main_view_test.ts +++ b/tensorboard/webapp/metrics/views/main_view/main_view_test.ts @@ -69,6 +69,7 @@ import {MainViewComponent, SHARE_BUTTON_COMPONENT} from './main_view_component'; import {MainViewContainer} from './main_view_container'; import {PinnedViewComponent} from './pinned_view_component'; import {PinnedViewContainer} from './pinned_view_container'; +import {buildMockState} from '../../../testing/utils'; @Component({ selector: 'card-view', @@ -182,7 +183,11 @@ describe('metrics main view', () => { ], providers: [ provideMockStore({ - initialState: appStateFromMetricsState(buildMetricsState()), + initialState: { + ...buildMockState({ + ...appStateFromMetricsState(buildMetricsState()), + }), + }, }), ], // Skip errors for card renderers, which are tested separately. @@ -1606,6 +1611,67 @@ describe('metrics main view', () => { expect(indicator).toBeTruthy(); }); }); + + describe('clear all pins button', () => { + beforeEach(() => { + store.overrideSelector(selectors.getEnableGlobalPins, true); + }); + + it('does not show the button if getEnableGlobalPins is false', () => { + store.overrideSelector(selectors.getEnableGlobalPins, false); + store.overrideSelector(selectors.getPinnedCardsWithMetadata, []); + const fixture = TestBed.createComponent(MainViewContainer); + fixture.detectChanges(); + + const clearAllButton = fixture.debugElement.query( + By.css('[aria-label="Clear all pinned cards"]') + ); + expect(clearAllButton).toBeNull(); + }); + + it('does not show the button if there is no pinned card', () => { + store.overrideSelector(selectors.getPinnedCardsWithMetadata, []); + const fixture = TestBed.createComponent(MainViewContainer); + fixture.detectChanges(); + + const clearAllButton = fixture.debugElement.query( + By.css('[aria-label="Clear all pinned cards"]') + ); + expect(clearAllButton).toBeNull(); + }); + + it('shows the button if there is a pinned card', () => { + store.overrideSelector(selectors.getPinnedCardsWithMetadata, [ + {cardId: 'card1', ...createCardMetadata(PluginType.SCALARS)}, + {cardId: 'card2', ...createCardMetadata(PluginType.IMAGES)}, + ]); + const fixture = TestBed.createComponent(MainViewContainer); + fixture.detectChanges(); + + const clearAllButton = fixture.debugElement.query( + By.css('[aria-label="Clear all pinned cards"]') + ); + expect(clearAllButton).toBeTruthy(); + }); + + it('dispatch clear all action when the button is clicked', () => { + store.overrideSelector(selectors.getPinnedCardsWithMetadata, [ + {cardId: 'card1', ...createCardMetadata(PluginType.SCALARS)}, + {cardId: 'card2', ...createCardMetadata(PluginType.IMAGES)}, + ]); + const fixture = TestBed.createComponent(MainViewContainer); + fixture.detectChanges(); + + const clearAllButton = fixture.debugElement.query( + By.css('[aria-label="Clear all pinned cards"]') + ); + clearAllButton.nativeElement.click(); + + expect(dispatchedActions).toEqual([ + actions.metricsClearAllPinnedCards(), + ]); + }); + }); }); describe('slideout menu', () => { diff --git a/tensorboard/webapp/metrics/views/main_view/pinned_view_component.scss b/tensorboard/webapp/metrics/views/main_view/pinned_view_component.scss index f6cc2f02bb..8f8df63b88 100644 --- a/tensorboard/webapp/metrics/views/main_view/pinned_view_component.scss +++ b/tensorboard/webapp/metrics/views/main_view/pinned_view_component.scss @@ -26,6 +26,25 @@ mat-icon { margin-right: 5px; } +.group-toolbar { + justify-content: space-between; +} + +.left-items { + display: flex; + align-items: center; +} + +.right-items { + button { + $_height: 25px; + font-size: 12px; + font-weight: normal; + height: $_height; + line-height: $_height; + } +} + .group-text { display: flex; align-items: baseline; diff --git a/tensorboard/webapp/metrics/views/main_view/pinned_view_component.ts b/tensorboard/webapp/metrics/views/main_view/pinned_view_component.ts index 5852ffb9a5..3d8583bf34 100644 --- a/tensorboard/webapp/metrics/views/main_view/pinned_view_component.ts +++ b/tensorboard/webapp/metrics/views/main_view/pinned_view_component.ts @@ -12,7 +12,13 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. ==============================================================================*/ -import {ChangeDetectionStrategy, Component, Input} from '@angular/core'; +import { + ChangeDetectionStrategy, + Component, + EventEmitter, + Input, + Output, +} from '@angular/core'; import {CardObserver} from '../card_renderer/card_lazy_loader'; import {CardIdWithMetadata} from '../metrics_view_types'; @@ -20,23 +26,37 @@ import {CardIdWithMetadata} from '../metrics_view_types'; selector: 'metrics-pinned-view-component', template: `
- - - Pinned - {{ cardIdsWithMetadata.length }} cards - - New card pinned + + + Pinned + {{ cardIdsWithMetadata.length }} cards + + New card pinned + - +
+
+ +
(); } diff --git a/tensorboard/webapp/metrics/views/main_view/pinned_view_container.ts b/tensorboard/webapp/metrics/views/main_view/pinned_view_container.ts index 67b9a60216..c280008c93 100644 --- a/tensorboard/webapp/metrics/views/main_view/pinned_view_container.ts +++ b/tensorboard/webapp/metrics/views/main_view/pinned_view_container.ts @@ -21,6 +21,8 @@ import {DeepReadonly} from '../../../util/types'; import {getLastPinnedCardTime, getPinnedCardsWithMetadata} from '../../store'; import {CardObserver} from '../card_renderer/card_lazy_loader'; import {CardIdWithMetadata} from '../metrics_view_types'; +import {metricsClearAllPinnedCards} from '../../actions'; +import {getEnableGlobalPins} from '../../../selectors'; @Component({ selector: 'metrics-pinned-view', @@ -29,6 +31,8 @@ import {CardIdWithMetadata} from '../metrics_view_types'; [cardIdsWithMetadata]="cardIdsWithMetadata$ | async" [lastPinnedCardTime]="lastPinnedCardTime$ | async" [cardObserver]="cardObserver" + [globalPinsEnabled]="globalPinsEnabled$ | async" + (onClearAllPinsClicked)="onClearAllPinsClicked()" > `, changeDetection: ChangeDetectionStrategy.OnPush, @@ -47,4 +51,10 @@ export class PinnedViewContainer { // pins after page load. skip(1) ); + + readonly globalPinsEnabled$ = this.store.select(getEnableGlobalPins); + + onClearAllPinsClicked() { + this.store.dispatch(metricsClearAllPinnedCards()); + } }