Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: Abstract Video Display Logic into Custom Hook #2013

Open
coderabbitai bot opened this issue Mar 20, 2024 · 2 comments
Open

Refactor: Abstract Video Display Logic into Custom Hook #2013

coderabbitai bot opened this issue Mar 20, 2024 · 2 comments

Comments

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 20, 2024

In the context of PR #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: #1967 (comment)

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

@mohitb35
Copy link
Collaborator

#1967 (comment) - for reference

@mohitb35
Copy link
Collaborator

mohitb35 commented Mar 21, 2024

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;  
    };  

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant