Skip to content

Commit d638512

Browse files
committed
resolve comments - use tap instead of map, change action name
1 parent a41e144 commit d638512

File tree

5 files changed

+26
-20
lines changed

5 files changed

+26
-20
lines changed

tensorboard/webapp/metrics/actions/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,8 +272,8 @@ export const metricsHideEmptyCardsToggled = createAction(
272272
'[Metrics] Hide Empty Cards Changed'
273273
);
274274

275-
export const metricsUnresolvedPinnedCardsAdded = createAction(
276-
'[Metrics] Unresolved Pinned Cards Added',
275+
export const metricsUnresolvedPinnedCardsFromLocalStorageAdded = createAction(
276+
'[Metrics] Unresolved Pinned Cards From Local Storage Added',
277277
props<{cards: CardUniqueInfo[]}>()
278278
);
279279

tensorboard/webapp/metrics/effects/index.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ export class MetricsEffects implements OnInitEffects {
274274
([, , enableGlobalPins, shouldPersistSettings]) =>
275275
enableGlobalPins && shouldPersistSettings
276276
),
277-
map(([{cardId, canCreateNewPins, wasPinned}, fetchInfos]) => {
277+
tap(([{cardId, canCreateNewPins, wasPinned}, fetchInfos]) => {
278278
const card = fetchInfos.find((value) => value.id === cardId);
279279
// Saving only scalar pinned cards.
280280
if (!card || card.plugin !== PluginType.SCALARS) {
@@ -289,7 +289,8 @@ export class MetricsEffects implements OnInitEffects {
289289
);
290290

291291
private readonly loadSavedPins$ = this.actions$.pipe(
292-
ofType(initAction, coreActions.pluginsListingLoaded),
292+
// Should be dispatch before stateRehydratedFromUrl.
293+
ofType(initAction),
293294
withLatestFrom(
294295
this.store.select(selectors.getEnableGlobalPins),
295296
this.store.select(selectors.getShouldPersistSettings)
@@ -298,7 +299,7 @@ export class MetricsEffects implements OnInitEffects {
298299
([, enableGlobalPins, shouldPersistSettings]) =>
299300
enableGlobalPins && shouldPersistSettings
300301
),
301-
map(() => {
302+
tap(() => {
302303
const tags = this.savedPinsDataSource.getSavedScalarPins();
303304
if (!tags || tags.length === 0) {
304305
return;
@@ -308,7 +309,7 @@ export class MetricsEffects implements OnInitEffects {
308309
tag: tag,
309310
}));
310311
this.store.dispatch(
311-
actions.metricsUnresolvedPinnedCardsAdded({
312+
actions.metricsUnresolvedPinnedCardsFromLocalStorageAdded({
312313
cards: unresolvedPinnedCards,
313314
})
314315
);
@@ -353,7 +354,7 @@ export class MetricsEffects implements OnInitEffects {
353354
*/
354355
this.addOrRemovePin$,
355356
/**
356-
* Subscribes to: dashboard shown.
357+
* Subscribes to: dashboard shown (initAction).
357358
*/
358359
this.loadSavedPins$
359360
);

tensorboard/webapp/metrics/effects/metrics_effects_test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -942,7 +942,7 @@ describe('metrics effects', () => {
942942
actions$.next(TEST_ONLY.initAction());
943943

944944
expect(actualActions).toEqual([
945-
actions.metricsUnresolvedPinnedCardsAdded({
945+
actions.metricsUnresolvedPinnedCardsFromLocalStorageAdded({
946946
cards: fakeUniqueCardInfos,
947947
}),
948948
]);

tensorboard/webapp/metrics/store/metrics_reducers.ts

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1514,15 +1514,18 @@ const reducer = createReducer(
15141514
on(actions.metricsSlideoutMenuClosed, (state) => {
15151515
return {...state, isSlideoutMenuOpen: false};
15161516
}),
1517-
on(actions.metricsUnresolvedPinnedCardsAdded, (state, {cards}) => {
1518-
return {
1519-
...state,
1520-
unresolvedImportedPinnedCards: [
1521-
...state.unresolvedImportedPinnedCards,
1522-
...cards,
1523-
],
1524-
};
1525-
})
1517+
on(
1518+
actions.metricsUnresolvedPinnedCardsFromLocalStorageAdded,
1519+
(state, {cards}) => {
1520+
return {
1521+
...state,
1522+
unresolvedImportedPinnedCards: [
1523+
...state.unresolvedImportedPinnedCards,
1524+
...cards,
1525+
],
1526+
};
1527+
}
1528+
)
15261529
);
15271530

15281531
export function reducers(state: MetricsState | undefined, action: Action) {

tensorboard/webapp/metrics/store/metrics_reducers_test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4472,16 +4472,18 @@ describe('metrics reducers', () => {
44724472
});
44734473
});
44744474

4475-
describe('#metricsUnresolvedPinnedCardsAdded', () => {
4476-
it('add unresolved pinned card to unresolvedImportedPinnedCards', () => {
4475+
describe('#metricsUnresolvedPinnedCardsFromLocalStorageAdded', () => {
4476+
it('adds unresolved pinned card to unresolvedImportedPinnedCards', () => {
44774477
const fakePinnedCard = {tag: 'tagA', plugin: PluginType.SCALARS};
44784478
const state1 = buildMetricsState({
44794479
unresolvedImportedPinnedCards: [],
44804480
});
44814481

44824482
const state2 = reducers(
44834483
state1,
4484-
actions.metricsUnresolvedPinnedCardsAdded({cards: [fakePinnedCard]})
4484+
actions.metricsUnresolvedPinnedCardsFromLocalStorageAdded({
4485+
cards: [fakePinnedCard],
4486+
})
44854487
);
44864488
expect(state2.unresolvedImportedPinnedCards).toEqual([fakePinnedCard]);
44874489
});

0 commit comments

Comments
 (0)