planet-webapp icon indicating copy to clipboard operation
planet-webapp copied to clipboard

Refactor: Abstract Video Display Logic into Custom Hook

Open coderabbitai[bot] opened this issue 1 year ago • 2 comments
trafficstars

In the context of PR https://github.com/Plant-for-the-Planet-org/planet-webapp/pull/1967, a suggestion was made to abstract the logic for showing or hiding the landing video based on the URL path and local storage settings into a custom hook or utility function. This abstraction aims to improve readability and maintainability of the code.

The specific suggestion can be found in this comment: https://github.com/Plant-for-the-Planet-org/planet-webapp/pull/1967#discussion_r1531543994

Creating this issue to track the implementation of this suggestion in the future.

coderabbitai[bot] avatar Mar 20 '24 06:03 coderabbitai[bot]

https://github.com/Plant-for-the-Planet-org/planet-webapp/pull/1967#discussion_r1531739480 - for reference

mohitb35 avatar Mar 21 '24 11:03 mohitb35

Comment from coderabbit: The logic for showing or hiding the landing video based on the URL path and local storage settings has been preserved from the previous version. While this logic is functional, consider abstracting it into a custom hook or utility function to improve readability and maintainability, as suggested in the previous comments. This would not only clean up the component but also make this piece of logic reusable across the application.

+ const useShowVideo = (router) => {  
+   const [showVideo, setShowVideo] = React.useState(false);  
+  
+   React.useEffect(() => {  
+     const pathsWithVideo = ['/', '/sites/[slug]', '/sites/[slug]/[locale]'];  
+     const show = pathsWithVideo.includes(router.pathname) && typeof window !== 'undefined' && localStorage.getItem('showVideo') !== 'false';  
+     setShowVideo(show);  
+   }, [router.pathname]);  
+  
+   return showVideo;  
+ };  
- // Existing logic for showing/hiding video  

Suggested change:

const useShowVideo = (router) => {  
      const [showVideo, setShowVideo] = React.useState(false);  

      React.useEffect(() => {  
        const pathsWithVideo = ['/', '/sites/[slug]', '/sites/[slug]/[locale]'];  
        const show = pathsWithVideo.includes(router.pathname) && typeof window !== 'undefined' && localStorage.getItem('showVideo') !== 'false';  
        setShowVideo(show);  
      }, [router.pathname]);  

      return showVideo;  
    };  

mohitb35 avatar Mar 21 '24 11:03 mohitb35