Load PT data into stories #25
Conversation
sracca
commented
Jul 29, 2019
•
sracca
commented
- maps each story to story.tsx
- add labels for each story to end
- shows types based on icon (not finished, waiting for chapman)
sracca
added
the
do not merge
sracca
force-pushed
the
sr/load_stories
branch
2 times, most recently
from
1546ac7 to
7bcf46c
Compare
There was a problem hiding this comment.
This looks awesome! Just a few nits but nothing significant. The base branch mpp/display_stories was already merged to master so maybe rebase to master with this. Also, were we holding off on implementing the arrows to change between stories and just displaying all of them at once for now?
| @@ -12,7 +12,7 @@ const Project = ({ data }: Props) => { | |||
| <> | |||
| <h1>Project {data.id}</h1> | |||
| {data.stories.map(story => ( | |||
| <Story key={story.id} data={story} /> | |||
| <Story data={story} /> | |||
There was a problem hiding this comment.
Unless we don't care about the Each child in an array should have a unique "key" prop React warning then I think we should keep the key={story.id}
There was a problem hiding this comment.
@sracca this was marked as resolved -- not sure if that was a mistake or if there's a reason to not want unique keys?
There was a problem hiding this comment.
no i did add it? That's weird
There was a problem hiding this comment.
@gerald are you looking at the most recent commits I've pushed up?
| @@ -1,6 +1,7 @@ | |||
| import React from 'react'; | |||
| import styled from 'styled-components'; | |||
| import { fonts, colors, fontSizes, spacing } from '~lib/theme'; | |||
| import { Label as LabelType } from '~components/projects/types'; | |||
There was a problem hiding this comment.
nit: I think @geraldalewis suggested it was better to do something like import * as Types from '~components/projects/types' and then do Types.Label instead of LabelType, but this was only in the case when we were importing a bunch of types.
@mariana The arrows were taken out of the zepplin designs |
| </DescripTitle> | ||
| <DescripBody>{text}</DescripBody> | ||
| <DescripTitle>{title}</DescripTitle> | ||
| {text !== null && text !== '' ? ( |
There was a problem hiding this comment.
Oh crud -- we need --strictNullChecks -- my bad :(
Updated in #27
There was a problem hiding this comment.
@geraldalewis how would this change once #27 gets merged?
|
I think it looks good to merge. All is left is to edit the base branch to be |
sracca
force-pushed
the
sr/load_stories
branch
2 times, most recently
from
2d0d81e to
1246407
Compare
sracca
force-pushed
the
sr/load_stories
branch
3 times, most recently
from
e04352a to
6788f32
Compare
mariana0pachon
removed
the
do not merge
