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

Contra Frontend technical assessment #64

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
8 changes: 7 additions & 1 deletion frontend/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,15 @@ const createJestConfig = nextJest({
// Add any custom config to be passed to Jest
const customJestConfig = {
// Add more setup options before each test is run
// setupFilesAfterEnv: ['<rootDir>/jest.setup.js'],
setupFilesAfterEnv: ['<rootDir>/jest.setup.js'],
// if using TypeScript with a baseUrl set to the root directory then you need the below for alias' to work
moduleDirectories: ['node_modules', '<rootDir>/'],
// recommend having this pre-setup as I had pathing issues with tests
moduleNameMapper: {
'^@/components/(.*)$': '<rootDir>/src/components/$1',
'^@/hooks/(.*)$': '<rootDir>/src/hooks/$1',
'^@/pages/(.*)$': '<rootDir>/src/pages/$1',
},
Copy link
Author

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)

testEnvironment: 'jest-environment-jsdom',
};

Expand Down
1 change: 1 addition & 0 deletions frontend/jest.setup.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import '@testing-library/jest-dom/extend-expect';
4 changes: 4 additions & 0 deletions frontend/next.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

const nextConfig = {
reactStrictMode: true,
compiler: {
// Enables the styled-components SWC transform
styledComponents: true,
},
};

module.exports = nextConfig;
14 changes: 12 additions & 2 deletions frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Author

Choose a reason for hiding this comment

The 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"
}
}
1 change: 1 addition & 0 deletions frontend/prettier.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@

module.exports = {
singleQuote: true,
tabWidth: 2,
};
129 changes: 129 additions & 0 deletions frontend/src/components/modal.tsx
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;
44 changes: 44 additions & 0 deletions frontend/src/hooks/__tests__/useDisableScroll.spec.tsx
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);
});
Copy link
Author

Choose a reason for hiding this comment

The 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.
Please forgive me 🙇

});
64 changes: 64 additions & 0 deletions frontend/src/hooks/__tests__/useOnClickOutside.spec.tsx
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();
});
});
24 changes: 24 additions & 0 deletions frontend/src/hooks/__tests__/useOnEscKeypress.spec.tsx
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);
});
});
5 changes: 5 additions & 0 deletions frontend/src/hooks/index.tsx
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 };
15 changes: 15 additions & 0 deletions frontend/src/hooks/useDisableScroll.tsx
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;
25 changes: 25 additions & 0 deletions frontend/src/hooks/useOnClickOutside.tsx
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;