Fixed event listeners not being detached correctly

- Improved preloader component performance. - Updated app dependencies.
Sandakan · Nov 7, 2024 · d6575a2 · d6575a2
1 parent 9c7304a
commit d6575a2
Show file tree

Hide file tree

Showing 9 changed files with 180 additions and 148 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx
@@ -265,11 +265,12 @@ export default function App() {
   useEffect(
     () => {
       // check for app updates on app startup after 5 seconds.
-      setTimeout(checkForAppUpdates, 5000);
+      const timeoutId = setTimeout(checkForAppUpdates, 5000);
       // checks for app updates every 10 minutes.
-      const id = setInterval(checkForAppUpdates, 1000 * 60 * 15);
+      const intervalId = setInterval(checkForAppUpdates, 1000 * 60 * 15);
       return () => {
-        clearInterval(id);
+        clearTimeout(timeoutId);
+        clearInterval(intervalId);
       };
     },
     // eslint-disable-next-line react-hooks/exhaustive-deps
@@ -321,21 +322,25 @@ export default function App() {
   }, []);
 
   useEffect(() => {
-    player.addEventListener('error', (err) => managePlaybackErrors(err));
-    player.addEventListener('play', () => {
+    const handlePlayerErrorEvent = (err: unknown) => managePlaybackErrors(err);
+    const handlePlayerPlayEvent = () => {
       dispatch({
         type: 'CURRENT_SONG_PLAYBACK_STATE',
         data: true
       });
       window.api.playerControls.songPlaybackStateChange(true);
-    });
-    player.addEventListener('pause', () => {
+    };
+    const handlePlayerPauseEvent = () => {
       dispatch({
         type: 'CURRENT_SONG_PLAYBACK_STATE',
         data: false
       });
       window.api.playerControls.songPlaybackStateChange(false);
-    });
+    };
+
+    player.addEventListener('error', handlePlayerErrorEvent);
+    player.addEventListener('play', handlePlayerPlayEvent);
+    player.addEventListener('pause', handlePlayerPauseEvent);
     window.api.quitEvent.beforeQuitEvent(handleBeforeQuitEvent);
 
     window.api.windowControls.onWindowBlur(() => manageWindowBlurOrFocus('blur'));
@@ -345,10 +350,14 @@ export default function App() {
     window.api.fullscreen.onLeaveFullscreen(() => manageWindowFullscreen('windowed'));
 
     return () => {
+      player.removeEventListener('error', handlePlayerErrorEvent);
+      player.removeEventListener('play', handlePlayerPlayEvent);
+      player.removeEventListener('pause', handlePlayerPauseEvent);
       window.api.quitEvent.removeBeforeQuitEventListener(handleBeforeQuitEvent);
     };
+
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, []);
+  }, [manageWindowBlurOrFocus, manageWindowFullscreen, handleBeforeQuitEvent]);
 
   useEffect(() => {
     const displayDefaultTitleBar = () => {
@@ -491,7 +500,7 @@ export default function App() {
     }
 
     return () => {
-      // document.removeEventListener('localStorage', syncLocalStorage);
+      document.removeEventListener('localStorage', syncLocalStorage);
     };
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);

diff --git a/src/renderer/src/components/AlbumsPage/AlbumsPage.tsx b/src/renderer/src/components/AlbumsPage/AlbumsPage.tsx
@@ -149,7 +149,7 @@ const AlbumsPage = () => {
               data={albumsData}
               fixedItemWidth={MIN_ITEM_WIDTH}
               fixedItemHeight={MIN_ITEM_HEIGHT}
-              scrollTopOffset={currentlyActivePage.data?.scrollTopOffset}
+              // scrollTopOffset={currentlyActivePage.data?.scrollTopOffset}
               itemContent={(index, item) => {
                 return (
                   <Album

diff --git a/src/renderer/src/components/GenresPage/GenresPage.tsx b/src/renderer/src/components/GenresPage/GenresPage.tsx
@@ -152,7 +152,7 @@ const GenresPage = () => {
               data={genresData}
               fixedItemWidth={MIN_ITEM_WIDTH}
               fixedItemHeight={MIN_ITEM_HEIGHT}
-              scrollTopOffset={currentlyActivePage.data?.scrollTopOffset}
+              // scrollTopOffset={currentlyActivePage.data?.scrollTopOffset}
               itemContent={(index, genre) => {
                 return (
                   <Genre

diff --git a/src/renderer/src/components/LyricsPage/LyricsPage.tsx b/src/renderer/src/components/LyricsPage/LyricsPage.tsx
@@ -30,9 +30,9 @@ export const syncedLyricsRegex = /^\[\d+:\d{1,2}\.\d{1,3}]/gm;
 // export const delay = 0.35;
 
 let isScrollingByCode = false;
-document.addEventListener('lyrics/scrollIntoView', () => {
-  isScrollingByCode = true;
-});
+// document.addEventListener('lyrics/scrollIntoView', () => {
+//   isScrollingByCode = true;
+// });
 
 const LyricsPage = () => {
   const preferences = useStore(store, (state) => state.localStorage.preferences);

diff --git a/src/renderer/src/components/NotificationPanel/Notification.tsx b/src/renderer/src/components/NotificationPanel/Notification.tsx
@@ -35,8 +35,11 @@ const Notification = (props: AppNotification) => {
 
     if (notificationRef.current && !isNotificationAnimationDisabled) {
       notificationRef.current.classList.add('disappear-to-bottom');
-      notificationRef.current.addEventListener('animationend', () =>
-        updateNotifications((currNotifications) => currNotifications.filter((x) => x.id !== id))
+      notificationRef.current.addEventListener(
+        'animationend',
+        () =>
+          updateNotifications((currNotifications) => currNotifications.filter((x) => x.id !== id)),
+        { once: true }
       );
     } else updateNotifications((currNotifications) => currNotifications.filter((x) => x.id !== id));
   }, [id, preferences?.isReducedMotion, type, updateNotifications]);

diff --git a/src/renderer/src/components/PlaylistsPage/PlaylistsPage.tsx b/src/renderer/src/components/PlaylistsPage/PlaylistsPage.tsx
@@ -207,7 +207,7 @@ const PlaylistsPage = () => {
               data={playlists}
               fixedItemWidth={MIN_ITEM_WIDTH}
               fixedItemHeight={MIN_ITEM_HEIGHT}
-              scrollTopOffset={currentlyActivePage.data?.scrollTopOffset}
+              // scrollTopOffset={currentlyActivePage.data?.scrollTopOffset}
               itemContent={(index, playlist) => {
                 return <Playlist index={index} selectAllHandler={selectAllHandler} {...playlist} />;
               }}

diff --git a/src/renderer/src/components/Preloader/Preloader.tsx b/src/renderer/src/components/Preloader/Preloader.tsx
@@ -10,31 +10,47 @@ let isLoadedOnce = false;
 
 const hidePreloader = () => {
   const preloader = document.querySelector('.preloader');
-  if (preloader) {
+  const isHidable = preloader && !preloader.classList.contains('!invisible');
+  console.warn('hide preloader requested', {
+    time: window.performance.now() - contentLoadStart,
+    state: document.readyState,
+    isHidable
+  });
+
+  if (isHidable) {
     preloader.classList.add('!invisible', '!opacity-0');
+    console.warn(
+      'preloader hidden',
+      window.performance.now() - contentLoadStart,
+      document.readyState
+    );
   }
 };
 
-const eventHandler = () => {
-  setTimeout(hidePreloader, 1000);
-
-  console.warn('contentLoad', window.performance.now() - contentLoadStart);
-};
+window.addEventListener(
+  'load',
+  () => {
+    setTimeout(hidePreloader, 1000);
 
-window.addEventListener('load', eventHandler);
+    console.warn('contentLoad', window.performance.now() - contentLoadStart, document.readyState);
+  },
+  { once: true }
+);
 
 const Preloader = () => {
   const [isPreloaderRemoved, setIsPreloaderRemoved] = useState(false);
 
   useEffect(() => {
-    // console.log(window.performance.now());
-    if (window.performance.now() > 3000) hidePreloader();
-    // this removes preloader in 10 seconds no matter what didn't load.
-    setTimeout(() => {
+    // this removes preloader in 5 seconds no matter what didn't load.
+    const timeoutId = setTimeout(() => {
       hidePreloader();
       isLoadedOnce = true;
       setIsPreloaderRemoved(true);
-    }, 3000);
+    }, 5000);
+
+    return () => {
+      clearTimeout(timeoutId);
+    };
   }, []);
 
   return (

diff --git a/src/renderer/src/hooks/useNetworkConnectivity.tsx b/src/renderer/src/hooks/useNetworkConnectivity.tsx
@@ -5,13 +5,16 @@ const useNetworkConnectivity = () => {
 
   useEffect(() => {
     const controller = new AbortController();
+
     setIsOnline(navigator.onLine);
+
     window.addEventListener('online', () => setIsOnline(true), {
       signal: controller.signal
     });
     window.addEventListener('offline', () => setIsOnline(false), {
       signal: controller.signal
     });
+
     return () => controller.abort();
   }, []);