From 33e5e74228dd013137589348cb887d448745c8be Mon Sep 17 00:00:00 2001 From: jerempy <95110820+jerempy@users.noreply.github.com> Date: Wed, 4 Oct 2023 13:12:52 -0400 Subject: [PATCH 1/2] custom hook --- frontend/src/App.tsx | 13 ++--- frontend/src/Hero.tsx | 4 +- frontend/src/Navigation.tsx | 60 +++++++++++------------- frontend/src/hooks.ts | 19 -------- frontend/src/hooks/index.test.ts | 20 ++++++++ frontend/src/hooks/index.ts | 45 ++++++++++++++++++ frontend/src/preferences/APIKeyModal.tsx | 8 ++-- 7 files changed, 102 insertions(+), 67 deletions(-) delete mode 100644 frontend/src/hooks.ts create mode 100644 frontend/src/hooks/index.test.ts create mode 100644 frontend/src/hooks/index.ts diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 8852d4315..29f2bd2b4 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -2,24 +2,19 @@ import { Routes, Route } from 'react-router-dom'; import Navigation from './Navigation'; import Conversation from './conversation/Conversation'; import About from './About'; -import { useState } from 'react'; -import { ActiveState } from './models/misc'; import { inject } from '@vercel/analytics'; +import { useMediaQuery } from './hooks'; inject(); export default function App() { - //TODO : below media query is disjoint from tailwind. Please wire it together. - const [navState, setNavState] = useState( - window.matchMedia('(min-width: 768px)').matches ? 'ACTIVE' : 'INACTIVE', - ); - + const { isMobile } = useMediaQuery(); return (
- +
diff --git a/frontend/src/Hero.tsx b/frontend/src/Hero.tsx index b187e0a71..1552887c3 100644 --- a/frontend/src/Hero.tsx +++ b/frontend/src/Hero.tsx @@ -10,8 +10,8 @@ export default function Hero({ className = '' }: { className?: string }) {

Enter a query related to the information in the documentation you - selected to receive and we will provide you with the most relevant - answers. + selected to receive +
and we will provide you with the most relevant answers.

Start by entering your query in the input field below and we will do the diff --git a/frontend/src/Navigation.tsx b/frontend/src/Navigation.tsx index 69e659400..cb5d02185 100644 --- a/frontend/src/Navigation.tsx +++ b/frontend/src/Navigation.tsx @@ -26,23 +26,19 @@ import { setConversation, updateConversationId, } from './conversation/conversationSlice'; -import { useOutsideAlerter } from './hooks'; +import { useMediaQuery, useOutsideAlerter } from './hooks'; import Upload from './upload/Upload'; import { Doc, getConversations } from './preferences/preferenceApi'; import SelectDocsModal from './preferences/SelectDocsModal'; -export default function Navigation({ - navState, - setNavState, -}: { - navState: ActiveState; - setNavState: React.Dispatch>; -}) { +export default function Navigation() { const dispatch = useDispatch(); const docs = useSelector(selectSourceDocs); const selectedDocs = useSelector(selectSelectedDocs); const conversations = useSelector(selectConversations); const conversationId = useSelector(selectConversationId); + const { isMobile } = useMediaQuery(); + const [navOpen, setNavOpen] = useState(!isMobile); const [isDocsListOpen, setIsDocsListOpen] = useState(false); @@ -59,7 +55,8 @@ export default function Navigation({ const navRef = useRef(null); const apiHost = import.meta.env.VITE_API_HOST || 'https://docsapi.arc53.com'; - const embeddingsName = import.meta.env.VITE_EMBEDDINGS_NAME || 'openai_text-embedding-ada-002'; + const embeddingsName = + import.meta.env.VITE_EMBEDDINGS_NAME || 'openai_text-embedding-ada-002'; useEffect(() => { if (!conversations) { @@ -123,51 +120,46 @@ export default function Navigation({ useOutsideAlerter( navRef, () => { - if ( - window.matchMedia('(max-width: 768px)').matches && - navState === 'ACTIVE' && - apiKeyModalState === 'INACTIVE' - ) { - setNavState('INACTIVE'); + if (isMobile && navOpen && apiKeyModalState === 'INACTIVE') { + setNavOpen(false); setIsDocsListOpen(false); } }, - [navState, isDocsListOpen, apiKeyModalState], + [navOpen, isDocsListOpen, apiKeyModalState], ); /* Needed to fix bug where if mobile nav was closed and then window was resized to desktop, nav would still be closed but the button to open would be gone, as per #1 on issue #146 */ + useEffect(() => { - window.addEventListener('resize', () => { - if (window.matchMedia('(min-width: 768px)').matches) { - setNavState('ACTIVE'); - } else { - setNavState('INACTIVE'); - } - }); - }, []); + if (isMobile) { + setNavOpen(false); + return; + } + setNavOpen(true); + }, [isMobile]); return ( <>

@@ -176,7 +168,11 @@ export default function Navigation({ to={'/'} onClick={() => { dispatch(setConversation([])); - dispatch(updateConversationId({ query: { conversationId: null } })); + dispatch( + updateConversationId({ + query: { conversationId: null }, + }), + ); }} className={({ isActive }) => `${ @@ -327,7 +323,7 @@ export default function Navigation({ link

Documentation

- + diff --git a/frontend/src/hooks.ts b/frontend/src/hooks.ts deleted file mode 100644 index eef2c72dd..000000000 --- a/frontend/src/hooks.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { useEffect, RefObject } from 'react'; - -export function useOutsideAlerter( - ref: RefObject, - handler: () => void, - additionalDeps: unknown[], -) { - useEffect(() => { - function handleClickOutside(this: Document, event: MouseEvent) { - if (ref.current && !ref.current.contains(event.target as Node)) { - handler(); - } - } - document.addEventListener('mousedown', handleClickOutside); - return () => { - document.removeEventListener('mousedown', handleClickOutside); - }; - }, [ref, ...additionalDeps]); -} diff --git a/frontend/src/hooks/index.test.ts b/frontend/src/hooks/index.test.ts new file mode 100644 index 000000000..66965c328 --- /dev/null +++ b/frontend/src/hooks/index.test.ts @@ -0,0 +1,20 @@ +// When testing library added this is unit test for the useMediaQuery + +// import { renderHook } from '@testing-library/react-hooks'; +// import { useMediaQuery } from '.'; + +// describe('useMediaQuery', () => { +// it('should update isMobile and isDesktop when window is resized', () => { +// const { result } = renderHook(() => useMediaQuery()); + +// global.innerWidth = 800; +// global.dispatchEvent(new Event('resize')); +// expect(result.current.isMobile).toBe(true); +// expect(result.current.isDesktop).toBe(false); + +// global.innerWidth = 1200; +// global.dispatchEvent(new Event('resize')); +// expect(result.current.isMobile).toBe(false); +// expect(result.current.isDesktop).toBe(true); +// }); +// }); diff --git a/frontend/src/hooks/index.ts b/frontend/src/hooks/index.ts new file mode 100644 index 000000000..bcf3cee9c --- /dev/null +++ b/frontend/src/hooks/index.ts @@ -0,0 +1,45 @@ +import { useEffect, RefObject, useState } from 'react'; + +export function useOutsideAlerter( + ref: RefObject, + handler: () => void, + additionalDeps: unknown[], +) { + useEffect(() => { + function handleClickOutside(this: Document, event: MouseEvent) { + if (ref.current && !ref.current.contains(event.target as Node)) { + handler(); + } + } + document.addEventListener('mousedown', handleClickOutside); + return () => { + document.removeEventListener('mousedown', handleClickOutside); + }; + }, [ref, ...additionalDeps]); +} + +// Use isMobile for checking if the width is in the expected mobile range (less than 768px) +// use IsDesktop for effects you explicitly only want when width is wider than 960px. +export function useMediaQuery() { + const mobileQuery = '(max-width: 768px)'; + const desktopQuery = '(min-width: 960px)'; + const [isMobile, setIsMobile] = useState(false); + const [isDesktop, setIsDesktop] = useState(false); + + useEffect(() => { + const mobileMedia = window.matchMedia(mobileQuery); + const desktopMedia = window.matchMedia(desktopQuery); + const updateMediaQueries = () => { + setIsMobile(mobileMedia.matches); + setIsDesktop(desktopMedia.matches); + }; + updateMediaQueries(); + const listener = () => updateMediaQueries(); + window.addEventListener('resize', listener); + return () => { + window.removeEventListener('resize', listener); + }; + }, [mobileQuery, desktopQuery]); + + return { isMobile, isDesktop }; +} diff --git a/frontend/src/preferences/APIKeyModal.tsx b/frontend/src/preferences/APIKeyModal.tsx index 0adc48815..7aeeadb27 100644 --- a/frontend/src/preferences/APIKeyModal.tsx +++ b/frontend/src/preferences/APIKeyModal.tsx @@ -2,7 +2,7 @@ import { useRef, useState } from 'react'; import { useDispatch, useSelector } from 'react-redux'; import { ActiveState } from '../models/misc'; import { selectApiKey, setApiKey } from './preferenceSlice'; -import { useOutsideAlerter } from './../hooks'; +import { useMediaQuery, useOutsideAlerter } from './../hooks'; import Modal from '../Modal'; export default function APIKeyModal({ @@ -19,14 +19,12 @@ export default function APIKeyModal({ const [key, setKey] = useState(apiKey); const [isError, setIsError] = useState(false); const modalRef = useRef(null); + const { isMobile } = useMediaQuery(); useOutsideAlerter( modalRef, () => { - if ( - window.matchMedia('(max-width: 768px)').matches && - modalState === 'ACTIVE' - ) { + if (isMobile && modalState === 'ACTIVE') { setModalState('INACTIVE'); } }, From 4be0c1c0eb0e8de2a53f290a58b30a1d4a48f04c Mon Sep 17 00:00:00 2001 From: jerempy <95110820+jerempy@users.noreply.github.com> Date: Wed, 4 Oct 2023 13:41:43 -0400 Subject: [PATCH 2/2] delete unused test --- frontend/src/hooks/index.test.ts | 20 -------------------- 1 file changed, 20 deletions(-) delete mode 100644 frontend/src/hooks/index.test.ts diff --git a/frontend/src/hooks/index.test.ts b/frontend/src/hooks/index.test.ts deleted file mode 100644 index 66965c328..000000000 --- a/frontend/src/hooks/index.test.ts +++ /dev/null @@ -1,20 +0,0 @@ -// When testing library added this is unit test for the useMediaQuery - -// import { renderHook } from '@testing-library/react-hooks'; -// import { useMediaQuery } from '.'; - -// describe('useMediaQuery', () => { -// it('should update isMobile and isDesktop when window is resized', () => { -// const { result } = renderHook(() => useMediaQuery()); - -// global.innerWidth = 800; -// global.dispatchEvent(new Event('resize')); -// expect(result.current.isMobile).toBe(true); -// expect(result.current.isDesktop).toBe(false); - -// global.innerWidth = 1200; -// global.dispatchEvent(new Event('resize')); -// expect(result.current.isMobile).toBe(false); -// expect(result.current.isDesktop).toBe(true); -// }); -// });