Repository: GoogleChromeLabs/ps-analysis-tool
Pull request #113 (2023-08-18 12:16:22)

Update popup loading state
## Description
This PR updates popup's `loading` state's timeout and adds loading text in UI.
<!-- What do we want to achieve with this PR? -->

## Relevant Technical Choices

<!-- Please describe your changes. -->

## Testing Instructions

<!--
How can the changes in this PR be verified?
Please provide step-by-step instructions to test the changes.
-->

## Additional Information:

<!-- Any other information. -->

## Screenshot/Screencast

<!-- Please provide Screenshot/Screencast, if applicable -->

https://github.com/GoogleChromeLabs/ps-analysis-tool/assets/58820001/74c9f554-662a-4c97-a533-6532f88d544b

https://github.com/GoogleChromeLabs/ps-analysis-tool/assets/58820001/a33acf30-2904-4a43-a0f5-c1f436ab04df



---

<!--
Example:

Fixes #123
Partially addresses #22
See #834
-->

Fixes #



Patch:
diff --git a/packages/extension/src/view/design-system/components/circlePieChart/emptyCirclePieChart.tsx b/packages/extension/src/view/design-system/components/circlePieChart/emptyCirclePieChart.tsx
index d238b39f..74ed4e06 100644
--- a/packages/extension/src/view/design-system/components/circlePieChart/emptyCirclePieChart.tsx
+++ b/packages/extension/src/view/design-system/components/circlePieChart/emptyCirclePieChart.tsx
@@ -18,7 +18,6 @@
  */
 import React from 'react';
 import { VictoryPie } from 'victory';
-import classNames from 'classnames';
 
 /**
  * Internal dependencies.
@@ -27,51 +26,21 @@ import { COLOR_MAP } from '../../theme/colors';
 
 interface EmptyCirclePieChartProps {
   fallbackText?: string;
-  title?: string;
-  isPrimary?: boolean;
 }
 
-const EmptyCirclePieChart = ({
-  fallbackText,
-  title,
-  isPrimary = true,
-}: EmptyCirclePieChartProps) => {
-  const containerWidthClass = isPrimary ? 'w-16' : 'w-8';
-  const centerTitleClasses = isPrimary
-    ? 'text-xs leading-4'
-    : 'text-[7px] leading-[7px]'; // Font size and line height are added as an exception to handle edge case.
-
+const EmptyCirclePieChart = ({ fallbackText }: EmptyCirclePieChartProps) => {
   return (
-    <>
-      <div
-        className={classNames(
-          'max-w-xs inline-block align-bottom',
-          containerWidthClass
-        )}
-      >
-        <div className="w-full h-full relative">
-          <VictoryPie
-            padding={0}
-            innerRadius={0}
-            colorScale={[COLOR_MAP.brightGray]}
-            data={[{ x: '', y: 100 }]}
-          />
-          <p
-            className={classNames(
-              'absolute top-1/2 left-1/2 -translate-x-1/2 -translate-y-1/2 text-center text-raisin-black opacity-40',
-              centerTitleClasses
-            )}
-          >
-            {fallbackText || 'Not Found'}
-          </p>
-        </div>
-      </div>
-      {title && (
-        <p className="text-xs text-center font-semibold mt-2 leading-relaxed dark:text-bright-gray">
-          {title}
-        </p>
-      )}
-    </>
+    <div className="w-full h-full relative">
+      <VictoryPie
+        padding={0}
+        innerRadius={0}
+        colorScale={[COLOR_MAP.brightGray]}
+        data={[{ x: '', y: 100 }]}
+      />
+      <p className="absolute top-1/2 left-1/2 -translate-x-1/2 -translate-y-1/2 text-center text-raisin-black opacity-40 text-xs leading-4'">
+        {fallbackText || 'Not Found'}
+      </p>
+    </div>
   );
 };
 
diff --git a/packages/extension/src/view/design-system/components/circlePieChart/index.tsx b/packages/extension/src/view/design-system/components/circlePieChart/index.tsx
index a5a1da3d..c288383c 100644
--- a/packages/extension/src/view/design-system/components/circlePieChart/index.tsx
+++ b/packages/extension/src/view/design-system/components/circlePieChart/index.tsx
@@ -30,7 +30,6 @@ interface CirclePieChartProps {
   centerCount: number;
   data: { count: number; color: string }[];
   title?: string;
-  isPrimary?: boolean;
   fallbackText?: string;
   infoIconClassName?: string;
 }
@@ -41,50 +40,36 @@ const CirclePieChart = ({
   centerCount,
   data,
   title,
-  isPrimary = true,
   fallbackText,
   infoIconClassName = '',
 }: CirclePieChartProps) => {
-  let centerTitleClasses = centerCount <= MAX_COUNT ? 'text-2xl' : 'text-l';
-  const containerWidthClass = isPrimary ? 'w-16' : 'w-8';
-
-  if (!isPrimary) {
-    centerTitleClasses = centerCount <= MAX_COUNT ? 'text-xs' : 'text-xxxs';
-  }
-
-  if (centerCount <= 0) {
-    return (
-      <EmptyCirclePieChart
-        title={title}
-        isPrimary={isPrimary}
-        fallbackText={fallbackText}
-      />
-    );
-  }
+  const centerTitleClasses = centerCount <= MAX_COUNT ? 'text-2xl' : 'text-l';
 
   return (
     <>
-      <div
-        className={classNames('inline-block align-bottom', containerWidthClass)}
-      >
-        <div className="w-full h-full relative">
-          <VictoryPie
-            padding={0}
-            innerRadius={175}
-            animate={{ duration: 400 }}
-            data={data.map(({ count }) => ({ x: '', y: count }))}
-            labels={() => ''}
-            colorScale={data.map(({ color }) => color)}
-          />
-          <p
-            className={classNames(
-              'absolute top-1/2 left-1/2 -translate-x-1/2 -translate-y-1/2 font-regular dark:text-bright-gray',
-              centerTitleClasses
-            )}
-          >
-            {centerCount <= MAX_COUNT ? centerCount : MAX_COUNT + '+'}
-          </p>
-        </div>
+      <div className="inline-block align-bottom w-16">
+        {centerCount <= 0 ? (
+          <EmptyCirclePieChart fallbackText={fallbackText} />
+        ) : (
+          <div className="w-full h-full relative">
+            <VictoryPie
+              padding={0}
+              innerRadius={175}
+              animate={{ duration: 400 }}
+              data={data.map(({ count }) => ({ x: '', y: count }))}
+              labels={() => ''}
+              colorScale={data.map(({ color }) => color)}
+            />
+            <p
+              className={classNames(
+                'absolute top-1/2 left-1/2 -translate-x-1/2 -translate-y-1/2 font-regular dark:text-bright-gray',
+                centerTitleClasses
+              )}
+            >
+              {centerCount <= MAX_COUNT ? centerCount : MAX_COUNT + '+'}
+            </p>
+          </div>
+        )}
       </div>
       {title && (
         <div className="flex items-center justify-center gap-1 mt-2 relative">
diff --git a/packages/extension/src/view/design-system/components/circlePieChart/stories/circlePieChart.stories.tsx b/packages/extension/src/view/design-system/components/circlePieChart/stories/circlePieChart.stories.tsx
index 81a182ef..4403a800 100644
--- a/packages/extension/src/view/design-system/components/circlePieChart/stories/circlePieChart.stories.tsx
+++ b/packages/extension/src/view/design-system/components/circlePieChart/stories/circlePieChart.stories.tsx
@@ -92,33 +92,3 @@ export const Wide: StoryObj<typeof meta> = {
     </div>
   ),
 };
-
-export const Secondary: StoryObj<typeof meta> = {
-  args: {
-    centerCount: 39,
-    isPrimary: false,
-    data: [
-      {
-        count: 5,
-        color: COLOR_MAP.functional,
-      },
-      {
-        count: 4,
-        color: COLOR_MAP.marketing,
-      },
-      {
-        count: 0,
-        color: COLOR_MAP.analytics,
-      },
-      {
-        count: 9,
-        color: COLOR_MAP.uncategorized,
-      },
-    ],
-  },
-  render: (args) => (
-    <div className="w-8 text-center">
-      <CirclePieChart {...args} />
-    </div>
-  ),
-};
diff --git a/packages/extension/src/view/design-system/components/circlePieChart/stories/emptyCirclePieChart.stories.tsx b/packages/extension/src/view/design-system/components/circlePieChart/stories/emptyCirclePieChart.stories.tsx
index 8b22e8f7..d640ac33 100644
--- a/packages/extension/src/view/design-system/components/circlePieChart/stories/emptyCirclePieChart.stories.tsx
+++ b/packages/extension/src/view/design-system/components/circlePieChart/stories/emptyCirclePieChart.stories.tsx
@@ -38,9 +38,7 @@ export const Primary: StoryObj<typeof meta> = {
       <EmptyCirclePieChart {...args} />
     </div>
   ),
-  args: {
-    title: '3rd party cookies',
-  },
+  args: {},
 };
 
 export const Wide: StoryObj<typeof meta> = {
@@ -49,18 +47,5 @@ export const Wide: StoryObj<typeof meta> = {
       <EmptyCirclePieChart {...args} />
     </div>
   ),
-  args: {
-    title: '3rd party cookies',
-  },
-};
-
-export const Secondary: StoryObj<typeof meta> = {
-  render: (args) => (
-    <div className="w-8 text-center">
-      <EmptyCirclePieChart {...args} />
-    </div>
-  ),
-  args: {
-    isPrimary: false,
-  },
+  args: {},
 };
diff --git a/packages/extension/src/view/popup/app.tsx b/packages/extension/src/view/popup/app.tsx
index fc94ab23..5e511965 100644
--- a/packages/extension/src/view/popup/app.tsx
+++ b/packages/extension/src/view/popup/app.tsx
@@ -29,36 +29,37 @@ import { CirclePieChart } from '../design-system/components';
 import { prepareCookieStatsComponents } from '../../utils/prepareCookieStatsComponents';
 
 const App: React.FC = () => {
-  const { cookieStats, loading } = useCookieStore(({ state }) => ({
-    cookieStats: state.tabCookieStats,
-    loading: state.loading,
-  }));
+  const { cookieStats, loading, showLoadingText } = useCookieStore(
+    ({ state }) => ({
+      cookieStats: state.tabCookieStats,
+      loading: state.loading,
+      showLoadingText: state.showLoadingText,
+    })
+  );
 
   if (loading) {
     return (
-      <div className="w-96 h-80 flex items-center justify-center">
+      <div className="w-96 min-h-[20rem] flex items-center justify-center flex-col gap-2 relative">
         <div className="w-10 h-10 rounded-full animate-spin border-t-transparent border-solid border-blue-700 border-4" />
-      </div>
-    );
-  }
-
-  if (!cookieStats) {
-    return (
-      <div className="w-96 h-fit p-5 flex justify-center items-center flex-col">
-        <p className="font-bold text-lg">
-          Please refresh this page to view cookies
-        </p>
+        {showLoadingText && (
+          <p className="absolute bottom-10 text-blue-700 text-lg ml-2">
+            Still listening to cookies, please wait...
+          </p>
+        )}
       </div>
     );
   }
 
   if (
-    cookieStats.firstParty.total === 0 &&
-    cookieStats.thirdParty.total === 0
+    !cookieStats ||
+    (cookieStats.firstParty.total === 0 && cookieStats.thirdParty.total === 0)
   ) {
     return (
-      <div className="w-96 h-fit p-5 flex justify-center items-center flex-col">
+      <div className="w-96 min-h-[318px] h-fit p-5 flex justify-center items-center flex-col">
         <p className="font-bold text-lg">No cookies found on this page</p>
+        <p className="text-chart-label text-xs">
+          Please try reloading the page
+        </p>
       </div>
     );
   }
@@ -66,7 +67,7 @@ const App: React.FC = () => {
   const statsComponents = prepareCookieStatsComponents(cookieStats);
 
   return (
-    <div className="w-96 h-fit p-5 flex justify-center items-center flex-col">
+    <div className="w-96 min-h-[318px] h-fit p-5 flex justify-center items-center flex-col">
       <div className="w-full flex gap-x-6 justify-center border-b border-hex-gray pb-3.5">
         <div className="w-32 text-center">
           <CirclePieChart
diff --git a/packages/extension/src/view/popup/stateProviders/syncCookieStore/index.tsx b/packages/extension/src/view/popup/stateProviders/syncCookieStore/index.tsx
index 4ef565b3..d9b5c05b 100644
--- a/packages/extension/src/view/popup/stateProviders/syncCookieStore/index.tsx
+++ b/packages/extension/src/view/popup/stateProviders/syncCookieStore/index.tsx
@@ -37,6 +37,7 @@ export interface CookieStoreContext {
   state: {
     tabCookieStats: CookiesCount | null;
     loading: boolean;
+    showLoadingText: boolean;
   };
   actions: object;
 }
@@ -61,6 +62,7 @@ const initialState: CookieStoreContext = {
       },
     },
     loading: true,
+    showLoadingText: false,
   },
   actions: {},
 };
@@ -77,16 +79,27 @@ export const Provider = ({ children }: PropsWithChildren) => {
 
   const [loading, setLoading] = useState<boolean>(true);
 
-  const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+  const [showLoadingText, setShowLoadingText] = useState<boolean>(false);
+
+  const loadingTimeout = useRef<ReturnType<typeof setTimeout> | null>(null);
+  const loadingTextTimeout = useRef<ReturnType<typeof setTimeout> | null>(null);
 
   useEffect(() => {
-    timeoutRef.current = setTimeout(() => {
+    loadingTimeout.current = setTimeout(() => {
       setLoading(false);
-    }, 10000);
+    }, 6500);
+
+    loadingTextTimeout.current = setTimeout(() => {
+      setShowLoadingText(true);
+    }, 2500);
 
     return () => {
-      if (timeoutRef.current) {
-        clearTimeout(timeoutRef.current);
+      if (loadingTimeout.current) {
+        clearTimeout(loadingTimeout.current);
+      }
+
+      if (loadingTextTimeout.current) {
+        clearTimeout(loadingTextTimeout.current);
       }
     };
   }, []);
@@ -94,6 +107,7 @@ export const Provider = ({ children }: PropsWithChildren) => {
   const setDebouncedStats = useDebouncedCallback((value) => {
     setTabCookieStats(value);
     setLoading(false);
+    setShowLoadingText(false);
   }, 100);
 
   const intitialSync = useCallback(async () => {
@@ -156,7 +170,10 @@ export const Provider = ({ children }: PropsWithChildren) => {
 
   return (
     <Context.Provider
-      value={{ state: { tabCookieStats, loading }, actions: {} }}
+      value={{
+        state: { tabCookieStats, loading, showLoadingText },
+        actions: {},
+      }}
     >
       {children}
     </Context.Provider>


Test patch:
diff --git a/packages/extension/src/view/design-system/components/circlePieChart/tests/circlePieChart.tsx b/packages/extension/src/view/design-system/components/circlePieChart/tests/circlePieChart.tsx
index ce155823..8662aca6 100644
--- a/packages/extension/src/view/design-system/components/circlePieChart/tests/circlePieChart.tsx
+++ b/packages/extension/src/view/design-system/components/circlePieChart/tests/circlePieChart.tsx
@@ -104,46 +104,4 @@ describe('CirclePieChart', () => {
     const primaryContainer = container.querySelector('.w-16');
     expect(primaryContainer).toBeInTheDocument();
   });
-
-  it('renders CirclePieChart with custom container width class when isPrimary is false', () => {
-    const centerCount = 60;
-
-    const { container } = render(
-      <CirclePieChart
-        centerCount={centerCount}
-        data={testData}
-        isPrimary={false}
-      />
-    );
-
-    // Check if the CirclePieChart renders with the custom container width class for non-primary charts
-    const secondaryContainer = container.querySelector('.w-8');
-    expect(secondaryContainer).toBeInTheDocument();
-  });
-
-  it('renders CirclePieChart with the correct font size class for primary and non-primary charts', () => {
-    const primaryCenterCount = 100;
-    const secondaryCenterCount = MAX_COUNT + 1; // Exceeds MAX_COUNT (999)
-
-    const { getByText, rerender } = render(
-      <CirclePieChart centerCount={primaryCenterCount} data={testData} />
-    );
-
-    // Check if the CirclePieChart renders with the correct font size class for primary charts
-    const primaryFontSizeClass = getByText(primaryCenterCount.toString());
-    expect(primaryFontSizeClass).toHaveClass('text-2xl');
-
-    // Rerender with non-primary centerCount
-    rerender(
-      <CirclePieChart
-        centerCount={secondaryCenterCount}
-        data={testData}
-        isPrimary={false}
-      />
-    );
-
-    // Check if the CirclePieChart renders with the correct font size class for non-primary charts
-    const nonPrimaryFontSizeClass = getByText(MAX_COUNT + '+');
-    expect(nonPrimaryFontSizeClass).toHaveClass('text-xxxs');
-  });
 });
diff --git a/packages/extension/src/view/popup/tests/index.tsx b/packages/extension/src/view/popup/tests/index.tsx
index 6cdf32c9..8f1e2e5a 100644
--- a/packages/extension/src/view/popup/tests/index.tsx
+++ b/packages/extension/src/view/popup/tests/index.tsx
@@ -57,7 +57,7 @@ describe('App', () => {
     render(<App />);
 
     expect(
-      screen.getByText('Please refresh this page to view cookies')
+      screen.getByText('Please try reloading the page')
     ).toBeInTheDocument();
   });