-
Notifications
You must be signed in to change notification settings - Fork 44
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
Contra Frontend technical assessment #64
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
{ | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
import '@testing-library/jest-dom/extend-expect'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,22 +13,32 @@ | |
"test:ci": "NODE_ENV=test jest --ci --reporters jest-silent-reporter" | ||
}, | ||
"dependencies": { | ||
"focus-trap-react": "^10.0.0", | ||
"next": "12.1.6", | ||
"react": "18.2.0", | ||
"react-dom": "18.2.0" | ||
"react-dom": "18.2.0", | ||
"react-icons": "^4.4.0", | ||
"styled-components": "^5.3.5" | ||
}, | ||
"devDependencies": { | ||
"@types/jest": "28.1.3", | ||
"@testing-library/dom": "^8.17.1", | ||
"@testing-library/jest-dom": "^5.16.5", | ||
"@testing-library/react": "^13.3.0", | ||
"@testing-library/user-event": "^14.4.3", | ||
"@types/jest": "^29.0.0", | ||
"@types/node": "18.0.0", | ||
"@types/react": "18.0.14", | ||
"@types/react-dom": "18.0.5", | ||
"@types/styled-components": "^5.1.26", | ||
"eslint": "8.18.0", | ||
"eslint-config-canonical": "35.0.1", | ||
"eslint-config-next": "12.1.6", | ||
"eslint-config-prettier": "8.5.0", | ||
"jest": "28.1.1", | ||
"jest-environment-jsdom": "^29.0.1", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would appreciate having this dependency pre-installed as well 😄 |
||
"lint-staged": "13.0.3", | ||
"prettier": "2.7.1", | ||
"ts-jest": "^28.0.8", | ||
"typescript": "4.7.4" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,4 +6,5 @@ | |
|
||
module.exports = { | ||
singleQuote: true, | ||
tabWidth: 2, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,129 @@ | ||
import React, { useRef } from 'react'; | ||
import styled from 'styled-components'; | ||
import { FiX as CloseIcon } from 'react-icons/fi'; | ||
import { createPortal } from 'react-dom'; | ||
import FocusTrap from 'focus-trap-react'; | ||
|
||
// hooks | ||
import { | ||
useDisableScroll, | ||
useOnClickOutside, | ||
useOnEscKeypress, | ||
} from '@/hooks/'; | ||
|
||
// styles | ||
const ModalOverlay = styled.div` | ||
align-items: center; | ||
background: rgba(0, 0, 0, 0.1); | ||
backdrop-filter: blur(2px); | ||
display: flex; | ||
height: 100vh; | ||
justify-content: center; | ||
left: 0; | ||
position: fixed; | ||
top: 0; | ||
width: 100vw; | ||
z-index: auto; | ||
`; | ||
|
||
const ModalContainer = styled.div` | ||
background: white; | ||
border-radius: 0.375rem; | ||
box-shadow: 0 10px 15px -3px rgba(0, 0, 0, 0.1), | ||
0 4px 6px -2px rgba(0, 0, 0, 0.05); | ||
display: flex; | ||
flex-direction: column; | ||
padding: 16px 24px; | ||
position: relative; | ||
`; | ||
|
||
const ModalWrapper = styled.div` | ||
max-width: 28rem; | ||
width: 90%; | ||
`; | ||
|
||
const CloseButton = styled.button` | ||
align-items: center; | ||
background: transparent; | ||
border: transparent; | ||
border-radius: 9999px; | ||
cursor: pointer; | ||
display: flex; | ||
height: 32px; | ||
position: absolute; | ||
right: 12px; | ||
top: 8px; | ||
width: 32px; | ||
|
||
transition: all 0.1s ease-in; | ||
|
||
&:hover { | ||
background-color: lightblue; | ||
color: white; | ||
} | ||
`; | ||
|
||
const ModalHeader = styled.h1` | ||
color: #2d3748; | ||
font-size: 28px; | ||
margin-bottom: 12px; | ||
`; | ||
|
||
const ModalBody = styled.div` | ||
color: #718096; | ||
font-size: 16px; | ||
`; | ||
|
||
type ModalType = { | ||
isOpen: boolean; | ||
onClose: () => void; | ||
title: string; | ||
children?: React.ReactNode; | ||
}; | ||
|
||
/** | ||
* Basic Modal inspired by Chakra-UI Modal. Supports focus management, background scroll-locking, tab navigation, is a11y compliant and mobile friendly. | ||
* Unfortunately ran out of time before implementing multi-modal environment. | ||
* | ||
* Decided to use npm package focus-trap-react for focus management as it is well maintained and scaleable. | ||
* As for a custom focus solution, I was thinking something along this (https://gist.github.com/asvny/99988385aa5b1573be49309bbaa0f588). | ||
* However that relies on querying all possible focusable elements (which is an exhausive list), | ||
* and would need to be maintained to make sure no new elements would slip by. | ||
*/ | ||
const Modal = ({ isOpen, onClose, title, children }: ModalType) => { | ||
const modalWrapperRef = useRef<HTMLDivElement>(null); | ||
|
||
useOnEscKeypress(onClose); | ||
useDisableScroll(isOpen); | ||
useOnClickOutside(modalWrapperRef, onClose); | ||
|
||
if (isOpen) { | ||
const component = ( | ||
<ModalOverlay> | ||
<FocusTrap active={isOpen}> | ||
<ModalWrapper ref={modalWrapperRef}> | ||
<ModalContainer | ||
role="dialog" | ||
aria-modal="true" | ||
aria-labelledby="header" | ||
aria-describedby="body" | ||
> | ||
<ModalHeader id="header">{title}</ModalHeader> | ||
<CloseButton data-testid="modal-close-button" onClick={onClose}> | ||
<CloseIcon size={32} /> | ||
</CloseButton> | ||
<ModalBody id="body">{children}</ModalBody> | ||
</ModalContainer> | ||
</ModalWrapper> | ||
</FocusTrap> | ||
</ModalOverlay> | ||
); | ||
|
||
const modalRoot = document.getElementById('modal-root')!; | ||
return createPortal(component, modalRoot); | ||
} else { | ||
return null; | ||
} | ||
}; | ||
|
||
export default Modal; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
import React from 'react'; | ||
import { render, screen, fireEvent } from '@testing-library/react'; | ||
import useDisableScroll from '../useDisableScroll'; | ||
|
||
describe('useOnClickOutside', () => { | ||
const callback = jest.fn(); | ||
const TestComp = ({ hideScroll }: { hideScroll: boolean }) => { | ||
useDisableScroll(hideScroll); | ||
return ( | ||
<div | ||
data-testid="overflow-container" | ||
style={{ height: '150vh' }} | ||
onScroll={callback} | ||
> | ||
<span>blah</span> | ||
</div> | ||
); | ||
}; | ||
|
||
afterEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
test('when scroll not disabled, callback triggered', async () => { | ||
render(<TestComp hideScroll={false} />); | ||
expect(callback).not.toHaveBeenCalled(); | ||
|
||
// click on div | ||
const container = screen.getByTestId('overflow-container'); | ||
fireEvent.scroll(container, { target: { scrollY: 100 } }); | ||
expect(callback).toHaveBeenCalledTimes(1); | ||
}); | ||
|
||
// can't seem to get this working, also not sure how to test changes in `document.body.style.overflow` | ||
test.skip('when scroll disabled, callback is not triggered', async () => { | ||
render(<TestComp hideScroll={true} />); | ||
expect(callback).not.toHaveBeenCalled(); | ||
|
||
// click on div | ||
const container = screen.getByTestId('overflow-container'); | ||
fireEvent.scroll(container, { target: { scrollY: 100 } }); | ||
expect(callback).toHaveBeenCalledTimes(1); | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a fan of pushing skipped/broken tests, but for the sake of the assessment, I left it in. |
||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
import React, { useRef } from 'react'; | ||
import { render, screen } from '@testing-library/react'; | ||
import userEvent from '@testing-library/user-event'; | ||
import useOnClickOutside from '../useOnClickOutside'; | ||
|
||
describe('useOnClickOutside', () => { | ||
const callback = jest.fn(); | ||
const TestComp = () => { | ||
const ref = useRef<HTMLDivElement>(null); | ||
useOnClickOutside(ref, callback); | ||
return ( | ||
<div data-testid="outside" style={{ width: '200px', height: '200px' }}> | ||
<div ref={ref}> | ||
<div data-testid="inside" style={{ width: '20px', height: '20px' }}> | ||
<span>Inside it's safe</span> | ||
<button data-testid="inside-button">Dummy button</button> | ||
</div> | ||
</div> | ||
<button data-testid="outside-button">Outside button</button> | ||
</div> | ||
); | ||
}; | ||
|
||
afterEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
test('callback triggered when clicking elements outside ', async () => { | ||
const user = userEvent.setup(); | ||
render(<TestComp />); | ||
expect(callback).not.toHaveBeenCalled(); | ||
|
||
// click on div | ||
const outsideComp = screen.getByTestId('outside'); | ||
await user.click(outsideComp); | ||
expect(callback).toHaveBeenCalledTimes(1); | ||
|
||
// click on button | ||
const button = screen.getByTestId('outside-button'); | ||
await user.click(button); | ||
expect(callback).toHaveBeenCalledTimes(2); | ||
}); | ||
|
||
test('callback not triggered when clicking elements inside ', async () => { | ||
const user = userEvent.setup(); | ||
render(<TestComp />); | ||
expect(callback).not.toHaveBeenCalled(); | ||
|
||
// click on div | ||
const insideComp = screen.getByTestId('inside'); | ||
await user.click(insideComp); | ||
expect(callback).not.toHaveBeenCalled(); | ||
|
||
// click on text | ||
const text = screen.getByText(/Inside it's safe/); | ||
await user.click(text); | ||
expect(callback).not.toHaveBeenCalled(); | ||
|
||
// click on button | ||
const button = screen.getByTestId('inside-button'); | ||
await user.click(button); | ||
expect(callback).not.toHaveBeenCalled(); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import React from 'react'; | ||
import { render } from '@testing-library/react'; | ||
import userEvent from '@testing-library/user-event'; | ||
import useOnEscKeypress from '../useOnEscKeypress'; | ||
|
||
describe('useOnEscKeypress', () => { | ||
const callback = jest.fn(); | ||
const TestComp = () => { | ||
useOnEscKeypress(callback); | ||
return <div>hello</div>; | ||
}; | ||
|
||
afterEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
test('callback called on escape key press', async () => { | ||
const user = userEvent.setup(); | ||
render(<TestComp />); | ||
expect(callback).not.toHaveBeenCalled(); | ||
await user.keyboard('{Escape}'); | ||
expect(callback).toHaveBeenCalledTimes(1); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import useDisableScroll from './useDisableScroll'; | ||
import useOnClickOutside from './useOnClickOutside'; | ||
import useOnEscKeypress from './useOnEscKeypress'; | ||
|
||
export { useDisableScroll, useOnClickOutside, useOnEscKeypress }; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
import { useEffect } from 'react'; | ||
|
||
const useDisableScroll = (hideScroll: boolean) => { | ||
useEffect(() => { | ||
if (hideScroll) { | ||
document.body.style.overflow = 'hidden'; | ||
} | ||
|
||
return () => { | ||
document.body.style.overflow = 'unset'; | ||
}; | ||
}, [hideScroll]); | ||
}; | ||
|
||
export default useDisableScroll; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import React, { useEffect } from 'react'; | ||
|
||
// citation: https://usehooks.com/useOnClickOutside/ | ||
const useOnClickOutside = ( | ||
ref: React.RefObject<HTMLDivElement>, | ||
handler: (event: MouseEvent | TouchEvent) => void | ||
) => { | ||
useEffect(() => { | ||
const callBack = (event: MouseEvent | TouchEvent) => { | ||
if (!ref.current || ref.current.contains(event.target as Node)) { | ||
return; | ||
} | ||
handler(event); | ||
}; | ||
|
||
document.addEventListener('mousedown', callBack); | ||
document.addEventListener('touchstart', callBack); | ||
return () => { | ||
document.removeEventListener('mousedown', callBack); | ||
document.removeEventListener('touchstart', callBack); | ||
}; | ||
}, [ref, handler]); | ||
}; | ||
|
||
export default useOnClickOutside; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would help to have this setup before interviewees start on the project.
Looks like this will be added by the Next team in the near future (link)