From e729bd52aad63fedd6181356fdc3a5b23d65eade Mon Sep 17 00:00:00 2001 From: Paul Armstrong Date: Wed, 10 Feb 2021 13:21:43 -0800 Subject: [PATCH] refactor(web): Split AppBar and add tests --- web/src/App.jsx | 4 +- web/src/AppBar.jsx | 46 +++++++ web/src/__tests__/AppBar.test.jsx | 53 ++++++++ web/src/components/AppBar.jsx | 83 ++++-------- web/src/components/__tests__/AppBar.test.jsx | 132 +++++++++++++++++++ 5 files changed, 261 insertions(+), 57 deletions(-) create mode 100644 web/src/AppBar.jsx create mode 100644 web/src/__tests__/AppBar.test.jsx create mode 100644 web/src/components/__tests__/AppBar.test.jsx diff --git a/web/src/App.jsx b/web/src/App.jsx index 891bf5807..2482efa1a 100644 --- a/web/src/App.jsx +++ b/web/src/App.jsx @@ -2,7 +2,7 @@ import * as Routes from './routes'; import { h } from 'preact'; import ActivityIndicator from './components/ActivityIndicator'; import AsyncRoute from 'preact-async-route'; -import AppBar from './components/AppBar'; +import AppBar from './AppBar'; import Cameras from './routes/Cameras'; import { Router } from 'preact-router'; import Sidebar from './Sidebar'; @@ -15,7 +15,7 @@ export default function App() {
- + {status !== FetchStatus.LOADED ? (
diff --git a/web/src/AppBar.jsx b/web/src/AppBar.jsx new file mode 100644 index 000000000..6a3ff2a72 --- /dev/null +++ b/web/src/AppBar.jsx @@ -0,0 +1,46 @@ +import { h, Fragment } from 'preact'; +import BaseAppBar from './components/AppBar'; +import LinkedLogo from './components/LinkedLogo'; +import Menu, { MenuItem, MenuSeparator } from './components/Menu'; +import AutoAwesomeIcon from './icons/AutoAwesome'; +import LightModeIcon from './icons/LightMode'; +import DarkModeIcon from './icons/DarkMode'; +import { useDarkMode } from './context'; +import { useCallback, useRef, useState } from 'preact/hooks'; + +export default function AppBar() { + const [showMoreMenu, setShowMoreMenu] = useState(false); + const { setDarkMode } = useDarkMode(); + + const handleSelectDarkMode = useCallback( + (value, label) => { + setDarkMode(value); + setShowMoreMenu(false); + }, + [setDarkMode, setShowMoreMenu] + ); + + const moreRef = useRef(null); + + const handleShowMenu = useCallback(() => { + setShowMoreMenu(true); + }, [setShowMoreMenu]); + + const handleDismissMoreMenu = useCallback(() => { + setShowMoreMenu(false); + }, [setShowMoreMenu]); + + return ( + + + {showMoreMenu ? ( + + + + + + + ) : null} + + ); +} diff --git a/web/src/__tests__/AppBar.test.jsx b/web/src/__tests__/AppBar.test.jsx new file mode 100644 index 000000000..66c1c34bc --- /dev/null +++ b/web/src/__tests__/AppBar.test.jsx @@ -0,0 +1,53 @@ +import { h } from 'preact'; +import * as Context from '../context'; +import AppBar from '../AppBar'; +import { fireEvent, render, screen } from '@testing-library/preact'; + +describe('AppBar', () => { + beforeEach(() => { + jest.spyOn(Context, 'useDarkMode').mockImplementation(() => ({ + setDarkMode: jest.fn(), + })); + jest.spyOn(Context, 'DarkModeProvider').mockImplementation(({ children }) => { + return
{children}
; + }); + }); + + test('shows a menu on overflow click', async () => { + render( + + + + + + ); + + const overflowButton = await screen.findByLabelText('More options'); + fireEvent.click(overflowButton); + + const menu = await screen.findByRole('listbox'); + expect(menu).toBeInTheDocument(); + }); + + test('sets dark mode on MenuItem select', async () => { + const setDarkModeSpy = jest.fn(); + jest.spyOn(Context, 'useDarkMode').mockImplementation(() => ({ + setDarkMode: setDarkModeSpy, + })); + render( + + + + + + ); + + const overflowButton = await screen.findByLabelText('More options'); + fireEvent.click(overflowButton); + + await screen.findByRole('listbox'); + + fireEvent.click(screen.getByText('Light')); + expect(setDarkModeSpy).toHaveBeenCalledWith('light'); + }); +}); diff --git a/web/src/components/AppBar.jsx b/web/src/components/AppBar.jsx index 591786a74..c35a4ad4a 100644 --- a/web/src/components/AppBar.jsx +++ b/web/src/components/AppBar.jsx @@ -1,48 +1,28 @@ import { h } from 'preact'; import Button from './Button'; -import LinkedLogo from './LinkedLogo'; -import Menu, { MenuItem, MenuSeparator } from './Menu'; import MenuIcon from '../icons/Menu'; import MoreIcon from '../icons/More'; -import AutoAwesomeIcon from '../icons/AutoAwesome'; -import LightModeIcon from '../icons/LightMode'; -import DarkModeIcon from '../icons/DarkMode'; -import { useDarkMode, useDrawer } from '../context'; -import { useLayoutEffect, useCallback, useRef, useState } from 'preact/hooks'; +import { useDrawer } from '../context'; +import { useLayoutEffect, useCallback, useState } from 'preact/hooks'; // We would typically preserve these in component state // But need to avoid too many re-renders let lastScrollY = window.scrollY; -export default function AppBar({ title }) { +export default function AppBar({ title: Title, overflowRef, onOverflowClick }) { const [show, setShow] = useState(true); const [atZero, setAtZero] = useState(window.scrollY === 0); - const [showMoreMenu, setShowMoreMenu] = useState(false); - const { setDarkMode } = useDarkMode(); const { setShowDrawer } = useDrawer(); - const handleSelectDarkMode = useCallback( - (value, label) => { - setDarkMode(value); - setShowMoreMenu(false); - }, - [setDarkMode, setShowMoreMenu] - ); + const scrollListener = useCallback(() => { + const scrollY = window.scrollY; - const moreRef = useRef(null); - - const scrollListener = useCallback( - (event) => { - const scrollY = window.scrollY; - - window.requestAnimationFrame(() => { - setShow(scrollY <= 0 || lastScrollY > scrollY); - setAtZero(scrollY === 0); - lastScrollY = scrollY; - }); - }, - [setShow] - ); + window.requestAnimationFrame(() => { + setShow(scrollY <= 0 || lastScrollY > scrollY); + setAtZero(scrollY === 0); + lastScrollY = scrollY; + }); + }, [setShow]); useLayoutEffect(() => { document.addEventListener('scroll', scrollListener); @@ -51,45 +31,38 @@ export default function AppBar({ title }) { }; }, [scrollListener]); - const handleShowMenu = useCallback(() => { - setShowMoreMenu(true); - }, [setShowMoreMenu]); - - const handleDismissMoreMenu = useCallback(() => { - setShowMoreMenu(false); - }, [setShowMoreMenu]); - const handleShowDrawer = useCallback(() => { setShowDrawer(true); }, [setShowDrawer]); return (
- + <div className="flex-grow-1 flex justify-end w-full"> - <div className="w-auto" ref={moreRef}> - <Button color="black" className="rounded-full w-12 h-12" onClick={handleShowMenu} type="text"> - <MoreIcon className="w-10 h-10" /> - </Button> - </div> + {overflowRef && onOverflowClick ? ( + <div className="w-auto" ref={overflowRef}> + <Button + aria-label="More options" + color="black" + className="rounded-full w-12 h-12" + onClick={onOverflowClick} + type="text" + > + <MoreIcon className="w-10 h-10" /> + </Button> + </div> + ) : null} </div> - {showMoreMenu ? ( - <Menu onDismiss={handleDismissMoreMenu} relativeTo={moreRef}> - <MenuItem icon={AutoAwesomeIcon} label="Auto dark mode" value="media" onSelect={handleSelectDarkMode} /> - <MenuSeparator /> - <MenuItem icon={LightModeIcon} label="Light" value="light" onSelect={handleSelectDarkMode} /> - <MenuItem icon={DarkModeIcon} label="Dark" value="dark" onSelect={handleSelectDarkMode} /> - </Menu> - ) : null} </div> ); } diff --git a/web/src/components/__tests__/AppBar.test.jsx b/web/src/components/__tests__/AppBar.test.jsx new file mode 100644 index 000000000..867b056fe --- /dev/null +++ b/web/src/components/__tests__/AppBar.test.jsx @@ -0,0 +1,132 @@ +import { h } from 'preact'; +import { DrawerProvider } from '../../context'; +import AppBar from '../AppBar'; +import { fireEvent, render, screen } from '@testing-library/preact'; +import { useRef } from 'preact/hooks'; + +function Title() { + return <div>I am the title</div>; +} + +describe('AppBar', () => { + test('renders the title', async () => { + render( + <DrawerProvider> + <AppBar title={Title} /> + </DrawerProvider> + ); + expect(screen.getByText('I am the title')).toBeInTheDocument(); + }); + + describe('overflow menu', () => { + test('is not rendered if a ref is not provided', async () => { + const handleOverflow = jest.fn(); + render( + <DrawerProvider> + <AppBar title={Title} onOverflowClick={handleOverflow} /> + </DrawerProvider> + ); + expect(screen.queryByLabelText('More options')).not.toBeInTheDocument(); + }); + + test('is not rendered if a click handler is not provided', async () => { + function Wrapper() { + const ref = useRef(null); + return <AppBar title={Title} overflowRef={ref} />; + } + + render( + <DrawerProvider> + <Wrapper /> + </DrawerProvider> + ); + expect(screen.queryByLabelText('More options')).not.toBeInTheDocument(); + }); + + test('is rendered with click handler and ref', async () => { + const handleOverflow = jest.fn(); + + function Wrapper() { + const ref = useRef(null); + return <AppBar title={Title} overflowRef={ref} onOverflowClick={handleOverflow} />; + } + + render( + <DrawerProvider> + <Wrapper /> + </DrawerProvider> + ); + expect(screen.queryByLabelText('More options')).toBeInTheDocument(); + }); + + test('calls the handler when clicked', async () => { + const handleOverflow = jest.fn(); + + function Wrapper() { + const ref = useRef(null); + return <AppBar title={Title} overflowRef={ref} onOverflowClick={handleOverflow} />; + } + + render( + <DrawerProvider> + <Wrapper /> + </DrawerProvider> + ); + + fireEvent.click(screen.queryByLabelText('More options')); + + expect(handleOverflow).toHaveBeenCalled(); + }); + }); + + describe('scrolling', () => { + test('is visible initially', async () => { + render( + <DrawerProvider> + <AppBar title={Title} /> + </DrawerProvider> + ); + + const classes = screen.getByTestId('appbar').classList; + + expect(classes.contains('translate-y-0')).toBe(true); + expect(classes.contains('-translate-y-full')).toBe(false); + }); + + test('hides when scrolled downward', async () => { + jest.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => cb()); + render( + <DrawerProvider> + <AppBar title={Title} /> + </DrawerProvider> + ); + + window.scrollY = 300; + await fireEvent.scroll(document, { target: { scrollY: 300 } }); + + const classes = screen.getByTestId('appbar').classList; + + expect(classes.contains('translate-y-0')).toBe(false); + expect(classes.contains('-translate-y-full')).toBe(true); + }); + + test('reappears when scrolled upward', async () => { + jest.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => cb()); + render( + <DrawerProvider> + <AppBar title={Title} /> + </DrawerProvider> + ); + + window.scrollY = 300; + await fireEvent.scroll(document, { target: { scrollY: 300 } }); + window.scrollY = 280; + await fireEvent.scroll(document, { target: { scrollY: 280 } }); + + const classes = screen.getByTestId('appbar').classList; + + expect(classes.contains('translate-y-0')).toBe(true); + expect(classes.contains('-translate-y-full')).toBe(false); + }); + }); +});