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

Kofi Nedjoh - Frontend Assessment #86

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions frontend/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,20 @@ $ yarn dev
```

and the application should come alive at `localhost:3000`

## Assessment
### Libraries I used
- focus-trap-react to help keep focus in currently opened Modal
- body-scroll-lock to help lock background scroll when Modal is open

### Liberties I took
- Outside click can be configured to either dismiss the Modal or not
- Option to make Modal fullscreen or not
- At very small screen sizes (*450px and below*), modal becomes fullscreen. This is a media query addition for the demo page and is not baked into the Modal implementation itself

### Improvements provided enough time
- **Styling** will be migrated to a SASS model to help with **encapulation** and **reuse**.
- **Storybook** will be added to provide more flexible and variable **testing**.
- Proper **snapshot testing** will be added for varying states.
- **Extensibility** will be implemented to take advantage of **custom styling** as well as **placement of segments like header and footer**. It will however remain foundational if you do not extend it.
- Look for more ways to **clean** the codebase
1 change: 1 addition & 0 deletions frontend/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const createJestConfig = nextJest({
const customJestConfig = {
// Add more setup options before each test is run
// setupFilesAfterEnv: ['<rootDir>/jest.setup.js'],
setupFilesAfterEnv: ['@testing-library/jest-dom/extend-expect'],
// if using TypeScript with a baseUrl set to the root directory then you need the below for alias' to work
moduleDirectories: ['node_modules', '<rootDir>/'],
testEnvironment: 'jest-environment-jsdom',
Expand Down
6 changes: 6 additions & 0 deletions frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,16 @@
"test:ci": "NODE_ENV=test jest --ci --reporters jest-silent-reporter"
},
"dependencies": {
"@types/body-scroll-lock": "^3.1.0",
"body-scroll-lock": "^4.0.0-beta.0",
"focus-trap-react": "^10.1.1",
"next": "12.1.6",
"react": "18.2.0",
"react-dom": "18.2.0"
},
"devDependencies": {
"@testing-library/jest-dom": "^5.16.5",
"@testing-library/react": "^14.0.0",
"@types/jest": "28.1.3",
"@types/node": "18.0.0",
"@types/react": "18.0.14",
Expand All @@ -27,6 +32,7 @@
"eslint-config-next": "12.1.6",
"eslint-config-prettier": "8.5.0",
"jest": "28.1.1",
"jest-environment-jsdom": "^29.5.0",
"lint-staged": "13.0.3",
"prettier": "2.7.1",
"typescript": "4.7.4"
Expand Down
28 changes: 28 additions & 0 deletions frontend/src/component/modal/Modal.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { fireEvent, render } from '@testing-library/react'
import Modal from './Modal'

const onClose = jest.fn(() => {})

describe('Modal', () => {
test('should render modal and test interactions successfully', async () => {
const { getByText, getByTestId } = render(
<Modal
isOpen={true}
title='Test Modal'
onClose={onClose}
>
The quick brown fox jumps over the lazy dog
</Modal>
)

expect( getByText(/test modal/i) ).toBeInTheDocument()
expect( getByText(/close/i) ).toBeInTheDocument()
expect( getByText(/the quick brown fox jumps over the lazy dog/i) ).toBeInTheDocument()

const button = getByTestId('data-modal__close-button')

fireEvent.click(button)

expect(onClose).toHaveBeenCalled()
})
})
147 changes: 147 additions & 0 deletions frontend/src/component/modal/Modal.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
import { Dispatch, ForwardedRef, KeyboardEvent, MouseEvent, ReactNode, SetStateAction, forwardRef, useEffect, useRef, useState } from 'react'
import { createPortal } from 'react-dom'
import { disableBodyScroll, enableBodyScroll, clearAllBodyScrollLocks } from 'body-scroll-lock'
import TrapFocus from 'focus-trap-react'

const ESCAPE_KEY = 'Escape'

type ModalProps = {
isOpen: boolean,
title: ReactNode,
fullscreen?: boolean,
dismissOnOutsideClick?: boolean,
children: ReactNode,
onClose: () => void,
}

type ActualModalProps = {
title: ReactNode,
fullscreen?: boolean,
children: ReactNode,
onClose: () => void,
onOverlayClick: (event: MouseEvent<HTMLDivElement>) => void
onHover: Dispatch<SetStateAction<boolean>>
}

type HeaderProps = {
children: ReactNode,
closeHandler: () => void,
}

const Header = ({ children, closeHandler }: HeaderProps) => {
return (
<div
className='horizontal-flex modal__title'
data-testid='data-modal__title'
aria-label='Modal Title'
>
{ children }
<button
className='button'
data-testid='data-modal__close-button'
aria-label='Close Button'
onClick={closeHandler}
>
Close
</button>
</div>
)
}

const ActualModal = forwardRef(({ title, fullscreen, children, onClose, onOverlayClick, onHover }: ActualModalProps, ref: ForwardedRef<HTMLDivElement>) => {
const handleKeyDown = (event: KeyboardEvent<HTMLDivElement>) => {
if( event.key === ESCAPE_KEY ) {
event.stopPropagation()
onClose()
}
}

const handleClose = () => onClose()

return (
<div
className='modal__overlay'
role='dialog'
aria-modal='true'
aria-label='Clickable Modal Overlay'
data-testid='data-modal'
tabIndex={-1}
onKeyDown={handleKeyDown}
onClick={onOverlayClick}
>
<div
ref={ref}
className={`modal__body${ fullscreen ? ' fullscreen' : ''}`}
data-testid='data-modal__body'
aria-label='Clickable Modal Body'
onClick={onOverlayClick}
onMouseEnter={() => onHover(false)}
onMouseLeave={() => onHover(true)}
>
<Header closeHandler={handleClose}>{ title }</Header>
<div
className='modal__content'
data-testid='data-modal__content'
aria-label='Modal Content'
>
{ children }
</div>
</div>
</div>
)
})

const Modal = ({ isOpen, title, children, fullscreen = false, dismissOnOutsideClick = true, onClose }: ModalProps) => {
const container = useRef<HTMLDivElement>(null)
const [isOutsideClickable, setIsOutsideClickable] = useState(dismissOnOutsideClick)

useEffect(() => {
if(container.current) {
if(isOpen) disableBodyScroll(container.current)
else enableBodyScroll(container.current)
}

return () => {
const nodes = document.querySelectorAll('.modal__overlay')
if( nodes.length === 0 ) clearAllBodyScrollLocks()
}
}, [isOpen])

const onOverlayClick = (event: MouseEvent<HTMLDivElement>) => {
if(dismissOnOutsideClick) {
event.stopPropagation()
if(!container.current?.contains(event.currentTarget)) onClose()
}
}

return (
<>
{
isOpen ?
createPortal(
<TrapFocus
paused={isOutsideClickable}
focusTrapOptions={{
fallbackFocus: '.modal__overlay'
}}
>
<ActualModal
ref={container}
title={title}
fullscreen={fullscreen}
onClose={onClose}
onOverlayClick={onOverlayClick}
onHover={setIsOutsideClickable}
>
{children}
</ActualModal>
</TrapFocus>,
document.body
) :
null
}
</>
)
}

export default Modal
7 changes: 7 additions & 0 deletions frontend/src/pages/_app.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
/* eslint-disable canonical/filename-match-exported */
import { type AppProps } from 'next/app';

import '../styles/flex.css'
import '../styles/font.css'
import '../styles/modal.css'
import '../styles/button.css'
import '../styles/general.css'
import '../styles/container.css'

const App = ({ Component, pageProps }: AppProps) => {
return <Component {...pageProps} />;
};
Expand Down
29 changes: 29 additions & 0 deletions frontend/src/pages/demos/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import Link from 'next/link';
import { NextPage } from 'next/types';

const DemosIndex: NextPage = () => {
return (
<div className='vertical-flex center-flex fill-screen'>
<Link href='/'>
<a
className='link font-large center-text'
tabIndex={0}
aria-label='Go To Back Main Page'
>
{'<< Back To Main Page'}
</a>
</Link>
<Link href='/demos/modals'>
<a
className='link font-large center-text'
tabIndex={1}
aria-label='Go To Modal Demo Page'
>
{'Go To Modal Demo >>'}
</a>
</Link>
</div>
)
}

export default DemosIndex;
Loading