[dagit] Invert stored state for expand/collapse in Overview pages (#1…

…0212)
dagster-io · Oct 26, 2022 · 16bc4cf · 16bc4cf
1 parent b5212c7
commit 16bc4cf
Show file tree

Hide file tree

Showing 7 changed files with 227 additions and 19 deletions.
diff --git a/js_modules/dagit/packages/core/src/overview/OverviewExpansionKey.tsx b/js_modules/dagit/packages/core/src/overview/OverviewExpansionKey.tsx
@@ -1 +1,3 @@
+// todo dish: Delete `OVERVIEW_EXPANSION_KEY` in favor of `OVERVIEW_COLLAPSED_KEY`.
 export const OVERVIEW_EXPANSION_KEY = 'virtualized-expansion-state';
+export const OVERVIEW_COLLAPSED_KEY = 'overview-collapsed-state';
diff --git a/js_modules/dagit/packages/core/src/overview/OverviewJobsTable.tsx b/js_modules/dagit/packages/core/src/overview/OverviewJobsTable.tsx
@@ -10,7 +10,7 @@ import {RepoRow} from '../workspace/VirtualizedWorkspaceTable';
 import {repoAddressAsString} from '../workspace/repoAddressAsString';
 import {RepoAddress} from '../workspace/types';
 
-import {OVERVIEW_EXPANSION_KEY} from './OverviewExpansionKey';
+import {OVERVIEW_COLLAPSED_KEY, OVERVIEW_EXPANSION_KEY} from './OverviewExpansionKey';
 
 type Repository = {
   repoAddress: RepoAddress;
@@ -37,6 +37,7 @@ export const OverviewJobsTable: React.FC<Props> = ({repos}) => {
 
   const {expandedKeys, onToggle, onToggleAll} = useRepoExpansionState(
     OVERVIEW_EXPANSION_KEY,
+    OVERVIEW_COLLAPSED_KEY,
     allKeys,
   );
 

diff --git a/js_modules/dagit/packages/core/src/overview/OverviewSchedulesTable.tsx b/js_modules/dagit/packages/core/src/overview/OverviewSchedulesTable.tsx
@@ -13,7 +13,7 @@ import {RepoRow} from '../workspace/VirtualizedWorkspaceTable';
 import {repoAddressAsString} from '../workspace/repoAddressAsString';
 import {RepoAddress} from '../workspace/types';
 
-import {OVERVIEW_EXPANSION_KEY} from './OverviewExpansionKey';
+import {OVERVIEW_COLLAPSED_KEY, OVERVIEW_EXPANSION_KEY} from './OverviewExpansionKey';
 
 type Repository = {
   repoAddress: RepoAddress;
@@ -36,6 +36,7 @@ export const OverviewScheduleTable: React.FC<Props> = ({repos}) => {
   );
   const {expandedKeys, onToggle, onToggleAll} = useRepoExpansionState(
     OVERVIEW_EXPANSION_KEY,
+    OVERVIEW_COLLAPSED_KEY,
     allKeys,
   );
 

diff --git a/js_modules/dagit/packages/core/src/overview/OverviewSensorsTable.tsx b/js_modules/dagit/packages/core/src/overview/OverviewSensorsTable.tsx
@@ -10,7 +10,7 @@ import {RepoRow} from '../workspace/VirtualizedWorkspaceTable';
 import {repoAddressAsString} from '../workspace/repoAddressAsString';
 import {RepoAddress} from '../workspace/types';
 
-import {OVERVIEW_EXPANSION_KEY} from './OverviewExpansionKey';
+import {OVERVIEW_COLLAPSED_KEY, OVERVIEW_EXPANSION_KEY} from './OverviewExpansionKey';
 
 type Repository = {
   repoAddress: RepoAddress;
@@ -33,6 +33,7 @@ export const OverviewSensorTable: React.FC<Props> = ({repos}) => {
   );
   const {expandedKeys, onToggle, onToggleAll} = useRepoExpansionState(
     OVERVIEW_EXPANSION_KEY,
+    OVERVIEW_COLLAPSED_KEY,
     allKeys,
   );
 

diff --git a/js_modules/dagit/packages/core/src/runs/RunTimeline.tsx b/js_modules/dagit/packages/core/src/runs/RunTimeline.tsx
@@ -18,7 +18,7 @@ import styled from 'styled-components/macro';
 import {useFeatureFlags} from '../app/Flags';
 import {TimezoneContext} from '../app/time/TimezoneContext';
 import {browserTimezone} from '../app/time/browserTimezone';
-import {OVERVIEW_EXPANSION_KEY} from '../overview/OverviewExpansionKey';
+import {OVERVIEW_COLLAPSED_KEY, OVERVIEW_EXPANSION_KEY} from '../overview/OverviewExpansionKey';
 import {TimestampDisplay} from '../schedules/TimestampDisplay';
 import {RunStatus} from '../types/globalTypes';
 import {AnchorButton} from '../ui/AnchorButton';
@@ -85,6 +85,7 @@ export const RunTimeline = (props: Props) => {
   const allKeys = Object.keys(buckets);
   const {expandedKeys, onToggle, onToggleAll} = useRepoExpansionState(
     OVERVIEW_EXPANSION_KEY,
+    OVERVIEW_COLLAPSED_KEY,
     allKeys,
   );
 

diff --git a/js_modules/dagit/packages/core/src/ui/useRepoExpansionState.test.tsx b/js_modules/dagit/packages/core/src/ui/useRepoExpansionState.test.tsx
@@ -0,0 +1,165 @@
+import {act, render, screen} from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import * as React from 'react';
+
+import {TestProvider} from '../testing/TestProvider';
+import {repoAddressFromPath} from '../workspace/repoAddressFromPath';
+
+import {buildStorageKey, useRepoExpansionState} from './useRepoExpansionState';
+
+const TEMP_EXPANDED_STORAGE_KEY = 'temp-expanded-key';
+const COLLAPSED_STORAGE_KEY = 'collapsed-key';
+const ALL_REPO_KEYS = ['lorem@ipsum', 'dolorsit@amet', 'consectetur@adipiscing'];
+
+describe('useRepoExpansionState', () => {
+  const Test = () => {
+    const {expandedKeys, onToggle, onToggleAll} = useRepoExpansionState(
+      TEMP_EXPANDED_STORAGE_KEY,
+      COLLAPSED_STORAGE_KEY,
+      ALL_REPO_KEYS,
+    );
+
+    return (
+      <TestProvider>
+        <div>
+          {ALL_REPO_KEYS.map((key) => (
+            <div key={key}>
+              <div>{`${key} ${expandedKeys.includes(key) ? 'expanded' : 'collapsed'}`}</div>
+              <button
+                onClick={() => {
+                  const repoAddress = repoAddressFromPath(key);
+                  if (repoAddress) {
+                    onToggle(repoAddress);
+                  }
+                }}
+              >
+                {`toggle ${key}`}
+              </button>
+            </div>
+          ))}
+          <button onClick={() => onToggleAll(true)}>expand all</button>
+          <button onClick={() => onToggleAll(false)}>collapse all</button>
+        </div>
+      </TestProvider>
+    );
+  };
+
+  beforeEach(() => {
+    window.localStorage.clear();
+  });
+
+  it('provides a list of expanded keys for the stored state', async () => {
+    window.localStorage.setItem(buildStorageKey('', COLLAPSED_STORAGE_KEY), JSON.stringify([]));
+    await act(async () => {
+      render(<Test />);
+    });
+
+    // Expect all keys to be expanded
+    expect(screen.getByText('lorem@ipsum expanded')).toBeVisible();
+    expect(screen.getByText('dolorsit@amet expanded')).toBeVisible();
+    expect(screen.getByText('consectetur@adipiscing expanded')).toBeVisible();
+  });
+
+  it('tracks collapsed keys', async () => {
+    window.localStorage.setItem(
+      buildStorageKey('', COLLAPSED_STORAGE_KEY),
+      JSON.stringify(['lorem@ipsum']),
+    );
+    await act(async () => {
+      render(<Test />);
+    });
+
+    // Expect keys to have appropriate state. One collapsed!
+    expect(screen.getByText('lorem@ipsum collapsed')).toBeVisible();
+    expect(screen.getByText('dolorsit@amet expanded')).toBeVisible();
+    expect(screen.getByText('consectetur@adipiscing expanded')).toBeVisible();
+  });
+
+  it('toggles a key to expanded', async () => {
+    const fullCollapsedKey = buildStorageKey('', COLLAPSED_STORAGE_KEY);
+    window.localStorage.setItem(fullCollapsedKey, JSON.stringify(['lorem@ipsum']));
+    await act(async () => {
+      render(<Test />);
+    });
+
+    const button = screen.getByRole('button', {name: 'toggle lorem@ipsum'});
+    await act(async () => {
+      userEvent.click(button);
+    });
+
+    expect(screen.getByText('lorem@ipsum expanded')).toBeVisible();
+    expect(window.localStorage.getItem(fullCollapsedKey)).toEqual('[]');
+  });
+
+  it('toggles a key to collapsed', async () => {
+    const fullCollapsedKey = buildStorageKey('', COLLAPSED_STORAGE_KEY);
+    window.localStorage.setItem(fullCollapsedKey, JSON.stringify([]));
+    await act(async () => {
+      render(<Test />);
+    });
+
+    const button = screen.getByRole('button', {name: 'toggle lorem@ipsum'});
+    await act(async () => {
+      userEvent.click(button);
+    });
+
+    expect(screen.getByText('lorem@ipsum collapsed')).toBeVisible();
+    expect(window.localStorage.getItem(fullCollapsedKey)).toEqual(JSON.stringify(['lorem@ipsum']));
+  });
+
+  it('toggles all to expanded', async () => {
+    const fullCollapsedKey = buildStorageKey('', COLLAPSED_STORAGE_KEY);
+    window.localStorage.setItem(fullCollapsedKey, JSON.stringify(['lorem@ipsum', 'dolorsit@amet']));
+    await act(async () => {
+      render(<Test />);
+    });
+
+    const button = screen.getByRole('button', {name: 'expand all'});
+    await act(async () => {
+      userEvent.click(button);
+    });
+
+    // Everything expanded!
+    expect(screen.getByText('lorem@ipsum expanded')).toBeVisible();
+    expect(screen.getByText('dolorsit@amet expanded')).toBeVisible();
+    expect(screen.getByText('consectetur@adipiscing expanded')).toBeVisible();
+  });
+
+  it('toggles all to collapsed', async () => {
+    const fullCollapsedKey = buildStorageKey('', COLLAPSED_STORAGE_KEY);
+    window.localStorage.setItem(fullCollapsedKey, JSON.stringify(['lorem@ipsum']));
+    await act(async () => {
+      render(<Test />);
+    });
+
+    const button = screen.getByRole('button', {name: 'collapse all'});
+    await act(async () => {
+      userEvent.click(button);
+    });
+
+    // Everything collapsed!
+    expect(screen.getByText('lorem@ipsum collapsed')).toBeVisible();
+    expect(screen.getByText('dolorsit@amet collapsed')).toBeVisible();
+    expect(screen.getByText('consectetur@adipiscing collapsed')).toBeVisible();
+  });
+
+  // Temporary! Todo dish: Delete in November 2022.
+  it('must convert "expanded" state to "collapsed" state if no stored collapsed state', async () => {
+    const fullExpandedKey = buildStorageKey('', TEMP_EXPANDED_STORAGE_KEY);
+    const fullCollapsedKey = buildStorageKey('', COLLAPSED_STORAGE_KEY);
+
+    window.localStorage.setItem(fullExpandedKey, JSON.stringify(['lorem@ipsum', 'dolorsit@amet']));
+    await act(async () => {
+      render(<Test />);
+    });
+
+    expect(window.localStorage.getItem(fullCollapsedKey)).toEqual(
+      JSON.stringify(['consectetur@adipiscing']),
+    );
+
+    // Expanded/collapsed state matches stored state.
+    expect(screen.getByText('lorem@ipsum expanded')).toBeVisible();
+    expect(screen.getByText('dolorsit@amet expanded')).toBeVisible();
+    expect(screen.getByText('consectetur@adipiscing collapsed')).toBeVisible();
+  });
+});
diff --git a/js_modules/dagit/packages/core/src/ui/useRepoExpansionState.tsx b/js_modules/dagit/packages/core/src/ui/useRepoExpansionState.tsx
@@ -6,47 +6,84 @@ import {repoAddressAsString} from '../workspace/repoAddressAsString';
 import {RepoAddress} from '../workspace/types';
 
 const validateExpandedKeys = (parsed: unknown) => (Array.isArray(parsed) ? parsed : []);
+export const buildStorageKey = (basePath: string, key: string) => `${basePath}:dagit.${key}`;
 
 /**
  * Use localStorage to persist the expanded/collapsed visual state of repository containers,
  * e.g. for the left nav or run timeline.
  */
-export const useRepoExpansionState = (storageKey: string, allKeys: string[]) => {
+export const useRepoExpansionState = (
+  // todo dish: Delete this arg.
+  expandedKey: string,
+  collapsedKey: string,
+  allKeys: string[],
+) => {
   const {basePath} = React.useContext(AppContext);
-  const [expandedKeys, setExpandedKeys] = useStateWithStorage<string[]>(
-    `${basePath}:dagit.${storageKey}`,
+
+  // todo dish: Delete this.
+  const expandedStorageKey = buildStorageKey(basePath, expandedKey);
+  const [expandedKeysDeleteMe] = useStateWithStorage<string[]>(
+    expandedStorageKey,
+    validateExpandedKeys,
+  );
+
+  const collapsedStorageKey = buildStorageKey(basePath, collapsedKey);
+  const [collapsedKeys, setCollapsedKeys] = useStateWithStorage<string[]>(
+    collapsedStorageKey,
     validateExpandedKeys,
   );
 
+  const needsInversion =
+    window.localStorage.getItem(expandedStorageKey) !== null &&
+    window.localStorage.getItem(collapsedStorageKey) === null;
+
+  /**
+   * Temporary: If the user has been using this feature already, they have been tracking
+   * "expanded" state instead of "collapsed" state. Immediately invert the tracked state,
+   * using the current "expanded" keys and the full list of keys.
+   *
+   * todo dish: Delete this effect at some point in November 2022.
+   */
+  React.useEffect(() => {
+    if (needsInversion) {
+      setCollapsedKeys(() => allKeys.filter((key) => !expandedKeysDeleteMe.includes(key)));
+    }
+  });
+
   const onToggle = React.useCallback(
     (repoAddress: RepoAddress) => {
       const key = repoAddressAsString(repoAddress);
-      setExpandedKeys((current) => {
-        const nextExpandedKeys = new Set(current || []);
-        if (nextExpandedKeys.has(key)) {
-          nextExpandedKeys.delete(key);
+      setCollapsedKeys((current) => {
+        const nextCollapsedKeys = new Set(current || []);
+        if (nextCollapsedKeys.has(key)) {
+          nextCollapsedKeys.delete(key);
         } else {
-          nextExpandedKeys.add(key);
+          nextCollapsedKeys.add(key);
         }
-        return Array.from(nextExpandedKeys);
+        return Array.from(nextCollapsedKeys);
       });
     },
-    [setExpandedKeys],
+    [setCollapsedKeys],
   );
 
   const onToggleAll = React.useCallback(
     (expand: boolean) => {
-      setExpandedKeys((current) => {
-        const nextExpandedKeys = new Set(current || []);
+      setCollapsedKeys((current) => {
+        const nextCollapsedKeys = new Set(current || []);
         allKeys.forEach((key) => {
-          expand ? nextExpandedKeys.add(key) : nextExpandedKeys.delete(key);
+          expand ? nextCollapsedKeys.delete(key) : nextCollapsedKeys.add(key);
         });
-        return Array.from(nextExpandedKeys);
+        return Array.from(nextCollapsedKeys);
       });
     },
-    [allKeys, setExpandedKeys],
+    [allKeys, setCollapsedKeys],
   );
 
+  const expandedKeys = React.useMemo(() => {
+    const collapsedSet = new Set(collapsedKeys);
+    return allKeys.filter((key) => !collapsedSet.has(key));
+  }, [allKeys, collapsedKeys]);
+
   return React.useMemo(
     () => ({
       expandedKeys,