feat: Add ability to create boards #33
Conversation
| @@ -1,5 +1,6 @@ | |||
| import React from 'react'; | |||
| import Box from '@material-ui/core/Box'; | |||
| import * as S from './Boards.styles'; | |||
There was a problem hiding this comment.
Do you think using the full name (like Styles) instead of S can help with readability?
There was a problem hiding this comment.
I feel like S is short and sweet.
| describe('generateBoardName', () => { | ||
| beforeAll(() => { | ||
| MockDate.set('1/30/2000'); | ||
| }); |
| }); | ||
| afterAll(() => { | ||
| MockDate.reset(); | ||
| }); |
| @@ -0,0 +1,9 @@ | |||
| export default function() { | |||
There was a problem hiding this comment.
Is this redundant given BoardContainer.tsx?
There was a problem hiding this comment.
You mean just leaving this in inline in board container? Can go either way on this one, not opinionated one way or another.
There was a problem hiding this comment.
We can keep as is for now and if it makes sense to refactor later, we can do it
|
|
||
| export default function(options?: { client?: typeof firebase }) { | ||
| const { firebase } = useContext(FirebaseContext); | ||
| const client = (options && options.client) || firebase; |
There was a problem hiding this comment.
Minor: you could also write as ternary (no need for change)
| import { FirebaseContext } from '../../components/Firebase'; | ||
| import { GroupType } from '../types'; | ||
|
|
||
| export default function(options?: { client?: typeof firebase }) { |
There was a problem hiding this comment.
Should we type the return value given that it's a Promise or is that inferred?
| @@ -0,0 +1,16 @@ | |||
| import generateBoardName from './generateBoardName'; | |||
| const MockDate = require('mockdate'); | |||
There was a problem hiding this comment.
mockdate might not be that valuable since we can do it ourselves, but I think it's nice for convenience
|
LGTM |
benox3
force-pushed
the
feat/create-boards
branch
from
2494845 to
85007c7
Compare
Partially resolves #25 and resolves #22