router: do not warn users about dirty updates on the same page (tenso…

…rflow#5275) * do not warn users about dirty updates on the same page When users change tabs in the same dashboard or edit query parameters in the same list view, unsaved updates should be kept without warnings.
rileyajones · Aug 30, 2021 · f103aec · f103aec
1 parent 5ce3171
commit f103aec
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 2 deletions.
diff --git a/tensorboard/webapp/app_routing/effects/app_routing_effects.ts b/tensorboard/webapp/app_routing/effects/app_routing_effects.ts
@@ -204,10 +204,18 @@ export class AppRoutingEffects {
    */
   navigate$ = createEffect(() => {
     const dispatchNavigating$ = this.validatedRoute$.pipe(
-      mergeMap((routes) => {
+      withLatestFrom(this.store.select(getActiveRoute)),
+      mergeMap(([routes, oldRoute]) => {
+        const sameRouteId =
+          oldRoute !== null &&
+          getRouteId(routes.routeMatch.routeKind, routes.routeMatch.params) ===
+            getRouteId(oldRoute.routeKind, oldRoute.params);
         const dirtySelectors = this.dirtyUpdatesRegistry.getDirtyUpdatesSelectors();
 
-        if (!dirtySelectors.length) return of(routes);
+        // Do not warn about dirty updates when route ID is the same (e.g. when
+        // changing tabs in the same experiment page or query params in experiment
+        // list).
+        if (sameRouteId || !dirtySelectors.length) return of(routes);
         return forkJoin(
           this.dirtyUpdatesRegistry
             .getDirtyUpdatesSelectors()

diff --git a/tensorboard/webapp/app_routing/effects/app_routing_effects_test.ts b/tensorboard/webapp/app_routing/effects/app_routing_effects_test.ts
@@ -334,6 +334,34 @@ describe('app_routing_effects', () => {
         );
       });
 
+      it('does not warn user when changing tab (same routeId)', fakeAsync(() => {
+        spyOn(window, 'confirm');
+        const activeRoute = buildRoute({
+          routeKind: RouteKind.EXPERIMENT,
+          params: {experimentId: 'meow'},
+          pathname: '/experiment/meow',
+          queryParams: [],
+        });
+        store.overrideSelector(getActiveRoute, activeRoute);
+        store.refreshState();
+        getPathSpy.and.returnValue('/experiment/meow');
+        // Changing tab.
+        getHashSpy.and.returnValue('#foo');
+        action.next(actions.navigationRequested(activeRoute));
+        tick();
+
+        expect(window.confirm).not.toHaveBeenCalled();
+        expect(actualActions).toEqual([
+          actions.navigating({
+            after: activeRoute,
+          }),
+          actions.navigated({
+            before: activeRoute,
+            after: activeRoute,
+          }),
+        ]);
+      }));
+
       it('noops if user cancels navigation', () => {
         spyOn(window, 'confirm').and.returnValue(false);
         action.next(